fix: allow project admins to create and manage project-scoped custom fields (#7518)

Author: 10done

api/metadata/permissions.py

@@ -74,9 +74,10 @@ def has_permission(self, request, view):  # type: ignore[no-untyped-def]
                 if view.action == "create":
                     field = MetadataField.objects.get(id=request.data.get("field"))
 
-                    return (
+                    return (organisation_pk == field.organisation.id) and (
                         request.user.is_organisation_admin(organisation_pk)
-                        and organisation_pk == field.organisation.id
+                        or (project := field.project)
+                        and request.user.is_project_admin(project)
                     )
 
         return False
@@ -86,6 +87,10 @@ def has_object_permission(self, request, view, obj):  # type: ignore[no-untyped-
             return request.user.belongs_to(obj.field.organisation.id)
 
         if view.action in ("update", "destroy", "partial_update"):
-            return request.user.is_organisation_admin(obj.field.organisation)
+            return (
+                request.user.is_organisation_admin(obj.field.organisation)
+                or (project := obj.field.project)
+                and request.user.is_project_admin(project)
+            )
 
         return False

api/tests/unit/metadata/test_views.py

@@ -14,7 +14,7 @@
 )
 from metadata.views import SUPPORTED_REQUIREMENTS_MAPPING  # type: ignore[attr-defined]
 from organisations.models import Organisation
-from projects.models import Project
+from projects.models import Project, UserProjectPermission
 from users.models import FFAdminUser
 
 
@@ -511,8 +511,6 @@ def test_create_metadata_field__project_admin__returns_201(
     project: Project,
 ) -> None:
     # Given
-    from projects.models import UserProjectPermission
-
     UserProjectPermission.objects.create(user=staff_user, project=project, admin=True)
 
     url = reverse("api-v1:metadata:metadata-fields-list")
@@ -942,8 +940,6 @@ def test_modify_metadata_field__project_admin__permission_check(
     request: pytest.FixtureRequest,
 ) -> None:
     # Given
-    from projects.models import UserProjectPermission
-
     UserProjectPermission.objects.create(user=staff_user, project=project, admin=True)
     field_project = (
         request.getfixturevalue(field_project_attr) if field_project_attr else None
@@ -969,3 +965,172 @@ def test_modify_metadata_field__project_admin__permission_check(
 
     # Then
     assert response.status_code == expected_status
+
+
+def test_create_model_metadata_field__project_admin__returns_201(
+    staff_user: FFAdminUser,
+    staff_client: APIClient,
+    organisation: Organisation,
+    project: Project,
+    feature_content_type: ContentType,
+) -> None:
+    # Given - a project-scoped MetadataField and a user who is project admin (not org admin)
+    UserProjectPermission.objects.create(user=staff_user, project=project, admin=True)
+    project_field = MetadataField.objects.create(
+        name="proj_field",
+        type="int",
+        organisation=organisation,
+        project=project,
+    )
+    url = reverse(
+        "api-v1:organisations:metadata-model-fields-list", args=[organisation.id]
+    )
+    data = {
+        "field": project_field.id,
+        "content_type": feature_content_type.id,
+        "is_required_for": [],
+    }
+
+    # When
+    response = staff_client.post(
+        url, data=json.dumps(data), content_type="application/json"
+    )
+
+    # Then - project admin should be allowed to bind a project-scoped field
+    assert response.status_code == status.HTTP_201_CREATED
+    assert response.json()["field"] == project_field.id
+
+
+def test_create_model_metadata_field__project_admin_org_scoped_field__returns_403(
+    staff_user: FFAdminUser,
+    staff_client: APIClient,
+    organisation: Organisation,
+    project: Project,
+    feature_content_type: ContentType,
+) -> None:
+    # Given - an org-scoped MetadataField (project=None) and a user who is project admin only
+    UserProjectPermission.objects.create(user=staff_user, project=project, admin=True)
+    org_field = MetadataField.objects.create(
+        name="org_field",
+        type="int",
+        organisation=organisation,
+    )
+    url = reverse(
+        "api-v1:organisations:metadata-model-fields-list", args=[organisation.id]
+    )
+    data = {
+        "field": org_field.id,
+        "content_type": feature_content_type.id,
+        "is_required_for": [],
+    }
+
+    # When
+    response = staff_client.post(
+        url, data=json.dumps(data), content_type="application/json"
+    )
+
+    # Then - project admin must not be able to bind org-scoped fields
+    assert response.status_code == status.HTTP_403_FORBIDDEN
+
+
+@pytest.mark.parametrize(
+    "field_project_attr, expected_status",
+    [
+        ("project", status.HTTP_200_OK),
+        (None, status.HTTP_403_FORBIDDEN),
+        ("project_b", status.HTTP_403_FORBIDDEN),
+    ],
+    ids=[
+        "own_project_field_allowed",
+        "org_scoped_field_denied",
+        "other_project_field_denied",
+    ],
+)
+def test_update_model_metadata_field__project_admin__returns_expected_status(
+    staff_user: FFAdminUser,
+    staff_client: APIClient,
+    organisation: Organisation,
+    project: Project,
+    project_b: Project,
+    feature_content_type: ContentType,
+    field_project_attr: str | None,
+    expected_status: int,
+    request: pytest.FixtureRequest,
+) -> None:
+    # Given - a project admin for `project`, and a MetadataModelField whose parent
+    # MetadataField belongs to one of: `project`, `project_b`, or the org (None)
+    UserProjectPermission.objects.create(user=staff_user, project=project, admin=True)
+    field_project = (
+        request.getfixturevalue(field_project_attr) if field_project_attr else None
+    )
+    field = MetadataField.objects.create(
+        name="model_field", type="int", organisation=organisation, project=field_project
+    )
+    model_field = MetadataModelField.objects.create(
+        field=field, content_type=feature_content_type
+    )
+    url = reverse(
+        "api-v1:organisations:metadata-model-fields-detail",
+        args=[organisation.id, model_field.id],
+    )
+    data = {
+        "field": field.id,
+        "content_type": feature_content_type.id,
+        "is_required_for": [],
+    }
+
+    # When
+    response = staff_client.put(
+        url, data=json.dumps(data), content_type="application/json"
+    )
+
+    # Then
+    assert response.status_code == expected_status
+
+
+@pytest.mark.parametrize(
+    "field_project_attr, expected_status",
+    [
+        ("project", status.HTTP_204_NO_CONTENT),
+        (None, status.HTTP_403_FORBIDDEN),
+        ("project_b", status.HTTP_403_FORBIDDEN),
+    ],
+    ids=[
+        "own_project_field_allowed",
+        "org_scoped_field_denied",
+        "other_project_field_denied",
+    ],
+)
+def test_delete_model_metadata_field__project_admin__returns_expected_status(
+    staff_user: FFAdminUser,
+    staff_client: APIClient,
+    organisation: Organisation,
+    project: Project,
+    project_b: Project,
+    feature_content_type: ContentType,
+    field_project_attr: str | None,
+    expected_status: int,
+    request: pytest.FixtureRequest,
+) -> None:
+    # Given - a project admin for `project`, and a MetadataModelField whose parent
+    # MetadataField belongs to one of: `project`, `project_b`, or the org (None)
+    UserProjectPermission.objects.create(user=staff_user, project=project, admin=True)
+    field_project = (
+        request.getfixturevalue(field_project_attr) if field_project_attr else None
+    )
+    field = MetadataField.objects.create(
+        name="model_field", type="int", organisation=organisation, project=field_project
+    )
+    model_field = MetadataModelField.objects.create(
+        field=field, content_type=feature_content_type
+    )
+    url = reverse(
+        "api-v1:organisations:metadata-model-fields-detail",
+        args=[organisation.id, model_field.id],
+    )
+
+    # When
+    response = staff_client.delete(url)
+
+    # Then
+    assert response.status_code == expected_status