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

Fixes #11335: Default manager for ObjectChange should filter by installed apps #11709

Merged
merged 5 commits into from
Jul 5, 2023

Conversation

jeremystretch
Copy link
Member

Fixes: #11335

Introduce a custom default manager for ObjectChange, which automatically filters results to those referencing models belonging to currently installed apps.

All credit goes to @kkthxbye-code for coming up with this solution!

netbox/extras/querysets.py Outdated Show resolved Hide resolved
@kkthxbye-code
Copy link
Contributor

Posting my slack message here as well for posterity:

ContentType.objects.clear_cache() after app_labels = ... fixes tests for me

@jeremystretch
Copy link
Member Author

We're probably breaking some Django rules by attempting to evaluate registered apps within the model's base manager. A compromise might be to introduce an alternate manager that returns only the "valid" instances (e.g. ObjectChange.valid_objects.all(), which can be called from views. This won't address the potential issue globally but it might be a sufficient band-aid to address the (somewhat niche) concern.

@jeremystretch jeremystretch force-pushed the 11335-objectchange-manager branch from b0c32ea to 167e851 Compare March 10, 2023 16:01
@jeremystretch
Copy link
Member Author

Although the current approach appears to resolve the root issue, it causes the tests to fail, presumably because we're attempting to resolve the content types too early. There may be a way to resole these lazily; still need to dig into this.

@abhi1693
Copy link
Member

abhi1693 commented May 6, 2023

@jeremystretch This is ready for another review

@abhi1693 abhi1693 requested a review from kkthxbye-code May 6, 2023 16:10
@jeremystretch jeremystretch force-pushed the 11335-objectchange-manager branch from fe50aa1 to 7852522 Compare May 24, 2023 20:37
@jeremystretch jeremystretch merged commit 63ba9fb into develop Jul 5, 2023
@jeremystretch jeremystretch deleted the 11335-objectchange-manager branch July 5, 2023 15:39
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

table.html throws an 'NoneType' object has no attribute '_base_manager' when a plugin is disabled
3 participants