[admin] allow administrator role access organization admin #143

[NoumbissiValere]

Closed issue by updating organization admin and also added
a couple number of tests for these changes.

Fixes #143

[coveralls]

Coverage increased (+0.02%) to 97.399% when pulling 52be16c on issue-143-administrators_access_organization into be43f32 on master.

[nemesifier]

Good progress! Sorry for my late response.

For some reason while testing, the permissions were not added to the administrator group correctly in my env.

So I wanted to run the migration backward to run it again and found this:

    raise IrreversibleError("Operation %s in %s is not reversible" % (operation, self))
django.db.migrations.exceptions.IrreversibleError: Operation <RunPython <function allow_admins_change_organization at 0x7fa9626b6dd0>> in openwisp_users.0010_allow_admins_change_organization is not reversible

PS: also rebase on the latest master please

[nemesifier]

rather than following the redirect, assert the status code to ensure it's a redirect and remove the following lines which are not relevant anymore

[NoumbissiValere]

Sure. Will do that today

[NoumbissiValere]

Good progress! Sorry for my late response.

For some reason while testing, the permissions were not added to the administrator group correctly in my env.

So I wanted to run the migration backward to run it again and found this:

    raise IrreversibleError("Operation %s in %s is not reversible" % (operation, self))
django.db.migrations.exceptions.IrreversibleError: Operation <RunPython <function allow_admins_change_organization at 0x7fa9626b6dd0>> in openwisp_users.0010_allow_admins_change_organization is not reversible

PS: also rebase on the latest master please

@nemesisdesign It says that because I didn't make the migration reversible. I didn't see a reason to make this reversible since its absence will cause other core functionalities which were added to malfunction.

Should i make it reversible?

[nemesifier]

@nemesisdesign It says that because I didn't make the migration reversible. I didn't see a reason to make this reversible since its absence will cause other core functionalities which were added to malfunction.

Should i make it reversible?

In this case we can simply set the backward migration to noop, no operation, look it up on the django docs please.

[NoumbissiValere]

@nemesisdesign This is ready for review

[nemesifier]

Good progress, see my hints below and we're ready to move on

[nemesifier]

it looks like this is here but it's not being imported at the top, don't you have flake8 configured in your editor to highlight these issues? I wonder why this doesn't pop up in the global flake8 check.

[NoumbissiValere]

@nemesisdesign Please i don't understand what you are asking for. it looks like your sentence is missing some words

[nemesifier]

no missing word, please read carefully: ObjectDoesNotExist is used here but it's not imported.
Please activate flake8 checks in your editor.

[nemesifier]

I created a patch for this issue: #157
I will merge that and then you only need to rebase.

[nemesifier]

please simplify, we added get_model to avoid those lines, remember?

Group = get_model(apps, 'Group')
try:
    admins = Group.objects.get(name='Administrator')
    permissions = [
        Permission.objects.get(
            content_type__app_label='openwisp_users', codename='change_organization'
        ).pk,
        Permission.objects.get(
            content_type__app_label='openwisp_users', codename='change_organizationowner'
        ).pk,
    ]
    admins.permissions.add(*permissions)

[nemesifier]

here's an improved version:

Here's a summary of the default permissions:

- All users who belong to the Administrators group and are organization
  managers (``OrganizationUser.is_admin=True``), have the permission to edit
  the organizations details which they administrate.
- Only super users have the permission to add and delete organizations.
- Only super users and organization owners have the permission to change
  the ``OrganizationOwner`` inline or delete the relation.

[nemesifier]

👍