Skip to content

Old Django Taggit codenames not removed #3345

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

Closed
bdlamprecht opened this issue Jul 12, 2019 · 2 comments
Closed

Old Django Taggit codenames not removed #3345

bdlamprecht opened this issue Jul 12, 2019 · 2 comments

Comments

@bdlamprecht
Copy link
Contributor

Environment

  • Python version: 3.6.8
  • NetBox version: v2.6.1

Similar to issue #2624 and interface-connections, when the custom tag models for Django was introduced, the old standard taggit models were not removed during any migration.

This is an issue for when you want to programatically assign permissions to users/groups (as is available in netbox-docker).

The evidence can be seen by following the steps below.

Steps to Reproduce

  1. Enter the python shell
  2. Import the Django Permission library
  3. Loop over and print each codename
$ cd /opt/netbox/netbox
$ ./manage.py shell

>>> from django.contrib.auth.models import Permission
>>> for p in Permission.objects.all():
>>>  print(p.codename)

What you see is two sets of permissions for tags with the same name.
On lines 207 - 214:

add_tag
change_tag
delete_tag
view_tag
add_taggeditem
change_taggeditem
delete_taggeditem
view_taggeditem

And another set on lines 280-287:

add_tag
change_tag
delete_tag
view_tag
add_taggeditem
change_taggeditem
delete_taggeditem
view_taggeditem

Expected Behavior

Be able to add permissions to users / groups programatically.

Observed Behavior

When bringing up netbox, this error occurs:

django.contrib.auth.models.MultipleObjectsReturned: get() returned more than one Permission -- it returned 2!
@jeremystretch
Copy link
Member

To clarify, one set of permissions is for extras.Tag and the other is for taggit.Tag:

>>> for p in Permission.objects.all():
...     if p.codename.endswith('tag'):
...         print(p.content_type.app_label, p.codename)
... 
extras add_tag
extras change_tag
extras delete_tag
extras view_tag
taggit add_tag
taggit change_tag
taggit delete_tag
taggit view_tag

Permission code names aren't necessarily unique on their own: only the set (content_type, codename) must be unique. NetBox doesn't currently have any duplicate model names, but this might not always be the case: #2199 for instance would result in two models named "Role." It's a good idea to get into the habit now of not assuming model names to be unique.

Beside that, the Tag model provided by django-taggit is still technically in use (since the model is installed by the package); it wouldn't make sense to remove the permission objects for it.

@jeremystretch
Copy link
Member

Related: I did open #3347 to extend the upgrade process to include removing stale content types. This won't affect the taggit.Tag model or its permissions but there are a couple other content types that are due to be removed.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants