-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deep tree handler customization support #265
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! That makes it easy to do the kind of stuff I want to do. I could get the demo running with django-guardian.
with the help of `SITETREE_CLS` setting. | ||
|
||
1. Subclass `sitetreeapp.SiteTree` and place that class into a separate module for convenience. | ||
2. Override methods you need for customization (usually `.apply_hook()`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From that description I don't really understand how .apply_hook() comes into play.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.appy_hook()
is most generic — this description doesn't necessarily cover your very case from #263
Rather you override .get_permissions()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, thats right. I just wanted to point out, that somebody who reads this sentence might not understand if he has to overwrite .apply_hook() or if this is just one option to deal with a usecase.
But I guess if people are doing such things they must have a look at the code and then everything is pretty self-explanatory anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. If you can propose a better wording, I'd be glad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be a little bit more explicit, like this:
You can now overwrite .apply_hook()
to manipulate values at pre-defined hooks. You can also overwrite any other methods to customize to your exact needs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Thank you.
No description provided.