Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#19178 closed Bug (wontfix)

create_permissions method fails if the model has a single new permission

Reported by: Mario César Owned by: nobody
Component: contrib.auth Version: 1.4
Severity: Normal Keywords:
Cc: mhaligowski Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I notice this by using this command → https://gist.github.com/3946353, it list all apps and update the permissions in the db if there is any new.

The stacktrace is:

Traceback (most recent call last):
  File "/home/mariocesar/Proyectos/Crowddeals/env/local/lib/python2.7/site-packages/django/core/management/base.py", line 222, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/home/mariocesar/Proyectos/Crowddeals/env/local/lib/python2.7/site-packages/django/core/management/base.py", line 252, in execute
    output = self.handle(*args, **options)
  File "/home/mariocesar/Proyectos/Crowddeals/crowddeals/core/management/commands/update_permissions.py", line 29, in handle
    create_permissions(app, get_models(), options.get('verbosity', 0))
  File "/home/mariocesar/Proyectos/Crowddeals/env/local/lib/python2.7/site-packages/django/contrib/auth/management/__init__.py", line 74, in create_permissions
    for perm in _get_all_permissions(klass._meta, ctype):
  File "/home/mariocesar/Proyectos/Crowddeals/env/local/lib/python2.7/site-packages/django/contrib/auth/management/__init__.py", line 28, in _get_all_permissions
    _check_permission_clashing(custom, builtin, ctype)
  File "/home/mariocesar/Proyectos/Crowddeals/env/local/lib/python2.7/site-packages/django/contrib/auth/management/__init__.py", line 49, in _check_permission_clashing
    for codename, _name in custom:
ValueError: too many values to unpack

I see that in ´contrib/auth/management/init.py´ the method _check_permission_clashing unpack the custom new methods. Normally the list of permissions will be a list of list, however if there is just one new permission, when getting the permissiosn it returns just a list of strings.

Making the _get_all_permissios code aware of that will fix the problem.

def _get_all_permissions(opts, ctype):
    """
    Returns (codename, name) for all permissions in the given opts.
    """
    builtin = _get_builtin_permissions(opts)
    custom = list(opts.permissions)
    _check_permission_clashing(custom, builtin, ctype)
    return builtin + custom
def _get_all_permissions(opts, ctype):
    """
    Returns (codename, name) for all permissions in the given opts.
    """
    builtin = _get_builtin_permissions(opts)
    custom = list(opts.permissions)

    if any(isinstance(el, basestring) for el in custom):
        custom = [custom]

    _check_permission_clashing(custom, builtin, ctype)
    return builtin + custom

Change History (7)

comment:1 by Mario César, 11 years ago

I just made a draft patch → https://github.com/django/django/pull/465

comment:2 by anonymous, 11 years ago

Could you attach a sample model with failing Meta.permissions (a test case in pull request would be even better...).

EDIT: forgot to log in - akaariai

Last edited 11 years ago by Anssi Kääriäinen (previous) (diff)

comment:3 by Mario César, 11 years ago

My case is something like, having first created with syncdb this model:

class UserProfile(models.Model):
    user = models.ForeignKey(User)

    class Meta:
        permissions = (
            ('can_approve', 'Can approve'),
            ('can_dismiss', 'Can dismiss')
        )

If later I add a single new permission

class UserProfile(models.Model):
    user = models.ForeignKey(User)

    class Meta:
        permissions = (
            ('can_approve', 'Can approve'),
            ('can_dismiss', 'Can dismiss'),
            ('can_dance', 'Can dance')
        )

It fails. If the new permissions where two 'can_dance', 'can_sing' it works.

Where do I put the test case? I'm kind of lost about how write a test for this.

Last edited 11 years ago by Mario César (previous) (diff)

comment:4 by Preston Holmes, 11 years ago

tests probably should go in contrib/auth/tests/management.py

the create_permission function really should be part of a permissions API that is defined outside of the management/management command context - but that is where it is for now.

comment:5 by Mario César, 11 years ago

I added a new test, here is the stacktrace if there is no patch

======================================================================
ERROR: test_bug_19178 (django.contrib.auth.tests.management.PermissionDuplicationTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mariocesar/Proyectos/Django/.env/local/lib/python2.7/site-packages/django/contrib/auth/tests/management.py", line 228, in test_bug_19178
    create_permissions(models, [], verbosity=0)
  File "/home/mariocesar/Proyectos/Django/.env/local/lib/python2.7/site-packages/django/contrib/auth/management/__init__.py", line 73, in create_permissions
    for perm in _get_all_permissions(klass._meta, ctype):
  File "/home/mariocesar/Proyectos/Django/.env/local/lib/python2.7/site-packages/django/contrib/auth/management/__init__.py", line 28, in _get_all_permissions
    _check_permission_clashing(custom, builtin, ctype)
  File "/home/mariocesar/Proyectos/Django/.env/local/lib/python2.7/site-packages/django/contrib/auth/management/__init__.py", line 48, in _check_permission_clashing
    for codename, _name in custom:
ValueError: too many values to unpack

will be pushing the patch in a minute

comment:6 by Mario César, 11 years ago

Resolution: wontfix
Status: newclosed

*!

May the earth swallow me ... Sorry for making lose your time, running the tests I realize that error was about packing incorrectly the tuples in my models.

My fix will solve the problem is someone do something like

permissions = (
  ('can_approve', 'Can approve')
)

permissions, will be end as ('can_approve', 'Can approve'), as it's tuple(tuple()). Of course all (not me) sane people do.

permissions = (
  ('can_approve', 'Can approve'),
)

comment:7 by Preston Holmes, 11 years ago

No problem - that is why a test is always a great way to validate a bug report.

Note: See TracTickets for help on using tickets.
Back to Top