Skip to content
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

Skip GenericForeignKey fields #58

Merged

Conversation

bmbouter
Copy link
Contributor

@bmbouter bmbouter commented Aug 3, 2020

The GenericForeignKey field does not provide a get_internal_type
method so when checking if it's a ForeignKey or not an AttributeError is
raised.

This adjusts the code to ignore this AttributeError which effectively
un-monitors the GenericForeignKey itself. However, it does leave the
underlying ForeignKey to the ContentType table and the primary key
storage field indexing into that table monitored. This does not enable
support for hooking on the name of the GenericForeignKey, but hooking
on the underlying fields that support that GenericForeignKey should
still be possible.

closes #42

@bmbouter
Copy link
Contributor Author

bmbouter commented Aug 3, 2020

@rsinger86 what do you think about this? Can we do any better in this case?

The `GenericForeignKey` field does not provide a `get_internal_type`
method so when checking if it's a ForeignKey or not an AttributeError is
raised.

This adjusts the code to ignore this `AttributeError` which effectively
un-monitors the `GenericForeignKey` itself. However, it does leave the
underlying `ForeignKey` to the `ContentType` table and the primary key
storage field indexing into that table monitored. This does not enable
support for hooking on the name of the `GenericForeignKey`, but hooking
on the underlying fields that support that `GenericForeignKey` should
still be possible.

closes rsinger86#42
@bmbouter bmbouter force-pushed the allow-use-with-generic-foreign-key branch from 157c6f1 to 8a9fead Compare August 4, 2020 15:17
@bmbouter
Copy link
Contributor Author

bmbouter commented Aug 4, 2020

These are all passing in the CI of my fork: https://travis-ci.com/github/bmbouter/django-lifecycle/builds/178427056

@bmbouter
Copy link
Contributor Author

bmbouter commented Aug 5, 2020

@rsinger86 I've just learned that I can't have our project use my fork of django-lifecycle because we release to PyPI. This is really challenging because to remove our use of django-lifecycle is a significant amount of effort, only to put it back in place. I need to merge my code by Friday and this PR and my code in my project are both ready to go. Please advise.

@rsinger86 rsinger86 merged commit 1aa4fb7 into rsinger86:master Aug 6, 2020
@bmbouter bmbouter deleted the allow-use-with-generic-foreign-key branch August 6, 2020 13:05
@bmbouter
Copy link
Contributor Author

bmbouter commented Aug 6, 2020

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using @hook on a model with a GenericForeignKey doesn't work
2 participants