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: #14840 - Forces API to use django user model instead of proxy model #14881

Merged
merged 8 commits into from
Feb 5, 2024

Conversation

DanSheps
Copy link
Member

@DanSheps DanSheps commented Jan 20, 2024

Fixes: #14840 - Forces API to use django user model instead of proxy model

  • Swap the model name in get_viewname for only NetBoxUser and NetBoxGroup

@DanSheps DanSheps removed the request for review from jeremystretch January 23, 2024 15:41
@DanSheps DanSheps marked this pull request as draft January 23, 2024 15:42
@DanSheps DanSheps marked this pull request as ready for review January 24, 2024 17:51
@DanSheps
Copy link
Member Author

I think this will be a much more acceptable resolution to the problem, it does introduce another API endpoint (but maintains all the filters from /users/, I couldn't find a clean way of customizing the "basename" without modifying NetBoxRouter).

Alternatively, we could swap the proxy model in the ContentTypeManager for the actual user model

@jeremystretch
Copy link
Member

We shouldn't add a redundant API endpoint for the same model. Instead, we can tweak the logic in get_viewname() to resolve the view for the concrete model (User) instead of the proxy model (NetBoxUser). There are a few other places where we use proxy models though, so we'll need to check that those are not broken as a result of this change.

Ultimately this problem should be side-stepped by the introduction of a proper custom user model in v4.0 under #12795.

@DanSheps
Copy link
Member Author

We shouldn't add a redundant API endpoint for the same model. Instead, we can tweak the logic in get_viewname() to resolve the view for the concrete model (User) instead of the proxy model (NetBoxUser). There are a few other places where we use proxy models though, so we'll need to check that those are not broken as a result of this change.

You are absolutely correct, that is a much easier way to handle that. Made a tweak to swap the proxy model to the user model and it looks to be good now.

@DanSheps DanSheps changed the title Fixes: #14840 - Forces API to use proxy model Fixes: #14840 - Forces API to use django user model Jan 26, 2024
@DanSheps DanSheps changed the title Fixes: #14840 - Forces API to use django user model Fixes: #14840 - Forces API to use django user model instead of proxy model Jan 26, 2024
@jeremystretch
Copy link
Member

This doesn't appear to fully resolve #14840. Although you can now edit an object with a custom field pointing to the User model, attempting to save it raises a SerializerNotFound exception:

Could not determine serializer for users.NetBoxUser with prefix 'Nested'

Copy link
Member

@jeremystretch jeremystretch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @DanSheps!

@jeremystretch jeremystretch merged commit 32083e5 into develop Feb 5, 2024
8 checks passed
@jeremystretch jeremystretch deleted the 14840-fix_netboxusers_reverse branch February 5, 2024 16:57
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 7, 2024
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.

Reverse for 'netboxuser-list' not found when editing
2 participants