fix(project/serializer): limit edit to only fields that make sense (#…

…4846)

api/projects/serializers.py

@@ -44,6 +44,10 @@ class Meta:
             "stale_flags_limit_days",
             "edge_v2_migration_status",
         )
+        read_only_fields = (
+            "enable_dynamo_db",
+            "edge_v2_migration_status",
+        )
 
     def get_migration_status(self, obj: Project) -> str:
         if not settings.PROJECT_METADATA_TABLE_NAME_DYNAMO:
@@ -65,9 +69,7 @@ def get_use_edge_identities(self, obj: Project) -> bool:
         )
 
 
-class ProjectUpdateOrCreateSerializer(
-    ReadOnlyIfNotValidPlanMixin, ProjectListSerializer
-):
+class ProjectCreateSerializer(ReadOnlyIfNotValidPlanMixin, ProjectListSerializer):
     invalid_plans_regex = r"^(free|startup.*|scale-up.*)$"
     field_names = ("stale_flags_limit_days", "enable_realtime_updates")
 
@@ -84,11 +86,21 @@ def get_subscription(self) -> typing.Optional[Subscription]:
             # Organisation should only have a single subscription
             return Subscription.objects.filter(organisation_id=organisation_id).first()
         elif view.action in ("update", "partial_update"):
+            # handle instance not being set
+            # When request comes from yasg2 (as part of schema generation)
+            if not self.instance:
+                return None
             return getattr(self.instance.organisation, "subscription", None)
-
         return None
 
 
+class ProjectUpdateSerializer(ProjectCreateSerializer):
+    class Meta(ProjectCreateSerializer.Meta):
+        read_only_fields = ProjectCreateSerializer.Meta.read_only_fields + (
+            "organisation",
+        )
+
+
 class ProjectRetrieveSerializer(ProjectListSerializer):
     total_features = serializers.SerializerMethodField()
     total_segments = serializers.SerializerMethodField()

api/projects/views.py

@@ -44,9 +44,10 @@
     CreateUpdateUserProjectPermissionSerializer,
     ListUserPermissionGroupProjectPermissionSerializer,
     ListUserProjectPermissionSerializer,
+    ProjectCreateSerializer,
     ProjectListSerializer,
     ProjectRetrieveSerializer,
-    ProjectUpdateOrCreateSerializer,
+    ProjectUpdateSerializer,
 )
 
 
@@ -75,11 +76,13 @@ class ProjectViewSet(viewsets.ModelViewSet):
     permission_classes = [ProjectPermissions]
 
     def get_serializer_class(self):
-        if self.action == "retrieve":
-            return ProjectRetrieveSerializer
-        elif self.action in ("create", "update", "partial_update"):
-            return ProjectUpdateOrCreateSerializer
-        return ProjectListSerializer
+        serializers = {
+            "retrieve": ProjectRetrieveSerializer,
+            "create": ProjectCreateSerializer,
+            "update": ProjectUpdateSerializer,
+            "partial_update": ProjectUpdateSerializer,
+        }
+        return serializers.get(self.action, ProjectListSerializer)
 
     pagination_class = None
 

api/tests/integration/conftest.py

@@ -56,11 +56,13 @@ def organisation_with_persist_trait_data_disabled(organisation):
 
 
 @pytest.fixture()
-def dynamo_enabled_project(admin_client, organisation):
+def dynamo_enabled_project(
+    admin_client: APIClient, organisation: Organisation, settings: SettingsWrapper
+):
+    settings.EDGE_ENABLED = True
     project_data = {
         "name": "Test Project",
         "organisation": organisation,
-        "enable_dynamo_db": True,
     }
     url = reverse("api-v1:projects:project-list")
     response = admin_client.post(url, data=project_data)

api/tests/unit/projects/test_unit_projects_views.py

@@ -139,6 +139,30 @@ def test_can_update_project(
     assert response.json()["stale_flags_limit_days"] == new_stale_flags_limit_days
 
 
+def test_can_not_update_project_organisation(
+    admin_client: APIClient,
+    project: Project,
+    organisation: Organisation,
+    organisation_two: Organisation,
+) -> None:
+    # Given
+    new_name = "New project name"
+
+    data = {
+        "name": new_name,
+        "organisation": organisation_two.id,
+    }
+    url = reverse("api-v1:projects:project-detail", args=[project.id])
+
+    # When
+    response = admin_client.put(url, data=data)
+
+    # Then
+    assert response.status_code == status.HTTP_200_OK
+    assert response.json()["name"] == new_name
+    assert response.json()["organisation"] == organisation.id
+
+
 @pytest.mark.parametrize(
     "client",
     [(lazy_fixture("admin_master_api_key_client")), (lazy_fixture("admin_client"))],
@@ -733,6 +757,9 @@ def test_update_project(client, project, mocker, settings, organisation):
         "organisation": organisation.id,
         "only_allow_lower_case_feature_names": False,
         "feature_name_regex": feature_name_regex,
+        # read only fields should not be updated
+        "enable_dynamo_db": not project.enable_dynamo_db,
+        "edge_v2_migration_status": project.edge_v2_migration_status + "random-string",
     }
 
     # When
@@ -742,6 +769,10 @@ def test_update_project(client, project, mocker, settings, organisation):
     assert response.status_code == status.HTTP_200_OK
     assert response.json()["only_allow_lower_case_feature_names"] is False
     assert response.json()["feature_name_regex"] == feature_name_regex
+    assert response.json()["enable_dynamo_db"] == project.enable_dynamo_db
+    assert (
+        response.json()["edge_v2_migration_status"] == project.edge_v2_migration_status
+    )
 
 
 @pytest.mark.parametrize(