create_permissions method fails if the model has a single new permission

[Mario César]

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

[Mario César]

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

[anonymous]

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

[Mario César]

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.

[Preston Holmes]

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.

[Mario César]

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

[Mario César]

*!

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'),
)

[Preston Holmes]

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