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

feat: add MANAGE_TAGS permission #4628

Merged
merged 4 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions api/permissions/migrations/0010_add_manage_tags_permission.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Generated by Django 4.2.15 on 2024-09-13 16:18
from django.apps.registry import Apps
from django.db import migrations
from django.db.backends.base.schema import BaseDatabaseSchemaEditor

from permissions.models import PROJECT_PERMISSION_TYPE
from projects.permissions import MANAGE_TAGS


def add_manage_tags_permission(apps: Apps, schema_editor: BaseDatabaseSchemaEditor) -> None:
permission_model_class = apps.get_model("permissions", "permissionmodel")
permission_model_class.objects.get_or_create(
key=MANAGE_TAGS,
description="Allows the user to manage tags in the given project.",
type=PROJECT_PERMISSION_TYPE,
)


def reverse(apps: Apps, schema_editor: BaseDatabaseSchemaEditor) -> None: # pragma: no cover
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Justification for adding pragma: no cover here:

Considering the complexity of the code, and the very limited scope in which this code will ever be executed, I figured that it wasn't worth writing a migration test for it (since they are quite arduous to write).

I have tested the migration forwards and backwards manually.

permission_model_class = apps.get_model("permissions", "permissionmodel")
permission_model_class.objects.filter(key=MANAGE_TAGS).delete()


class Migration(migrations.Migration):

dependencies = [
("permissions", "0009_move_view_audit_log_permission"),
]

operations = [
migrations.RunPython(add_manage_tags_permission, reverse_code=reverse),
]
2 changes: 2 additions & 0 deletions api/projects/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
CREATE_FEATURE = "CREATE_FEATURE"
EDIT_FEATURE = "EDIT_FEATURE"
MANAGE_SEGMENTS = "MANAGE_SEGMENTS"
MANAGE_TAGS = "MANAGE_TAGS"

TAG_SUPPORTED_PERMISSIONS = [DELETE_FEATURE]

Expand All @@ -28,6 +29,7 @@
(EDIT_FEATURE, "Ability to edit features in the given project."),
(MANAGE_SEGMENTS, "Ability to manage segments in the given project."),
(VIEW_AUDIT_LOG, "Allows the user to view the audit logs for this organisation."),
(MANAGE_TAGS, "Allows the user to manage tags in the given project."),
]


Expand Down
11 changes: 4 additions & 7 deletions api/projects/tags/permissions.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from rest_framework.permissions import BasePermission

from projects.models import Project
from projects.permissions import VIEW_PROJECT
from projects.permissions import MANAGE_TAGS, VIEW_PROJECT


class TagPermissions(BasePermission):
Expand All @@ -22,12 +22,9 @@ def has_permission(self, request, view):

def has_object_permission(self, request, view, obj):
project = obj.project
if request.user.is_project_admin(obj.project):
if request.user.has_project_permission(MANAGE_TAGS, obj.project):
return True

if view.action == "detail" and request.user.has_project_permission(
return view.action == "detail" and request.user.has_project_permission(
VIEW_PROJECT, project
):
return True

return False
)
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from unittest import mock

from projects.models import Project
from projects.permissions import VIEW_PROJECT
from projects.permissions import MANAGE_TAGS, VIEW_PROJECT
from projects.tags.models import Tag
from projects.tags.permissions import TagPermissions
from tests.types import WithProjectPermissionsCallable
Expand Down Expand Up @@ -142,3 +142,25 @@ def test_project_user_has_detail_permission(

# Then
assert result is True


def test_project_user_with_manage_tags_has_detail_permission(
staff_user: FFAdminUser,
project: Project,
with_project_permissions: WithProjectPermissionsCallable,
) -> None:
# Given
with_project_permissions([VIEW_PROJECT, MANAGE_TAGS])
mock_request = mock.MagicMock(user=staff_user)
mock_view = mock.MagicMock(
action="detail",
kwargs={"project_pk": project.id},
)
permissions = TagPermissions()
tag = Tag.objects.create(label="test", project=project)

# When
result = permissions.has_object_permission(mock_request, mock_view, tag)

# Then
assert result is True
2 changes: 1 addition & 1 deletion api/tests/unit/projects/test_unit_projects_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ def test_can_list_project_permission(client, project):
# Then
assert response.status_code == status.HTTP_200_OK
assert (
len(response.json()) == 6
len(response.json()) == 7
) # hard code how many permissions we expect there to be


Expand Down
Loading