From 3c3b1d7af2e4b62f75979b8c8c2ea931f8736cbd Mon Sep 17 00:00:00 2001 From: Novak Zaballa <41410593+novakzaballa@users.noreply.github.com> Date: Tue, 12 Sep 2023 16:43:57 -0400 Subject: [PATCH] fix: Feature id in mv-option request is undefined (#2751) --- api/features/multivariate/views.py | 6 ++ .../test_integration_multivariate.py | 68 +++++++++++++++++++ frontend/common/stores/feature-list-store.js | 2 +- 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/api/features/multivariate/views.py b/api/features/multivariate/views.py index 80a2d8c06037..4f115d7634a6 100644 --- a/api/features/multivariate/views.py +++ b/api/features/multivariate/views.py @@ -33,6 +33,12 @@ def get_permissions(self): ) ] + def create(self, request, *args, **kwargs): + feature_pk = self.kwargs.get("feature_pk") + project_pk = self.kwargs.get("project_pk") + get_object_or_404(Feature.objects.filter(project__id=project_pk), pk=feature_pk) + return super().create(request, *args, **kwargs) + def get_queryset(self): if getattr(self, "swagger_fake_view", False): return MultivariateFeatureOption.objects.none() diff --git a/api/tests/integration/features/multivariate/test_integration_multivariate.py b/api/tests/integration/features/multivariate/test_integration_multivariate.py index f423d29cf000..4cca8e8abd2b 100644 --- a/api/tests/integration/features/multivariate/test_integration_multivariate.py +++ b/api/tests/integration/features/multivariate/test_integration_multivariate.py @@ -4,6 +4,12 @@ from django.urls import reverse from pytest_lazyfixture import lazy_fixture from rest_framework import status +from rest_framework.test import APIClient + +from features.models import Feature +from organisations.models import Organisation +from projects.models import Project +from users.models import FFAdminUser @pytest.mark.parametrize( @@ -34,6 +40,68 @@ def test_can_create_mv_option(client, project, mv_option_50_percent, feature): assert set(data.items()).issubset(set(response.json().items())) +@pytest.mark.parametrize( + "client, feature_id", + [ + (lazy_fixture("admin_client"), "undefined"), + (lazy_fixture("admin_client"), "89809"), + ], +) +def test_cannot_create_mv_option_when_feature_id_invalid(client, feature_id, project): + # Given + url = reverse( + "api-v1:projects:feature-mv-options-list", + args=[project, feature_id], + ) + + data = { + "type": "unicode", + "feature": feature_id, + "string_value": "bigger", + "default_percentage_allocation": 50, + } + # When + response = client.post( + url, + data=json.dumps(data), + content_type="application/json", + ) + # Then + assert response.status_code == status.HTTP_404_NOT_FOUND + + +def test_cannot_create_mv_option_when_user_is_not_owner_of_the_feature(project): + # Given + new_user = FFAdminUser.objects.create(email="testuser@mail.com") + organisation = Organisation.objects.create(name="Test Org") + new_project = Project.objects.create(name="Test project", organisation=organisation) + feature = Feature.objects.create( + name="New_feature", + project=new_project, + ) + url = reverse( + "api-v1:projects:feature-mv-options-list", + args=[project, feature.id], + ) + + data = { + "type": "unicode", + "feature": feature.id, + "string_value": "bigger", + "default_percentage_allocation": 50, + } + client = APIClient() + client.force_authenticate(user=new_user) + # When + response = client.post( + url, + data=json.dumps(data), + content_type="application/json", + ) + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN + + @pytest.mark.parametrize( "client", [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")], diff --git a/frontend/common/stores/feature-list-store.js b/frontend/common/stores/feature-list-store.js index 079fe33724a8..9c0acbd4d615 100644 --- a/frontend/common/stores/feature-list-store.js +++ b/frontend/common/stores/feature-list-store.js @@ -66,7 +66,7 @@ const controller = { (flag.multivariate_options || []).map((v) => data .post( - `${Project.api}projects/${projectId}/features/${flag.id}/mv-options/`, + `${Project.api}projects/${projectId}/features/${res.id}/mv-options/`, { ...v, feature: res.id,