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

Define roles in migration instead of fixture #748

Closed
1 task
timobrembeck opened this issue Mar 17, 2021 · 0 comments · Fixed by #1088
Closed
1 task

Define roles in migration instead of fixture #748

timobrembeck opened this issue Mar 17, 2021 · 0 comments · Fixed by #1088
Assignees
Labels
‼️ prio: high Needs to be resolved ASAP. 🔨 enhancement This improves an existing feature

Comments

@timobrembeck
Copy link
Member

Motivation

Defining our roles as a fixture comes with the caveat that permissions are referenced by the ids. This is barely readable and once the ids change (e.g. because a new permission gets added), the permissions assigned to a role are completely different.

Proposed Solution

Program the permissions as a Django migration instead of a fixture. This also makes it easier to add and edit roles because the changes can be defined in new migrations based on the initial one.

Proposed implementation
from django.db import migrations, models


def add_roles(apps, schema_editor):

    ROLES = [
        {
            "name": "Verwaltung",
            "staff_role": False,
            "permissions": [
                "edit_events",
                "publish_events",
                "view_events",
                "manage_feedback",
                "manage_imprint",
                "manage_language_tree",
                "manage_offers",
                "edit_pages",
                "grant_page_permissions",
                "publish_pages",
                "manage_pois",
                "edit_push_notifications",
                "send_push_notifications",
                "view_push_notifications",
                "manage_region_users",
            ],
        },
        {
            "name": "Terminplaner",
            "staff_role": False,
            "permissions": [
                "view_events",
                "edit_events",
                "publish_events",
                "manage_pois",
            ],
        },
        {
            "name": "Redakteur",
            "staff_role": False,
            "permissions": ["edit_pages", "publish_pages", "grant_page_permissions"],
        },
    ]

    # We can't import the Person model directly as it may be a newer
    # version than this migration expects. We use the historical version.
    Group = apps.get_model("auth", "Group")
    Permission = apps.get_model("auth", "Permission")
    Role = apps.get_model("cms", "Role")

    for role_conf in ROLES:
        group, created = Group.objects.get_or_create(name=role_conf.get("name"))
        role, created = Role.objects.get_or_create(
            group=group, staff_role=role_conf.get("staff_role")
        )
        permissions = Permission.objects.filter(codename__in=permission_codenames)
        group.permissions.add(*permissions)


class Migration(migrations.Migration):

    dependencies = [("cms", "0001_initial")]

    operations = [
        migrations.RunPython(add_roles),
    ]

Alternatives

Additional Context

This is blocked by:

@timobrembeck timobrembeck added ⁉️ prio: low Not urgent, can be resolved in the distant future. 🔨 enhancement This improves an existing feature labels Mar 17, 2021
@timobrembeck timobrembeck added this to the Feature Completion milestone Mar 17, 2021
@timobrembeck timobrembeck self-assigned this Mar 17, 2021
@svenseeberg svenseeberg added ‼️ prio: high Needs to be resolved ASAP. and removed ⁉️ prio: low Not urgent, can be resolved in the distant future. labels Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
‼️ prio: high Needs to be resolved ASAP. 🔨 enhancement This improves an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants