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

Feature 15492 - Copying of Admin->Authentication->Permissions #15976

Conversation

Julio-Oliveira-Encora
Copy link
Contributor

Fixes: #15492

It was created a copy of CloningMixin in users/features.py and used it in the ObjectPermission.

Added CloningUserMixin to ObjectPermission to enable clone_fields.
Comment on lines 10 to 12
Provides the clone() method used to prepare a copy of existing objects.
The same code from netbox/users/models/features.py (CloningMixin) is used here.
It was necessary to avoid circular imports.
Copy link
Member

Choose a reason for hiding this comment

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

It's not acceptable to duplicate code from other modules, as this poses serious challenges to long-term maintainability. If employing the original mixin class results in a circular dependency, some other solution must be identified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One solution would be to create a "clone" method in the ObjectPermission and remove support for "tags" and "custom_fields" (they do not exist on the Users module).
I tried some solutions to avoid duplicate code, but they did not work.
I tested removing "from .permissions import *" from "init.py" and it showed promise, however this impacts several other files and I believe this solution is not viable.

@jeremystretch
Copy link
Member

Going to close this out as #15492 is blocked by #16058.

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.

None yet

2 participants