-
Notifications
You must be signed in to change notification settings - Fork 4
Feature/modelhook #8
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
Conversation
I'm not merging this just yet, mostly coz of the lack of tests. But I'm pretty satisfy by the way this works so far. |
abstract = True | ||
|
||
def save(self, *args, **kwargs): | ||
updated_at = timezone.now() |
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.
shoud be self.updated_at
I can't decide whether this is a good or bad idea. Except for the potential clash in field names I see no downsides. It avoids resorting to oneToOneField and have N extra join (N == Number of plugins), and it makes working with forms and ordering their fields easier (or possible at all). But I've not merged it in 3 years, so closing this. |
I think this is a bad idea because it won't handle migrations well. We would need to ask the final user to create a concrete model, so all migrations are stored on the user project side instead of as part of django-hooks, mainly because when django-hooks is updated the migrations will be deleted. This also has major caveats, the main app will have no way to create data migrations when needed, since the migrations won't be stored in that app, but in some user custom app. |
Fixes #7
Changes: