diff --git a/api/projects/serializers.py b/api/projects/serializers.py index 14f8af3dfa84..7a934861d7c8 100644 --- a/api/projects/serializers.py +++ b/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() diff --git a/api/projects/views.py b/api/projects/views.py index 42929321015e..27c720f6a138 100644 --- a/api/projects/views.py +++ b/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 diff --git a/api/tests/integration/conftest.py b/api/tests/integration/conftest.py index 7afc966ccdb8..0a0cf6495c04 100644 --- a/api/tests/integration/conftest.py +++ b/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) diff --git a/api/tests/unit/projects/test_unit_projects_views.py b/api/tests/unit/projects/test_unit_projects_views.py index 7f949f3c6fc5..ed1d349e5a50 100644 --- a/api/tests/unit/projects/test_unit_projects_views.py +++ b/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(