diff --git a/api/conftest.py b/api/conftest.py index 13f72607cf2b..0d0b8efa9109 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -371,6 +371,11 @@ def project(organisation): # type: ignore[no-untyped-def] return Project.objects.create(name="Test Project", organisation=organisation) +@pytest.fixture() +def project_b(organisation: Organisation) -> Project: + return Project.objects.create(name="Test Project B", organisation=organisation) # type: ignore[no-any-return] + + @pytest.fixture() def segment(project: Project) -> Segment: segment: Segment = Segment.objects.create(name="segment", project=project) diff --git a/api/environments/serializers.py b/api/environments/serializers.py index 408f3c575c02..c6a3ae8724b8 100644 --- a/api/environments/serializers.py +++ b/api/environments/serializers.py @@ -88,7 +88,9 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: attrs = super().validate(attrs) project = self.instance.project if self.instance else attrs["project"] # type: ignore[union-attr] organisation = project.organisation - self._validate_required_metadata(organisation, attrs.get("metadata", [])) + self._validate_required_metadata( + organisation, attrs.get("metadata", []), project=project + ) return attrs def create(self, validated_data: dict[str, Any]) -> Environment: diff --git a/api/features/serializers.py b/api/features/serializers.py index 134181afe3a0..2607560e7ac5 100644 --- a/api/features/serializers.py +++ b/api/features/serializers.py @@ -359,7 +359,9 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: attrs = super().validate(attrs) project = self.instance.project if self.instance else self.context["project"] # type: ignore[union-attr] organisation = project.organisation - self._validate_required_metadata(organisation, attrs.get("metadata", [])) + self._validate_required_metadata( + organisation, attrs.get("metadata", []), project=project + ) return attrs def create(self, validated_data: dict[str, Any]) -> Feature: diff --git a/api/metadata/migrations/0002_add_project_to_metadata_field.py b/api/metadata/migrations/0002_add_project_to_metadata_field.py new file mode 100644 index 000000000000..d14651304655 --- /dev/null +++ b/api/metadata/migrations/0002_add_project_to_metadata_field.py @@ -0,0 +1,46 @@ +# Generated by Django 5.2.11 on 2026-02-16 10:22 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("metadata", "0001_initial"), + ("organisations", "0058_update_audit_and_history_limits_in_sub_cache"), + ("projects", "0027_add_create_project_level_change_requests_permission"), + ] + + operations = [ + migrations.AlterUniqueTogether( + name="metadatafield", + unique_together=set(), + ), + migrations.AddField( + model_name="metadatafield", + name="project", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="projects.project", + ), + ), + migrations.AddConstraint( + model_name="metadatafield", + constraint=models.UniqueConstraint( + condition=models.Q(("project__isnull", True)), + fields=("name", "organisation"), + name="unique_org_level_metadata_field", + ), + ), + migrations.AddConstraint( + model_name="metadatafield", + constraint=models.UniqueConstraint( + condition=models.Q(("project__isnull", False)), + fields=("name", "organisation", "project"), + name="unique_project_level_metadata_field", + ), + ), + ] diff --git a/api/metadata/models.py b/api/metadata/models.py index 4e134731efae..9f69de8e70bb 100644 --- a/api/metadata/models.py +++ b/api/metadata/models.py @@ -36,6 +36,9 @@ class MetadataField(AbstractBaseExportableModel): ) description = models.TextField(blank=True, null=True) organisation = models.ForeignKey(Organisation, on_delete=models.CASCADE) + project = models.ForeignKey( + "projects.Project", on_delete=models.CASCADE, null=True, blank=True + ) def is_field_value_valid(self, field_value: str) -> bool: if len(field_value) > FIELD_VALUE_MAX_LENGTH: @@ -68,7 +71,18 @@ def validate_multiline_str(self, field_value: str): # type: ignore[no-untyped-d return True class Meta: - unique_together = ("name", "organisation") + constraints = [ + models.UniqueConstraint( + fields=["name", "organisation"], + condition=models.Q(project__isnull=True), + name="unique_org_level_metadata_field", + ), + models.UniqueConstraint( + fields=["name", "organisation", "project"], + condition=models.Q(project__isnull=False), + name="unique_project_level_metadata_field", + ), + ] class MetadataModelField(AbstractBaseExportableModel): diff --git a/api/metadata/permissions.py b/api/metadata/permissions.py index c287a8ab9165..65cd54a57881 100644 --- a/api/metadata/permissions.py +++ b/api/metadata/permissions.py @@ -4,6 +4,7 @@ from metadata.models import MetadataField from organisations.models import Organisation +from projects.models import Project class MetadataFieldPermissions(IsAuthenticated): @@ -19,7 +20,16 @@ def has_permission(self, request, view): # type: ignore[no-untyped-def] with suppress(Organisation.DoesNotExist): organisation_id = request.data.get("organisation") organisation = Organisation.objects.get(id=organisation_id) - return request.user.is_organisation_admin(organisation) + + if request.user.is_organisation_admin(organisation): + return True + + project_id = request.data.get("project") + if project_id is not None: + with suppress(Project.DoesNotExist): + project = Project.objects.get(id=project_id) + if project.organisation_id == organisation.id: + return request.user.is_project_admin(project) return False @@ -32,7 +42,11 @@ def has_object_permission(self, request, view, obj): # type: ignore[no-untyped- "destroy", "partial_update", ): - return request.user.is_organisation_admin(obj.organisation) + if request.user.is_organisation_admin(obj.organisation): + return True + + if obj.project is not None: + return request.user.is_project_admin(obj.project) return False diff --git a/api/metadata/serializers.py b/api/metadata/serializers.py index 8cfca2adb5b4..5e53b957be88 100644 --- a/api/metadata/serializers.py +++ b/api/metadata/serializers.py @@ -3,7 +3,7 @@ from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ObjectDoesNotExist from django.db import transaction -from django.db.models import Model +from django.db.models import Model, Q from rest_framework import serializers from metadata.models import ( @@ -13,6 +13,7 @@ MetadataModelFieldRequirement, ) from organisations.models import Organisation +from projects.models import Project from util.drf_writable_nested.serializers import ( DeleteBeforeUpdateWritableNestedModelSerializer, ) @@ -24,14 +25,22 @@ class MetadataFieldQuerySerializer(serializers.Serializer): # type: ignore[type ) -class SupportedRequiredForModelQuerySerializer(serializers.Serializer): # type: ignore[type-arg] - model_name = serializers.CharField(required=True) +class ProjectMetadataFieldQuerySerializer(serializers.Serializer): # type: ignore[type-arg] + include_organisation = serializers.BooleanField( + required=False, + default=False, + help_text="Include inherited organisation-level fields. " + "Project-level fields override same-named org fields.", + ) + entity = serializers.ChoiceField( + required=False, + choices=["feature", "segment", "environment"], + help_text="Filter by entity type (feature, segment, or environment).", + ) -class MetadataFieldSerializer(serializers.ModelSerializer): # type: ignore[type-arg] - class Meta: - model = MetadataField - fields = ("id", "name", "type", "description", "organisation") +class SupportedRequiredForModelQuerySerializer(serializers.Serializer): # type: ignore[type-arg] + model_name = serializers.CharField(required=True) class MetadataModelFieldQuerySerializer(serializers.Serializer): # type: ignore[type-arg] @@ -46,6 +55,70 @@ class Meta: fields = ("content_type", "object_id") +class MetadataModelFieldNestedSerializer(serializers.ModelSerializer): # type: ignore[type-arg] + is_required_for = MetadataModelFieldRequirementSerializer(many=True, read_only=True) + + class Meta: + model = MetadataModelField + fields = ("id", "content_type", "is_required_for") + + +class MetadataFieldSerializer(serializers.ModelSerializer): # type: ignore[type-arg] + project = serializers.IntegerField( + required=False, allow_null=True, default=None, source="project_id" + ) + model_fields = MetadataModelFieldNestedSerializer( + source="metadatamodelfield_set", many=True, read_only=True + ) + + class Meta: + model = MetadataField + fields = ( + "id", + "name", + "type", + "description", + "organisation", + "project", + "model_fields", + ) + # Disable auto-generated unique validators — conditional + # UniqueConstraints are enforced at the database level. + validators: list[object] = [] + + def validate(self, data: dict[str, Any]) -> dict[str, Any]: + data = super().validate(data) + project_id = data.get("project_id") + organisation = data.get("organisation") + + if ( + project_id is not None + and not Project.objects.filter( + id=project_id, organisation=organisation + ).exists() + ): + raise serializers.ValidationError( + {"project": "Project must belong to the specified organisation."} + ) + + # Replicate uniqueness checks that DRF can't auto-generate + # from conditional UniqueConstraints. + qs = MetadataField.objects.filter( + name=data.get("name"), + organisation=organisation, + project_id=project_id, + ) + if self.instance is not None: + assert isinstance(self.instance, MetadataField) + qs = qs.exclude(pk=self.instance.pk) + if qs.exists(): + raise serializers.ValidationError( + {"name": "A metadata field with this name already exists."} + ) + + return data + + class MetaDataModelFieldSerializer(DeleteBeforeUpdateWritableNestedModelSerializer): is_required_for = MetadataModelFieldRequirementSerializer(many=True, required=False) @@ -109,20 +182,50 @@ class MetadataSerializerMixin: """ def _validate_required_metadata( - self, organisation: Organisation, metadata: list[dict[str, Any]] + self, + organisation: Organisation, + metadata: list[dict[str, Any]], + project: Project | None = None, ) -> None: content_type = ContentType.objects.get_for_model(self.Meta.model) # type: ignore[attr-defined] - requirements = MetadataModelFieldRequirement.objects.filter( + org_ct = ContentType.objects.get_for_model(Organisation) + + # Field scoping: org-level fields + this project's fields + field_scope = Q( model_field__content_type=content_type, model_field__field__organisation=organisation, + model_field__field__project__isnull=True, + ) + # Requirement scoping: org-level + this project's requirements + req_scope = Q(content_type=org_ct, object_id=organisation.id) + + overridden_names: set[str] = set() + if project is not None: + field_scope |= Q( + model_field__content_type=content_type, + model_field__field__organisation=organisation, + model_field__field__project=project, + ) + project_ct = ContentType.objects.get_for_model(Project) + req_scope |= Q(content_type=project_ct, object_id=project.id) + overridden_names = set( + MetadataField.objects.filter( + organisation=organisation, project=project + ).values_list("name", flat=True) + ) + + requirements = MetadataModelFieldRequirement.objects.filter( + field_scope & req_scope, ).select_related("model_field__field") metadata_fields = {field["model_field"] for field in metadata} for requirement in requirements: + field = requirement.model_field.field + if field.project is None and field.name in overridden_names: + continue if requirement.model_field not in metadata_fields: - field_name = requirement.model_field.field.name raise serializers.ValidationError( - {"metadata": f"Missing required metadata field: {field_name}"} + {"metadata": f"Missing required metadata field: {field.name}"} ) def _update_metadata( diff --git a/api/metadata/views.py b/api/metadata/views.py index 4600989f90f8..afa32052ddea 100644 --- a/api/metadata/views.py +++ b/api/metadata/views.py @@ -1,13 +1,17 @@ from itertools import chain +from common.projects.permissions import VIEW_PROJECT from django.contrib.contenttypes.models import ContentType +from django.shortcuts import get_object_or_404 from django.utils.decorators import method_decorator from drf_spectacular.utils import extend_schema from rest_framework import status, viewsets from rest_framework.decorators import action -from rest_framework.exceptions import ValidationError from rest_framework.response import Response +from app.pagination import CustomPagination +from projects.permissions import NestedProjectPermissions + from .models import ( SUPPORTED_REQUIREMENTS_MAPPING, MetadataField, @@ -23,6 +27,7 @@ MetadataFieldSerializer, MetadataModelFieldQuerySerializer, MetaDataModelFieldSerializer, + ProjectMetadataFieldQuerySerializer, SupportedRequiredForModelQuerySerializer, ) @@ -34,6 +39,7 @@ class MetadataFieldViewSet(viewsets.ModelViewSet): # type: ignore[type-arg] permission_classes = [MetadataFieldPermissions] serializer_class = MetadataFieldSerializer + pagination_class = CustomPagination def get_queryset(self): # type: ignore[no-untyped-def] if getattr(self, "swagger_fake_view", False): @@ -45,11 +51,12 @@ def get_queryset(self): # type: ignore[no-untyped-def] serializer.is_valid(raise_exception=True) organisation_id = serializer.validated_data["organisation"] - if organisation_id is None: - raise ValidationError("organisation parameter is required") - queryset = queryset.filter(organisation__id=organisation_id) + queryset = queryset.filter( + organisation_id=organisation_id, + project__isnull=True, + ) - return queryset + return queryset.prefetch_related("metadatamodelfield_set__is_required_for") class MetaDataModelFieldViewSet(viewsets.ModelViewSet): # type: ignore[type-arg] @@ -113,3 +120,47 @@ def supported_required_for_models(self, request, organisation_pk=None): # type: serializer = ContentTypeSerializer(qs, many=True) # type: ignore[assignment] return Response(serializer.data) + + +@method_decorator( + name="list", + decorator=extend_schema(parameters=[ProjectMetadataFieldQuerySerializer]), +) +class ProjectMetadataFieldViewSet(viewsets.ReadOnlyModelViewSet): # type: ignore[type-arg] + serializer_class = MetadataFieldSerializer + permission_classes = [NestedProjectPermissions] + pagination_class = CustomPagination + + def get_queryset(self): # type: ignore[no-untyped-def] + if getattr(self, "swagger_fake_view", False): + return MetadataField.objects.none() + + project = get_object_or_404( + self.request.user.get_permitted_projects(VIEW_PROJECT), # type: ignore[union-attr] + pk=self.kwargs["project_pk"], + ) + + queryset = MetadataField.objects.filter( + organisation=project.organisation, + project_id=project.id, + ) + + serializer = ProjectMetadataFieldQuerySerializer(data=self.request.query_params) + serializer.is_valid(raise_exception=True) + + if serializer.validated_data.get("include_organisation"): + overridden_names = queryset.values_list("name", flat=True) + org_fields = MetadataField.objects.filter( + organisation=project.organisation, + project__isnull=True, + ).exclude(name__in=overridden_names) + queryset = queryset | org_fields + + entity = serializer.validated_data.get("entity") + if entity: + content_type = get_object_or_404(ContentType, model=entity) + queryset = queryset.filter( + metadatamodelfield__content_type=content_type, + ) + + return queryset.prefetch_related("metadatamodelfield_set__is_required_for") diff --git a/api/projects/urls.py b/api/projects/urls.py index 01f6e70ad3ed..e65b86ffb19f 100644 --- a/api/projects/urls.py +++ b/api/projects/urls.py @@ -22,6 +22,7 @@ from integrations.grafana.views import GrafanaProjectConfigurationViewSet from integrations.launch_darkly.views import LaunchDarklyImportRequestViewSet from integrations.new_relic.views import NewRelicConfigurationViewSet +from metadata.views import ProjectMetadataFieldViewSet from projects.tags.views import TagViewSet from segments.views import SegmentViewSet @@ -84,6 +85,11 @@ FeatureHealthEventViewSet, basename="feature-health-events", ) +projects_router.register( + r"metadata/fields", + ProjectMetadataFieldViewSet, + basename="project-metadata-fields", +) if settings.WORKFLOWS_LOGIC_INSTALLED: # pragma: no cover workflow_views = importlib.import_module("workflows_logic.views") diff --git a/api/segments/serializers.py b/api/segments/serializers.py index 4cbc203894f4..2bf44a4778e1 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -118,7 +118,9 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: project = self.instance.project if self.instance else attrs["project"] # type: ignore[union-attr] organisation = project.organisation - self._validate_required_metadata(organisation, attrs.get("metadata", [])) + self._validate_required_metadata( + organisation, attrs.get("metadata", []), project=project + ) self._validate_segment_rules_conditions_limit(attrs["rules"]) self._validate_project_segment_limit(project) return attrs diff --git a/api/tests/unit/environments/test_unit_environments_views.py b/api/tests/unit/environments/test_unit_environments_views.py index 16537fa6efcf..88e5efd86b33 100644 --- a/api/tests/unit/environments/test_unit_environments_views.py +++ b/api/tests/unit/environments/test_unit_environments_views.py @@ -24,7 +24,12 @@ from environments.permissions.models import UserEnvironmentPermission from features.models import Feature, FeatureState from features.versioning.models import EnvironmentFeatureVersion -from metadata.models import Metadata, MetadataModelField +from metadata.models import ( + Metadata, + MetadataField, + MetadataModelField, + MetadataModelFieldRequirement, +) from organisations.models import Organisation from permissions.models import PermissionModel from projects.models import Project, UserProjectPermission @@ -1325,3 +1330,32 @@ def test_total_segment_overrides_correctly_ignores_old_versions( # Then assert response.json()["total_segment_overrides"] == 1 + + +def test_create_environment__required_metadata_on_other_project__returns_201( + admin_client: APIClient, + project: Project, + project_b: Project, + organisation: Organisation, + a_metadata_field: MetadataField, + environment_content_type: ContentType, + project_content_type: ContentType, +) -> None: + # Given + model_field = MetadataModelField.objects.create( + field=a_metadata_field, + content_type=environment_content_type, + ) + MetadataModelFieldRequirement.objects.create( + content_type=project_content_type, + object_id=project_b.id, + model_field=model_field, + ) + url = reverse("api-v1:environments:environment-list") + data = {"name": "New env", "project": project.id} + + # When + response = admin_client.post(url, data=data) + + # Then + assert response.status_code == status.HTTP_201_CREATED diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index 05abbecf7562..7097dcab33a0 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -16,6 +16,7 @@ VIEW_PROJECT, ) from django.conf import settings +from django.contrib.contenttypes.models import ContentType from django.forms import model_to_dict from django.urls import reverse from django.utils import timezone @@ -49,7 +50,11 @@ from features.multivariate.models import MultivariateFeatureOption from features.value_types import STRING from features.versioning.models import EnvironmentFeatureVersion -from metadata.models import MetadataModelField +from metadata.models import ( + MetadataField, + MetadataModelField, + MetadataModelFieldRequirement, +) from organisations.models import Organisation, OrganisationRole from permissions.models import PermissionModel from projects.code_references.models import FeatureFlagCodeReferencesScan @@ -4240,3 +4245,34 @@ def test_create_multiple_features_with_metadata_keeps_metadata_isolated( second_feature_metadata_after = second_feature_check.json()["metadata"] assert len(second_feature_metadata_after) == 1 assert second_feature_metadata_after[0]["field_value"] == "200" + + +def test_create_feature__required_metadata_on_other_project__returns_201( + admin_client: APIClient, + project: Project, + project_b: Project, + organisation: Organisation, + a_metadata_field: MetadataField, + feature_content_type: ContentType, + project_content_type: ContentType, +) -> None: + # Given + model_field = MetadataModelField.objects.create( + field=a_metadata_field, + content_type=feature_content_type, + ) + MetadataModelFieldRequirement.objects.create( + content_type=project_content_type, + object_id=project_b.id, + model_field=model_field, + ) + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + data = {"name": "Test feature cross project", "description": "desc"} + + # When + response = admin_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED diff --git a/api/tests/unit/metadata/test_serializers.py b/api/tests/unit/metadata/test_serializers.py index 8e2e5059d24c..085fc9a2f13d 100644 --- a/api/tests/unit/metadata/test_serializers.py +++ b/api/tests/unit/metadata/test_serializers.py @@ -2,15 +2,22 @@ import pytest from django.contrib.contenttypes.models import ContentType +from rest_framework import serializers from metadata.models import ( FIELD_VALUE_MAX_LENGTH, MetadataField, MetadataModelField, + MetadataModelFieldRequirement, +) +from metadata.serializers import ( + MetaDataModelFieldSerializer, + MetadataSerializer, + MetadataSerializerMixin, ) -from metadata.serializers import MetaDataModelFieldSerializer, MetadataSerializer from organisations.models import Organisation from projects.models import Project +from segments.models import Segment @pytest.mark.parametrize( @@ -168,3 +175,145 @@ def test_metadata_model_field_serializer_validation_invalid_content_type( serializer.errors["non_field_errors"][0] == "The requirement content type must be project or organisation" ) + + +class _DummySegmentSerializer(MetadataSerializerMixin, serializers.Serializer): # type: ignore[type-arg] + """A minimal serializer for testing _validate_required_metadata with Segment model.""" + + class Meta: + model = Segment + + +def test_validate_required_metadata__requirement_on_project_a__not_enforced_in_project_b( + organisation: Organisation, + project: Project, + project_b: Project, + a_metadata_field: MetadataField, + segment_content_type: ContentType, + project_content_type: ContentType, +) -> None: + # Given + model_field = MetadataModelField.objects.create( + field=a_metadata_field, + content_type=segment_content_type, + ) + MetadataModelFieldRequirement.objects.create( + content_type=project_content_type, + object_id=project.id, + model_field=model_field, + ) + serializer = _DummySegmentSerializer() + + # When / Then + serializer._validate_required_metadata(organisation, [], project=project_b) + + +def test_validate_required_metadata__requirement_on_project_a__enforced_in_project_a( + organisation: Organisation, + project: Project, + a_metadata_field: MetadataField, + segment_content_type: ContentType, + project_content_type: ContentType, +) -> None: + # Given + model_field = MetadataModelField.objects.create( + field=a_metadata_field, + content_type=segment_content_type, + ) + MetadataModelFieldRequirement.objects.create( + content_type=project_content_type, + object_id=project.id, + model_field=model_field, + ) + serializer = _DummySegmentSerializer() + + # When / Then + with pytest.raises(serializers.ValidationError, match="Missing required metadata"): + serializer._validate_required_metadata(organisation, [], project=project) + + +def test_validate_required_metadata__org_level_requirement__enforced_in_all_projects( + organisation: Organisation, + project: Project, + project_b: Project, + a_metadata_field: MetadataField, + segment_content_type: ContentType, + organisation_content_type: ContentType, +) -> None: + # Given + model_field = MetadataModelField.objects.create( + field=a_metadata_field, + content_type=segment_content_type, + ) + MetadataModelFieldRequirement.objects.create( + content_type=organisation_content_type, + object_id=organisation.id, + model_field=model_field, + ) + serializer = _DummySegmentSerializer() + + # When / Then + with pytest.raises(serializers.ValidationError, match="Missing required metadata"): + serializer._validate_required_metadata(organisation, [], project=project) + + with pytest.raises(serializers.ValidationError, match="Missing required metadata"): + serializer._validate_required_metadata(organisation, [], project=project_b) + + +def test_validate_required_metadata__project_level_field_with_requirement__only_enforced_in_its_project( + organisation: Organisation, + project: Project, + project_b: Project, + segment_content_type: ContentType, + project_content_type: ContentType, +) -> None: + # Given + project_field = MetadataField.objects.create( + name="proj_field", type="str", organisation=organisation, project=project + ) + model_field = MetadataModelField.objects.create( + field=project_field, + content_type=segment_content_type, + ) + MetadataModelFieldRequirement.objects.create( + content_type=project_content_type, + object_id=project.id, + model_field=model_field, + ) + serializer = _DummySegmentSerializer() + + # When / Then + with pytest.raises(serializers.ValidationError, match="Missing required metadata"): + serializer._validate_required_metadata(organisation, [], project=project) + + serializer._validate_required_metadata(organisation, [], project=project_b) + + +@pytest.mark.django_db +def test_validate_required_metadata__project_field_overrides_required_org_field__org_requirement_skipped( + organisation: Organisation, + project: Project, + a_metadata_field: MetadataField, + segment_content_type: ContentType, + organisation_content_type: ContentType, +) -> None: + # Given + model_field = MetadataModelField.objects.create( + field=a_metadata_field, + content_type=segment_content_type, + ) + MetadataModelFieldRequirement.objects.create( + content_type=organisation_content_type, + object_id=organisation.id, + model_field=model_field, + ) + MetadataField.objects.create( + name=a_metadata_field.name, + type=a_metadata_field.type, + organisation=organisation, + project=project, + ) + serializer = _DummySegmentSerializer() + + # When / Then + serializer._validate_required_metadata(organisation, [], project=project) diff --git a/api/tests/unit/metadata/test_views.py b/api/tests/unit/metadata/test_views.py index b9fde12eb8e3..172cf8953949 100644 --- a/api/tests/unit/metadata/test_views.py +++ b/api/tests/unit/metadata/test_views.py @@ -1,6 +1,7 @@ import json from itertools import chain +import pytest from django.contrib.contenttypes.models import ContentType from django.urls import reverse from rest_framework import status @@ -14,6 +15,7 @@ from metadata.views import SUPPORTED_REQUIREMENTS_MAPPING # type: ignore[attr-defined] from organisations.models import Organisation from projects.models import Project +from users.models import FFAdminUser def test_can_create_metadata_field(admin_client, organisation): # type: ignore[no-untyped-def] @@ -439,3 +441,520 @@ def test_get_supported_required_for_models(admin_client, organisation): # type: assert len(response.json()) == 2 assert response.json()[0]["model"] == "organisation" assert response.json()[1]["model"] == "project" + + +def test_create_metadata_field__with_project__returns_201( + admin_client: APIClient, + organisation: Organisation, + project: Project, +) -> None: + # Given + url = reverse("api-v1:metadata:metadata-fields-list") + data = { + "name": "project_field", + "type": "str", + "organisation": organisation.id, + "project": project.id, + } + + # When + response = admin_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + assert response.json()["project"] == project.id + assert response.json()["organisation"] == organisation.id + + +def test_create_metadata_field__project_from_different_org__returns_400( + admin_client: APIClient, + organisation: Organisation, + project: Project, +) -> None: + # Given + other_org = Organisation.objects.create(name="Other Org") + other_project = Project.objects.create(name="Other Project", organisation=other_org) + url = reverse("api-v1:metadata:metadata-fields-list") + data = { + "name": "bad_field", + "type": "str", + "organisation": organisation.id, + "project": other_project.id, + } + + # When + response = admin_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +def test_create_metadata_field__project_admin__returns_201( + staff_user: FFAdminUser, + staff_client: APIClient, + organisation: Organisation, + 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") + data = { + "name": "project_admin_field", + "type": "str", + "organisation": organisation.id, + "project": project.id, + } + + # When + response = staff_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + + +def test_list_metadata_fields__returns_org_level_only( + admin_client: APIClient, + organisation: Organisation, + project: Project, + project_b: Project, +) -> None: + # Given + org_field = MetadataField.objects.create( + name="org_field", type="str", organisation=organisation + ) + MetadataField.objects.create( + name="proj_a_field", type="str", organisation=organisation, project=project + ) + MetadataField.objects.create( + name="proj_b_field", type="str", organisation=organisation, project=project_b + ) + base_url = reverse("api-v1:metadata:metadata-fields-list") + url = f"{base_url}?organisation={organisation.id}" + + # When + response = admin_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + returned_ids = {r["id"] for r in response.json()["results"]} + assert returned_ids == {org_field.id} + + +def test_list_metadata_fields__response_includes_nested_model_fields( + admin_client: APIClient, + organisation: Organisation, + environment_content_type: ContentType, + project_content_type: ContentType, + project: Project, +) -> None: + # Given + field = MetadataField.objects.create( + name="my_field", type="str", organisation=organisation + ) + model_field = MetadataModelField.objects.create( + field=field, content_type=environment_content_type + ) + MetadataModelFieldRequirement.objects.create( + content_type=project_content_type, + object_id=project.id, + model_field=model_field, + ) + base_url = reverse("api-v1:metadata:metadata-fields-list") + url = f"{base_url}?organisation={organisation.id}" + + # When + response = admin_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + results = response.json()["results"] + assert len(results) == 1 + assert results[0]["model_fields"] == [ + { + "id": model_field.id, + "content_type": environment_content_type.id, + "is_required_for": [ + { + "content_type": project_content_type.id, + "object_id": project.id, + } + ], + } + ] + + +def test_list_project_metadata_fields__returns_project_fields_only( + admin_client: APIClient, + organisation: Organisation, + project: Project, + project_b: Project, +) -> None: + # Given + MetadataField.objects.create( + name="org_field", type="str", organisation=organisation + ) + project_field = MetadataField.objects.create( + name="proj_a_field", type="str", organisation=organisation, project=project + ) + MetadataField.objects.create( + name="proj_b_field", type="str", organisation=organisation, project=project_b + ) + url = reverse("api-v1:projects:project-metadata-fields-list", args=[project.id]) + + # When + response = admin_client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + returned_ids = {r["id"] for r in response.json()["results"]} + assert returned_ids == {project_field.id} + + +def test_list_project_metadata_fields__include_organisation__returns_both( + admin_client: APIClient, + organisation: Organisation, + project: Project, +) -> None: + # Given + org_field = MetadataField.objects.create( + name="org_field", type="str", organisation=organisation + ) + project_field = MetadataField.objects.create( + name="project_field", type="str", organisation=organisation, project=project + ) + url = reverse("api-v1:projects:project-metadata-fields-list", args=[project.id]) + + # When + response = admin_client.get(f"{url}?include_organisation=true") + + # Then + assert response.status_code == status.HTTP_200_OK + returned_ids = {r["id"] for r in response.json()["results"]} + assert org_field.id in returned_ids + assert project_field.id in returned_ids + + +def test_list_project_metadata_fields__include_organisation__project_overrides_org( + admin_client: APIClient, + organisation: Organisation, + project: Project, +) -> None: + # Given + org_field = MetadataField.objects.create( + name="shared_name", type="str", organisation=organisation + ) + project_field = MetadataField.objects.create( + name="shared_name", type="int", organisation=organisation, project=project + ) + url = reverse("api-v1:projects:project-metadata-fields-list", args=[project.id]) + + # When + response = admin_client.get(f"{url}?include_organisation=true") + + # Then + assert response.status_code == status.HTTP_200_OK + returned_ids = {r["id"] for r in response.json()["results"]} + assert project_field.id in returned_ids + assert org_field.id not in returned_ids + + +def test_list_project_metadata_fields__excludes_other_project_fields( + admin_client: APIClient, + organisation: Organisation, + project: Project, + project_b: Project, +) -> None: + # Given + project_a_field = MetadataField.objects.create( + name="proj_a_field", type="str", organisation=organisation, project=project + ) + MetadataField.objects.create( + name="proj_b_field", type="str", organisation=organisation, project=project_b + ) + url = reverse("api-v1:projects:project-metadata-fields-list", args=[project.id]) + + # When + response = admin_client.get(f"{url}?include_organisation=true") + + # Then + assert response.status_code == status.HTTP_200_OK + returned_ids = {r["id"] for r in response.json()["results"]} + assert project_a_field.id in returned_ids + assert all(r["project"] in (project.id, None) for r in response.json()["results"]) + + +@pytest.mark.parametrize( + "existing_project_attr, new_project_attr, expected_status", + [ + (None, None, status.HTTP_400_BAD_REQUEST), + ("project", "project", status.HTTP_400_BAD_REQUEST), + ("project", "project_b", status.HTTP_201_CREATED), + (None, "project", status.HTTP_201_CREATED), + ], + ids=[ + "duplicate_org_level", + "duplicate_project_level", + "same_name_different_projects", + "same_name_org_and_project_level", + ], +) +def test_create_metadata_field__uniqueness( + admin_client: APIClient, + organisation: Organisation, + project: Project, + project_b: Project, + existing_project_attr: str | None, + new_project_attr: str | None, + expected_status: int, + request: pytest.FixtureRequest, +) -> None: + # Given + existing_project = ( + request.getfixturevalue(existing_project_attr) + if existing_project_attr + else None + ) + MetadataField.objects.create( + name="the_field", + type="str", + organisation=organisation, + project=existing_project, + ) + + new_project = ( + request.getfixturevalue(new_project_attr) if new_project_attr else None + ) + url = reverse("api-v1:metadata:metadata-fields-list") + data: dict[str, object] = { + "name": "the_field", + "type": "str", + "organisation": organisation.id, + } + if new_project is not None: + data["project"] = new_project.id + + # When + response = admin_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == expected_status + + +def test_list_project_metadata_fields__page_size__returns_all( + admin_client: APIClient, + organisation: Organisation, + project: Project, + feature_content_type: ContentType, +) -> None: + # Given - create 15 fields to exceed the default page size of 10 + fields = [] + for i in range(15): + field = MetadataField.objects.create( + name=f"field_{i}", type="str", organisation=organisation, project=project + ) + MetadataModelField.objects.create( + field=field, content_type=feature_content_type + ) + fields.append(field) + + url = reverse("api-v1:projects:project-metadata-fields-list", args=[project.id]) + + # When + response = admin_client.get(f"{url}?page_size=100") + + # Then + assert response.status_code == status.HTTP_200_OK + assert len(response.json()["results"]) == 15 + + +@pytest.mark.parametrize( + "entity", + ["feature", "segment", "environment"], +) +def test_list_project_metadata_fields__entity_filter__returns_matching_only( + admin_client: APIClient, + organisation: Organisation, + project: Project, + feature_content_type: ContentType, + segment_content_type: ContentType, + environment_content_type: ContentType, + entity: str, +) -> None: + # Given + content_types = { + "feature": feature_content_type, + "segment": segment_content_type, + "environment": environment_content_type, + } + + created_fields: dict[str, MetadataField] = {} + for name, ct in content_types.items(): + field = MetadataField.objects.create( + name=f"{name}_only_field", + type="str", + organisation=organisation, + project=project, + ) + MetadataModelField.objects.create(field=field, content_type=ct) + created_fields[name] = field + + url = reverse("api-v1:projects:project-metadata-fields-list", args=[project.id]) + + # When + response = admin_client.get(f"{url}?entity={entity}") + + # Then + assert response.status_code == status.HTTP_200_OK + returned_ids = {r["id"] for r in response.json()["results"]} + assert returned_ids == {created_fields[entity].id} + + +def test_list_project_metadata_fields__entity_filter__includes_multi_entity_fields( + admin_client: APIClient, + organisation: Organisation, + project: Project, + feature_content_type: ContentType, + segment_content_type: ContentType, + environment_content_type: ContentType, +) -> None: + # Given - a field assigned to all three entities + all_entities_field = MetadataField.objects.create( + name="all_entities_field", + type="str", + organisation=organisation, + project=project, + ) + for ct in [feature_content_type, segment_content_type, environment_content_type]: + MetadataModelField.objects.create(field=all_entities_field, content_type=ct) + + # And a field assigned only to feature + feature_only_field = MetadataField.objects.create( + name="feature_only_field", + type="str", + organisation=organisation, + project=project, + ) + MetadataModelField.objects.create( + field=feature_only_field, content_type=feature_content_type + ) + + url = reverse("api-v1:projects:project-metadata-fields-list", args=[project.id]) + + # When - filtering by segment + response = admin_client.get(f"{url}?entity=segment") + + # Then - only the all_entities_field is returned + assert response.status_code == status.HTTP_200_OK + returned_ids = {r["id"] for r in response.json()["results"]} + assert returned_ids == {all_entities_field.id} + + +def test_list_project_metadata_fields__entity_filter_with_include_organisation__returns_both( + admin_client: APIClient, + organisation: Organisation, + project: Project, + feature_content_type: ContentType, + segment_content_type: ContentType, +) -> None: + # Given - an org-level field for feature + org_field = MetadataField.objects.create( + name="org_feature_field", type="str", organisation=organisation + ) + MetadataModelField.objects.create( + field=org_field, content_type=feature_content_type + ) + + # And a project-level field for feature + project_field = MetadataField.objects.create( + name="project_feature_field", + type="str", + organisation=organisation, + project=project, + ) + MetadataModelField.objects.create( + field=project_field, content_type=feature_content_type + ) + + # And a project-level field for segment (should be excluded) + segment_field = MetadataField.objects.create( + name="project_segment_field", + type="str", + organisation=organisation, + project=project, + ) + MetadataModelField.objects.create( + field=segment_field, content_type=segment_content_type + ) + + url = reverse("api-v1:projects:project-metadata-fields-list", args=[project.id]) + + # When + response = admin_client.get(f"{url}?include_organisation=true&entity=feature") + + # Then + assert response.status_code == status.HTTP_200_OK + returned_ids = {r["id"] for r in response.json()["results"]} + assert returned_ids == {org_field.id, project_field.id} + + +@pytest.mark.parametrize( + "field_project_attr, action, expected_status", + [ + ("project", "put", status.HTTP_200_OK), + ("project", "delete", status.HTTP_204_NO_CONTENT), + (None, "put", status.HTTP_403_FORBIDDEN), + (None, "delete", status.HTTP_403_FORBIDDEN), + ("project_b", "put", status.HTTP_403_FORBIDDEN), + ("project_b", "delete", status.HTTP_403_FORBIDDEN), + ], +) +def test_modify_metadata_field__project_admin__permission_check( + staff_user: FFAdminUser, + staff_client: APIClient, + organisation: Organisation, + project: Project, + project_b: Project, + field_project_attr: str | None, + action: str, + expected_status: int, + 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 + ) + field = MetadataField.objects.create( + name="the_field", type="str", organisation=organisation, project=field_project + ) + url = reverse("api-v1:metadata:metadata-fields-detail", args=[field.id]) + + # When + if action == "put": + data = { + "name": "the_field_updated", + "type": "str", + "organisation": organisation.id, + **({"project": field_project.id} if field_project else {}), + } + response = staff_client.put( + url, data=json.dumps(data), content_type="application/json" + ) + else: + response = staff_client.delete(url) + + # Then + assert response.status_code == expected_status diff --git a/api/tests/unit/segments/test_unit_segments_views.py b/api/tests/unit/segments/test_unit_segments_views.py index 57cc212884ed..5dfd501f9406 100644 --- a/api/tests/unit/segments/test_unit_segments_views.py +++ b/api/tests/unit/segments/test_unit_segments_views.py @@ -24,7 +24,13 @@ from environments.models import Environment from features.models import Feature, FeatureSegment, FeatureState from features.versioning.models import EnvironmentFeatureVersion -from metadata.models import Metadata, MetadataModelField +from metadata.models import ( + Metadata, + MetadataField, + MetadataModelField, + MetadataModelFieldRequirement, +) +from organisations.models import Organisation from projects.models import Project from segments.models import Condition, Segment, SegmentRule, WhitelistedSegment from tests.types import WithProjectPermissionsCallable @@ -1842,3 +1848,38 @@ def test_clone_segment_without_name_should_fail( # Then assert response.status_code == status.HTTP_400_BAD_REQUEST + + +def test_create_segment__required_metadata_on_other_project__returns_201( + admin_client: APIClient, + project: Project, + project_b: Project, + organisation: Organisation, + a_metadata_field: MetadataField, + segment_content_type: ContentType, + project_content_type: ContentType, +) -> None: + # Given + model_field = MetadataModelField.objects.create( + field=a_metadata_field, + content_type=segment_content_type, + ) + MetadataModelFieldRequirement.objects.create( + content_type=project_content_type, + object_id=project_b.id, + model_field=model_field, + ) + url = reverse("api-v1:projects:project-segments-list", args=[project.id]) + data = { + "name": "Test segment cross project", + "project": project.id, + "rules": [{"type": "ALL", "rules": [], "conditions": []}], + } + + # When + response = admin_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED diff --git a/frontend/common/services/useMetadataField.ts b/frontend/common/services/useMetadataField.ts index e33711b000bd..9b8f1d8bae52 100644 --- a/frontend/common/services/useMetadataField.ts +++ b/frontend/common/services/useMetadataField.ts @@ -1,7 +1,50 @@ +import { sortBy } from 'lodash' + import { Res } from 'common/types/responses' import { Req } from 'common/types/requests' import { service } from 'common/service' +import transformCorePaging from 'common/transformCorePaging' import Utils from 'common/utils/utils' +import { CustomMetadataField } from 'common/types/metadata-field' +import { + Environment, + Metadata, + MetadataField, + PagedResponse, + ProjectFlag, + Segment, +} from 'common/types/responses' + +type EntityType = 'feature' | 'segment' | 'environment' + +type EntityMetadataParams = { + organisationId: number + projectId: number + entityContentType: number + entityType: EntityType + entityId?: number +} + +type EntityData = ProjectFlag | Segment | Environment + +type EntityWithMetadata = { + metadata?: Metadata[] +} + +function getEntityUrl(params: EntityMetadataParams): string | null { + const { entityId, entityType, projectId } = params + + switch (entityType) { + case 'feature': + return `projects/${projectId}/features/${entityId}/` + case 'segment': + return `projects/${projectId}/segments/${entityId}/` + case 'environment': + return `environments/${entityId}/` + default: + return null + } +} export const metadataService = service .enhanceEndpoints({ addTagTypes: ['Metadata'] }) @@ -11,7 +54,7 @@ export const metadataService = service Res['metadataField'], Req['createMetadataField'] >({ - invalidatesTags: [{ id: 'LIST', type: 'Metadata' }], + invalidatesTags: [{ type: 'Metadata' }], query: (query: Req['createMetadataField']) => ({ body: query.body, method: 'POST', @@ -22,12 +65,96 @@ export const metadataService = service Res['metadataField'], Req['deleteMetadataField'] >({ - invalidatesTags: [{ id: 'LIST', type: 'Metadata' }], + invalidatesTags: [{ type: 'Metadata' }], query: (query: Req['deleteMetadataField']) => ({ method: 'DELETE', url: `metadata/fields/${query.id}/`, }), }), + getEntityMetadataFields: builder.query< + CustomMetadataField[], + EntityMetadataParams + >({ + providesTags: (_res, _err, arg) => [ + { + id: `${arg.entityType}-${arg.entityId ?? 'new'}-${ + arg.entityContentType + }`, + type: 'Metadata', + }, + ], + queryFn: async (arg, _api, _extraOptions, baseQuery) => { + const entityUrl = getEntityUrl(arg) + + // Build queries to run in parallel + const queries: Promise<{ data?: unknown; error?: unknown }>[] = [ + baseQuery({ + url: `projects/${arg.projectId}/metadata/fields/?${Utils.toParam({ + entity: arg.entityType, + include_organisation: true, + page_size: 100, + })}`, + }), + ] + + // Only fetch entity data if we have an entityId + if (arg.entityId && entityUrl) { + queries.push(baseQuery({ url: entityUrl })) + } + + // Fetch all in parallel + const results = await Promise.all(queries) + + const [fieldsRes, entityRes] = results + + // Handle errors + if (fieldsRes.error) { + return { error: fieldsRes.error as Res['metadataList'] } + } + + if (entityRes?.error) { + return { error: entityRes.error as EntityData } + } + + const fieldList = fieldsRes.data as PagedResponse + const entityData = (entityRes?.data ?? + null) as EntityWithMetadata | null + + // Map fields to custom metadata fields with required status + const fieldsForContentType: CustomMetadataField[] = + fieldList.results.map((meta) => { + const matchingModelField = meta.model_fields.find( + (mf) => mf.content_type === arg.entityContentType, + ) + return { + ...meta, + isRequiredFor: !!matchingModelField?.is_required_for.length, + metadataModelFieldId: matchingModelField + ? matchingModelField.id + : null, + } + }) + + // Get existing values from the entity + const existingValues: Metadata[] = entityData?.metadata ?? [] + + // Merge field definitions with existing values + const mergedMetadata = fieldsForContentType.map((field) => { + const existingValue = existingValues.find( + (v) => v.model_field === field.metadataModelFieldId, + ) + return { + ...field, + field_value: existingValue?.field_value ?? '', + hasValue: !!existingValue, + } + }) + + return { + data: sortBy(mergedMetadata, (m) => (m.isRequiredFor ? -1 : 1)), + } + }, + }), getMetadataField: builder.query< Res['metadataField'], Req['getMetadataField'] @@ -45,15 +172,31 @@ export const metadataService = service query: (query: Req['getMetadataList']) => ({ url: `metadata/fields/?${Utils.toParam(query)}`, }), + transformResponse: (res: Res['metadataList'], _, req) => + transformCorePaging(req, res), + }), + getProjectMetadataFieldList: builder.query< + Res['projectMetadataFieldList'], + Req['getProjectMetadataFieldList'] + >({ + providesTags: [{ id: 'LIST', type: 'Metadata' }], + query: (query: Req['getProjectMetadataFieldList']) => ({ + url: `projects/${query.project_id}/metadata/fields/?${Utils.toParam({ + ...(query.include_organisation + ? { include_organisation: true } + : {}), + page: query.page, + page_size: query.page_size, + })}`, + }), + transformResponse: (res: Res['projectMetadataFieldList'], _, req) => + transformCorePaging(req, res), }), updateMetadataField: builder.mutation< Res['metadataField'], Req['updateMetadataField'] >({ - invalidatesTags: (res) => [ - { id: 'LIST', type: 'Metadata' }, - { id: res?.id, type: 'Metadata' }, - ], + invalidatesTags: [{ type: 'Metadata' }], query: (query: Req['updateMetadataField']) => ({ body: query.body, method: 'PUT', @@ -119,13 +262,40 @@ export async function updateMetadata( metadataService.endpoints.updateMetadataField.initiate(data, options), ) } +export async function getEntityMetadataFields( + store: any, + data: EntityMetadataParams, + options?: Parameters< + typeof metadataService.endpoints.getEntityMetadataFields.initiate + >[1], +) { + return store.dispatch( + metadataService.endpoints.getEntityMetadataFields.initiate(data, options), + ) +} +export async function getProjectMetadataFieldList( + store: any, + data: Req['getProjectMetadataFieldList'], + options?: Parameters< + typeof metadataService.endpoints.getProjectMetadataFieldList.initiate + >[1], +) { + return store.dispatch( + metadataService.endpoints.getProjectMetadataFieldList.initiate( + data, + options, + ), + ) +} // END OF FUNCTION_EXPORTS export const { useCreateMetadataFieldMutation, useDeleteMetadataFieldMutation, + useGetEntityMetadataFieldsQuery, useGetMetadataFieldListQuery, useGetMetadataFieldQuery, + useGetProjectMetadataFieldListQuery, useUpdateMetadataFieldMutation, // END OF EXPORTS } = metadataService diff --git a/frontend/common/types/metadata-field.ts b/frontend/common/types/metadata-field.ts new file mode 100644 index 000000000000..a71dbf84088c --- /dev/null +++ b/frontend/common/types/metadata-field.ts @@ -0,0 +1,9 @@ +import { MetadataField } from './responses' + +export type CustomMetadataField = MetadataField & { + metadataModelFieldId: number | string | null + isRequiredFor: boolean + model_field?: string | number + hasValue?: boolean + field_value?: string +} diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index 9a7329328831..fa1569a779fb 100644 --- a/frontend/common/types/requests.ts +++ b/frontend/common/types/requests.ts @@ -425,7 +425,11 @@ export type Req = { } } getMetadataField: { organisation_id: number } - getMetadataList: { organisation: number } + getMetadataList: PagedRequest<{ organisation: number }> + getProjectMetadataFieldList: PagedRequest<{ + project_id: number + include_organisation?: boolean + }> updateMetadataField: { id: number body: { @@ -433,6 +437,7 @@ export type Req = { type: string description: string organisation: number + project?: number | null } } deleteMetadataField: { id: number } @@ -442,6 +447,7 @@ export type Req = { name: string organisation: number type: string + project?: number | null } } diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index 951c0fdfd217..d73a95353bda 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -772,12 +772,20 @@ export type Metadata = { field_value: string } +export type MetadataFieldModelField = { + id: number + content_type: number + is_required_for: isRequiredFor[] +} + export type MetadataField = { id: number name: string type: string description: string organisation: number + project: number | null + model_fields: MetadataFieldModelField[] } export type ContentType = { @@ -789,6 +797,7 @@ export type ContentType = { export type isRequiredFor = { content_type: number + object_id: number } export type MetadataModelField = { @@ -1120,6 +1129,7 @@ export type Res = { metadataModelFieldList: PagedResponse metadataModelField: MetadataModelField metadataList: PagedResponse + projectMetadataFieldList: PagedResponse metadataField: MetadataField launchDarklyProjectImport: LaunchDarklyProjectImport launchDarklyProjectsImport: LaunchDarklyProjectImport[] diff --git a/frontend/common/utils/__tests__/metadataValidation.test.ts b/frontend/common/utils/__tests__/metadataValidation.test.ts new file mode 100644 index 000000000000..860bd86df82c --- /dev/null +++ b/frontend/common/utils/__tests__/metadataValidation.test.ts @@ -0,0 +1,90 @@ +import { getGlobalMetadataValidationState } from 'common/utils/metadataValidation' +import { CustomMetadataField } from 'common/types/metadata-field' + +const createField = ( + partialField: Partial = {}, +): CustomMetadataField => ({ + description: 'A test field', + field_value: '', + hasValue: false, + id: 1, + isRequiredFor: false, + metadataModelFieldId: 1, + name: 'Test Field', + organisation: 1, + type: 'str', + ...partialField, +}) + +describe('getGlobalMetadataValidationState', () => { + it('returns all zeros for empty fields array', () => { + const result = getGlobalMetadataValidationState([]) + + expect(result).toEqual({ + hasUnfilledRequired: false, + totalFilledRequired: 0, + totalRequired: 0, + }) + }) + + it('returns hasUnfilledRequired false when no required fields', () => { + const fields = [ + createField({ id: 1, isRequiredFor: false }), + createField({ id: 2, isRequiredFor: false }), + ] + + const result = getGlobalMetadataValidationState(fields) + + expect(result).toEqual({ + hasUnfilledRequired: false, + totalFilledRequired: 0, + totalRequired: 0, + }) + }) + + it('returns hasUnfilledRequired false when required field is filled', () => { + const fields = [ + createField({ field_value: 'some value', id: 1, isRequiredFor: true }), + ] + + const result = getGlobalMetadataValidationState(fields) + + expect(result).toEqual({ + hasUnfilledRequired: false, + totalFilledRequired: 1, + totalRequired: 1, + }) + }) + + it('returns hasUnfilledRequired true when some required fields are unfilled', () => { + const fields = [ + createField({ field_value: 'filled', id: 1, isRequiredFor: true }), + createField({ field_value: '', id: 2, isRequiredFor: true }), + createField({ field_value: '', id: 3, isRequiredFor: false }), + ] + + const result = getGlobalMetadataValidationState(fields) + + expect(result).toEqual({ + hasUnfilledRequired: true, + totalFilledRequired: 1, + totalRequired: 2, + }) + }) + + it('returns hasUnfilledRequired false when all required fields are filled', () => { + const fields = [ + createField({ field_value: 'filled', id: 1, isRequiredFor: true }), + createField({ field_value: 'also filled', id: 2, isRequiredFor: true }), + createField({ field_value: '', id: 3, isRequiredFor: false }), + ] + + const result = getGlobalMetadataValidationState(fields) + + expect(result).toEqual({ + hasUnfilledRequired: false, + totalFilledRequired: 2, + totalRequired: 2, + }) + }) +}) diff --git a/frontend/common/utils/metadataValidation.ts b/frontend/common/utils/metadataValidation.ts new file mode 100644 index 000000000000..d66e9c4be57c --- /dev/null +++ b/frontend/common/utils/metadataValidation.ts @@ -0,0 +1,29 @@ +import { useMemo } from 'react' +import { CustomMetadataField } from 'common/types/metadata-field' + +export type MetadataValidationState = { + hasUnfilledRequired: boolean + totalRequired: number + totalFilledRequired: number +} + +export function getGlobalMetadataValidationState( + fields: CustomMetadataField[], +): MetadataValidationState { + const totalRequired = fields.filter((f) => f.isRequiredFor).length + const totalFilledRequired = fields.filter( + (f) => f.isRequiredFor && f.field_value && f.field_value !== '', + ).length + + return { + hasUnfilledRequired: + totalRequired > 0 && totalFilledRequired < totalRequired, + totalFilledRequired, + totalRequired, + } +} +export function useGlobalMetadataValidation( + fields: CustomMetadataField[], +): MetadataValidationState { + return useMemo(() => getGlobalMetadataValidationState(fields), [fields]) +} diff --git a/frontend/web/components/metadata/AddMetadataToEntity.tsx b/frontend/web/components/metadata/AddMetadataToEntity.tsx index f460b07af5d4..23582ab1c5f7 100644 --- a/frontend/web/components/metadata/AddMetadataToEntity.tsx +++ b/frontend/web/components/metadata/AddMetadataToEntity.tsx @@ -1,43 +1,54 @@ -import React, { FC, useEffect, useState } from 'react' +import React, { FC, useCallback, useEffect, useState } from 'react' import PanelSearch from 'components/PanelSearch' import Button from 'components/base/forms/Button' -import { useGetMetadataModelFieldListQuery } from 'common/services/useMetadataModelField' -import { useGetMetadataFieldListQuery } from 'common/services/useMetadataField' -import { useGetSegmentQuery } from 'common/services/useSegment' -import { - useGetEnvironmentQuery, - useUpdateEnvironmentMutation, -} from 'common/services/useEnvironment' -import { MetadataField, Metadata } from 'common/types/responses' +import { useUpdateEnvironmentMutation } from 'common/services/useEnvironment' +import { Metadata } from 'common/types/responses' import Utils from 'common/utils/utils' -import { useGetProjectFlagQuery } from 'common/services/useProjectFlag' -import { sortBy } from 'lodash' import Switch from 'components/Switch' import InputGroup from 'components/base/forms/InputGroup' +import { useGetEntityMetadataFieldsQuery } from 'common/services/useMetadataField' +import { CustomMetadataField } from 'common/types/metadata-field' +import { useGlobalMetadataValidation } from 'common/utils/metadataValidation' +import RedirectCreateCustomFields from './RedirectCreateCustomFields' -export type CustomMetadataField = MetadataField & { - metadataModelFieldId: number | string | null - isRequiredFor: boolean - model_field?: string | number - metadataEntity?: boolean - field_value?: string -} - -type CustomMetadata = (Metadata & CustomMetadataField) | null - -type AddMetadataToEntityType = { +type AddMetadataToEntityProps = { isCloningEnvironment?: boolean - organisationId: string - projectId: string | number + organisationId: number + projectId: number entityContentType: number - entityId: string + entityId?: number entity: string envName?: string - onChange?: (m: CustomMetadataField[]) => void + onChange?: (metadata: Metadata[]) => void setHasMetadataRequired?: (b: boolean) => void } -const AddMetadataToEntity: FC = ({ +function formatMetadataToApi(fields: CustomMetadataField[]): Metadata[] { + return fields + .filter((f) => f.hasValue) + .map(({ field_value, metadataModelFieldId }) => ({ + field_value: field_value ?? '', + model_field: metadataModelFieldId as number, + })) +} + +type MetadataErrorResponse = { + data?: { + metadata?: Array<{ + non_field_errors?: string[] + [key: string]: unknown + }> + } +} + +function getMetadataErrors(error: MetadataErrorResponse): string { + const metadataErrors = error?.data?.metadata + if (!metadataErrors) return '' + + return metadataErrors.flatMap((m) => m.non_field_errors ?? []).join('\n') +} + +const AddMetadataToEntity: FC = ({ entity, entityContentType, entityId, @@ -48,348 +59,174 @@ const AddMetadataToEntity: FC = ({ projectId, setHasMetadataRequired, }) => { - const { data: metadataFieldList, isSuccess: metadataFieldListLoaded } = - useGetMetadataFieldListQuery({ - organisation: organisationId, + const { data: initialFields = [], isLoading } = + useGetEntityMetadataFieldsQuery({ + entityContentType, + entityId, + entityType: entity as 'feature' | 'segment' | 'environment', + organisationId, + projectId, }) - const { - data: metadataModelFieldList, - isSuccess: metadataModelFieldListLoaded, - } = useGetMetadataModelFieldListQuery({ - organisation_id: organisationId, - }) - const { data: projectFeatureData, isSuccess: projectFeatureDataLoaded } = - useGetProjectFlagQuery( - { - id: entityId, - project: projectId, - }, - { skip: entity !== 'feature' || !entityId }, - ) - - const { data: segmentData, isSuccess: segmentDataLoaded } = - useGetSegmentQuery( - { - id: `${entityId}`, - projectId: `${projectId}`, - }, - { skip: entity !== 'segment' || !entityId }, - ) - - const { data: envData, isSuccess: envDataLoaded } = useGetEnvironmentQuery( - { id: entityId }, - { skip: entity !== 'environment' || !entityId }, + const [metadataFields, setMetadataFields] = useState( + [], ) + const [hasChanges, setHasChanges] = useState(false) - const [updateEnvironment] = useUpdateEnvironmentMutation() - - const [ - metadataFieldsAssociatedtoEntity, - setMetadataFieldsAssociatedtoEntity, - ] = useState() + const { hasUnfilledRequired } = useGlobalMetadataValidation(metadataFields) useEffect(() => { - if (metadataFieldsAssociatedtoEntity?.length && metadataChanged) { - const metadataParsed = metadataFieldsAssociatedtoEntity - .filter((m) => m.metadataEntity) - .map((i) => { - const { metadataModelFieldId, ...rest } = i - return { model_field: metadataModelFieldId, ...rest } - }) - onChange?.(metadataParsed as CustomMetadataField[]) + if (initialFields.length > 0) { + setMetadataFields(initialFields) } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [metadataFieldsAssociatedtoEntity]) + }, [initialFields]) - const [metadataChanged, setMetadataChanged] = useState(false) + useEffect(() => { + setHasMetadataRequired?.(hasUnfilledRequired) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [hasUnfilledRequired]) + + const handleFieldChange = useCallback( + (fieldId: number, newValue: string) => { + setMetadataFields((prev) => { + const updatedMetadataFields = prev.map((field) => + field.id === fieldId + ? { ...field, field_value: newValue, hasValue: !!newValue } + : field, + ) - const mergeMetadataEntityWithMetadataField = ( - metadata: Metadata[], // Metadata array - metadataField: CustomMetadataField[], // Custom metadata field array - ) => { - // Create a map of metadata fields using metadataModelFieldId as key - const map = new Map( - metadataField.map((item) => [item.metadataModelFieldId, item]), - ) + // Propagate the change to upstream parents + if (entity !== 'environment' || isCloningEnvironment) { + const formattedMetadata = formatMetadataToApi(updatedMetadataFields) + onChange?.(formattedMetadata) + } + + return updatedMetadataFields + }) + setHasChanges(true) + }, + [entity, isCloningEnvironment, onChange], + ) - // Merge metadata fields with metadata entities - return metadataField.map((item) => { - const mergedItem = { - ...item, // Spread the properties of the metadata field - ...(map.get(item.model_field!) || {}), // Get the corresponding metadata field from the map - ...(metadata.find((m) => m.model_field === item.metadataModelFieldId) || - {}), // Find the corresponding metadata entity - } + const [updateEnvironment] = useUpdateEnvironmentMutation() - // Determine if metadata entity exists - mergedItem.metadataEntity = - mergedItem.metadataModelFieldId !== undefined && - mergedItem.model_field !== undefined + const handleEnvironmentSave = async () => { + if (!envName || !entityId) return - return mergedItem // Return the merged item + const result = await updateEnvironment({ + body: { + metadata: formatMetadataToApi(metadataFields), + name: envName, + project: projectId, + }, + id: entityId, }) - } - useEffect(() => { - if ( - metadataFieldList && - metadataFieldListLoaded && - metadataModelFieldList && - metadataModelFieldListLoaded - ) { - // Filter metadata fields based on the provided content type - const metadataForContentType = metadataFieldList.results - // Filter metadata fields that have corresponding entries in the metadata model field list - .filter((meta) => { - return metadataModelFieldList.results.some((item) => { - return ( - item.field === meta.id && item.content_type === entityContentType - ) - }) - }) - // Map each filtered metadata field to include additional information from the metadata model field list - .map((meta) => { - // Find the matching item in the metadata model field list - const matchingItem = metadataModelFieldList.results.find((item) => { - return ( - item.field === meta.id && item.content_type === entityContentType - ) - }) - // Determine if isRequiredFor should be true or false based on is_required_for array - const isRequiredFor = !!matchingItem?.is_required_for.length - setHasMetadataRequired?.(isRequiredFor) - // Return the metadata field with additional metadata model field information including isRequiredFor - return { - ...meta, - isRequiredFor: isRequiredFor || false, - metadataModelFieldId: matchingItem ? matchingItem.id : null, - } - }) - if (projectFeatureData?.metadata && projectFeatureDataLoaded) { - const mergedFeatureEntity = mergeMetadataEntityWithMetadataField( - projectFeatureData?.metadata, - metadataForContentType, - ) - const sortedArray = sortBy(mergedFeatureEntity, (m) => - m.isRequiredFor ? -1 : 1, - ) - setMetadataFieldsAssociatedtoEntity(sortedArray) - } else if (segmentData?.metadata && segmentDataLoaded) { - const mergedSegmentEntity = mergeMetadataEntityWithMetadataField( - segmentData?.metadata, - metadataForContentType, - ) - const sortedArray = sortBy(mergedSegmentEntity, (m) => - m.isRequiredFor ? -1 : 1, - ) - setMetadataFieldsAssociatedtoEntity(sortedArray) - } else if (envData?.metadata && envDataLoaded) { - const mergedEnvEntity = mergeMetadataEntityWithMetadataField( - envData?.metadata, - metadataForContentType, - ) - const sortedArray = sortBy(mergedEnvEntity, (m) => - m.isRequiredFor ? -1 : 1, - ) - setMetadataFieldsAssociatedtoEntity(sortedArray) - } else { - const sortedArray = sortBy(metadataForContentType, (m) => - m.isRequiredFor ? -1 : 1, - ) - setMetadataFieldsAssociatedtoEntity(sortedArray) - } + if ('error' in result) { + toast(getMetadataErrors(result.error as MetadataErrorResponse), 'danger') + } else { + toast('Environment Field Updated') + setHasChanges(false) } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [ - metadataFieldList, - metadataFieldListLoaded, - metadataModelFieldList, - metadataModelFieldListLoaded, - projectFeatureDataLoaded, - projectFeatureData, - entityId, - envData, - envDataLoaded, - segmentData, - segmentDataLoaded, - ]) - - const getMetadataErrors = (error: any) => { - const nonFieldErrors = - error?.data?.metadata?.map( - (metadata: any) => metadata?.non_field_errors, - ) || [] - const fieldErrors = - error?.data?.metadata?.map((metadata: any) => metadata) || [] - - const allErrors = [...nonFieldErrors, ...fieldErrors] - - return allErrors.join('\n') } return ( - <> - - - Field - Value - - } - items={metadataFieldsAssociatedtoEntity} - renderRow={(m) => { - return ( - { - setMetadataFieldsAssociatedtoEntity((prevState) => - prevState?.map((metadata) => { - if (metadata.id === m?.id) { - return { - ...metadata, - field_value: m?.field_value, - metadataEntity: !!m?.field_value, - } - } - return metadata - }), - ) - setMetadataChanged(true) - }} - /> - ) - }} - renderNoResults={ - - No custom fields configured for {entity}s. Add custom fields in - your{' '} - - Organisation Settings - - . - - } - /> - {entity === 'environment' && !isCloningEnvironment && ( -
- -
+ + + Field + Value + + } + items={metadataFields} + renderRow={(field: CustomMetadataField) => ( + )} - - + renderNoResults={ + + + + } + /> + {entity === 'environment' && !isCloningEnvironment && ( +
+ +
+ )} + ) } -type MetadataRowType = { +type MetadataRowProps = { metadata: CustomMetadataField - getMetadataValue?: (metadata: CustomMetadata) => void - entity: string + onFieldChange: (fieldId: number, value: string) => void } -const MetadataRow: FC = ({ - entity, - getMetadataValue, - metadata, -}) => { - const [metadataValueChanged, setMetadataValueChanged] = - useState(false) - const metadataValue = - metadata?.type === 'bool' - ? metadata?.field_value === 'true' - : metadata?.field_value || '' - const handleChange = (newMetadataValue: string | boolean) => { - setMetadataValueChanged(false) - const updatedMetadataObject = { ...metadata } - updatedMetadataObject.field_value = - metadata?.type === 'bool' ? `${!newMetadataValue}` : `${newMetadataValue}` - getMetadataValue?.(updatedMetadataObject as CustomMetadata) - } +const MetadataRow: FC = ({ metadata, onFieldChange }) => { + const displayValue = + metadata.type === 'bool' + ? metadata.field_value === 'true' + : metadata.field_value || '' - const isRequiredForAndCorrectType = - metadata?.isRequiredFor && - Utils.validateMetadataType(metadata?.type, metadataValue) - const isNotRequiredAndCorrectType = - !!metadataValue && Utils.validateMetadataType(metadata?.type, metadataValue) - const isEmptyAuthorized = !metadataValue && !metadata?.isRequiredFor + const handleChange = (newValue: string | boolean) => { + const stringValue = metadata.type === 'bool' ? `${newValue}` : `${newValue}` + onFieldChange(metadata.id, stringValue) + } + const isEmpty = !displayValue || displayValue === '' + const isValidType = Utils.validateMetadataType(metadata.type, displayValue) + const isValid = isEmpty ? !metadata.isRequiredFor : isValidType return ( - {metadataValueChanged && entity !== 'segment' && ( -
{'*'}
- )} - {`${metadata?.name} ${ - metadata?.isRequiredFor ? '*' : '' - }`} - {metadata?.type === 'bool' ? ( + + {metadata.name} + {metadata.isRequiredFor && '*'} + + {metadata.type === 'bool' ? ( { - setMetadataValueChanged(true) - handleChange(!metadataValue) + const currentBool = + displayValue === true || displayValue === 'true' + handleChange(!currentBool) }} /> ) : ( { - setMetadataValueChanged(true) handleChange(Utils.safeParseEventValue(e)) }} type='text' diff --git a/frontend/web/components/metadata/ContentTypesMetadataFieldTable.tsx b/frontend/web/components/metadata/ContentTypesMetadataFieldTable.tsx index 8a652b0f816a..8812eafb9681 100644 --- a/frontend/web/components/metadata/ContentTypesMetadataFieldTable.tsx +++ b/frontend/web/components/metadata/ContentTypesMetadataFieldTable.tsx @@ -5,7 +5,7 @@ import Button from 'components/base/forms/Button' import Switch from 'components/Switch' import Icon from 'components/Icon' -import { MetadataModelField } from 'common/types/responses' +import { MetadataFieldModelField } from 'common/types/responses' type selectedContentType = { label: string value: string @@ -18,7 +18,7 @@ type ContentTypesMetadataFieldTableType = { onDelete: (removed: selectedContentType) => void isEdit: boolean changeMetadataRequired: (value: string, isRequired: boolean) => void - metadataModelFieldList: MetadataModelField[] + metadataModelFieldList: MetadataFieldModelField[] } type ContentTypesMetadataRowBase = Omit< diff --git a/frontend/web/components/metadata/ContentTypesValues.tsx b/frontend/web/components/metadata/ContentTypesValues.tsx index d05c08b186d0..644ec073f9bd 100644 --- a/frontend/web/components/metadata/ContentTypesValues.tsx +++ b/frontend/web/components/metadata/ContentTypesValues.tsx @@ -1,10 +1,10 @@ import React, { FC } from 'react' import { useGetSupportedContentTypeQuery } from 'common/services/useSupportedContentType' -import { MetadataModelField } from 'common/types/responses' +import { MetadataFieldModelField } from 'common/types/responses' import classNames from 'classnames' type ContentTypesValuesType = { - contentTypes: MetadataModelField[] + contentTypes: MetadataFieldModelField[] organisationId: string } diff --git a/frontend/web/components/metadata/MetadataPage.tsx b/frontend/web/components/metadata/MetadataPage.tsx index 5c4522bfc135..9cf53d757806 100644 --- a/frontend/web/components/metadata/MetadataPage.tsx +++ b/frontend/web/components/metadata/MetadataPage.tsx @@ -1,77 +1,82 @@ -import React, { FC, useMemo } from 'react' +import React, { FC, useState } from 'react' import Button from 'components/base/forms/Button' import PanelSearch from 'components/PanelSearch' import Icon from 'components/Icon' import Panel from 'components/base/grid/Panel' import CreateMetadataField from 'components/modals/CreateMetadataField' import ContentTypesValues from './ContentTypesValues' -import { MetadataModelField } from 'common/types/responses' +import { MetadataFieldModelField } from 'common/types/responses' import { useGetMetadataFieldListQuery, + useGetProjectMetadataFieldListQuery, useDeleteMetadataFieldMutation, } from 'common/services/useMetadataField' -import { useGetMetadataModelFieldListQuery } from 'common/services/useMetadataModelField' import PlanBasedBanner from 'components/PlanBasedAccess' +import RedirectCreateCustomFields from './RedirectCreateCustomFields' +const PAGE_SIZE = 20 const metadataWidth = [200, 150, 150, 90] type MetadataPageType = { organisationId: string + projectId?: string } type MergeMetadata = { - content_type_fields: MetadataModelField[] + model_fields: MetadataFieldModelField[] id: number name: string type: string description: string organisation: number + project: number | null } -const MetadataPage: FC = ({ organisationId }) => { - const { data: metadataFieldList } = useGetMetadataFieldListQuery({ - organisation: organisationId, - }) +const MetadataPage: FC = ({ organisationId, projectId }) => { + const [orgPage, setOrgPage] = useState(1) + const [projectPage, setProjectPage] = useState(1) - const { data: MetadataModelFieldList } = useGetMetadataModelFieldListQuery({ - organisation_id: organisationId, + const { data: orgMetadataFieldList } = useGetMetadataFieldListQuery({ + organisation: parseInt(organisationId), + page: orgPage, + page_size: PAGE_SIZE, }) + const { data: projectMetadataFieldList } = + useGetProjectMetadataFieldListQuery( + { + page: projectPage, + page_size: PAGE_SIZE, + project_id: parseInt(projectId!), + }, + { skip: !projectId }, + ) + const [deleteMetadata] = useDeleteMetadataFieldMutation() - const mergeMetadata = useMemo(() => { - if (metadataFieldList && MetadataModelFieldList) { - return metadataFieldList.results - .map((item1) => { - const matchingItems2 = MetadataModelFieldList.results.filter( - (item2) => item2.field === item1.id, - ) - return { - ...item1, - content_type_fields: matchingItems2, - } - }) - ?.sort((a, b) => a.id - b.id) - } - return null - }, [metadataFieldList, MetadataModelFieldList]) + const orgFields = orgMetadataFieldList?.results ?? undefined + const projectFields = projectMetadataFieldList?.results ?? undefined const metadataCreatedToast = () => { toast('Custom Field Created') closeModal() } - const createMetadataField = () => { + const openCreateMetadataField = () => { openModal( `Create Custom Field`, , 'side-modal create-feature-modal', ) } - const editMetadata = (id: string, contentTypeList: MetadataModelField[]) => { + const editMetadata = ( + id: string, + contentTypeList: MetadataFieldModelField[], + ) => { openModal( `Edit Custom Field`, = ({ organisationId }) => { toast('Custom Field Updated') }} organisationId={organisationId} + projectId={projectId} />, 'side-modal create-feature-modal', ) @@ -104,13 +110,143 @@ const MetadataPage: FC = ({ organisationId }) => { }) } + const renderFieldRow = ( + metadata: MergeMetadata, + { readOnly }: { readOnly: boolean }, + ) => { + const handleEdit = () => + editMetadata(`${metadata.id}`, metadata.model_fields) + + return ( + + +
+ {metadata.name} + {readOnly && Inherited} +
+ + + {!readOnly && ( +
+ +
+ )} + + ) + } + + const renderTableHeader = ({ showRemove }: { showRemove: boolean }) => ( + + Name + {showRemove && ( +
+ Remove +
+ )} + + ) + + if (projectId) { + return ( + + + +
Custom Fields
+ + + +

+ Manage project-level custom fields and view inherited organisation + fields.{' '} + +

+ + +
Organisation Fields
+ + renderFieldRow(metadata, { readOnly: true }) + } + paging={orgMetadataFieldList} + nextPage={() => setOrgPage(orgPage + 1)} + prevPage={() => setOrgPage(orgPage - 1)} + goToPage={(p: number) => setOrgPage(p)} + renderNoResults={ +
+ + + +
+ } + /> + + + +
Project Fields
+ + renderFieldRow(metadata, { readOnly: false }) + } + paging={projectMetadataFieldList} + nextPage={() => setProjectPage(projectPage + 1)} + prevPage={() => setProjectPage(projectPage - 1)} + goToPage={(p: number) => setProjectPage(p)} + renderNoResults={ +
+ + No project-level custom fields configured. + +
+ } + /> + + + ) + } + return (
Custom Fields
 - @@ -128,46 +264,15 @@ const MetadataPage: FC = ({ organisationId }) => { - Name -
- Remove -
- + items={orgFields} + header={renderTableHeader({ showRemove: true })} + renderRow={(metadata: MergeMetadata) => + renderFieldRow(metadata, { readOnly: false }) } - renderRow={(metadata) => ( - { - editMetadata(`${metadata.id}`, metadata.content_type_fields) - }} - > - -
{metadata.name}
- - -
- -
- - )} + paging={orgMetadataFieldList} + nextPage={() => setOrgPage(orgPage + 1)} + prevPage={() => setOrgPage(orgPage - 1)} + goToPage={(p: number) => setOrgPage(p)} renderNoResults={
diff --git a/frontend/web/components/metadata/RedirectCreateCustomFields.tsx b/frontend/web/components/metadata/RedirectCreateCustomFields.tsx new file mode 100644 index 000000000000..d5ddd38fc6d2 --- /dev/null +++ b/frontend/web/components/metadata/RedirectCreateCustomFields.tsx @@ -0,0 +1,44 @@ +import React, { FC } from 'react' + +type RedirectCreateCustomFieldsProps = { + organisationId: number + projectId?: number + organisationOnly: boolean +} + +const RedirectCreateCustomFields: FC = ({ + organisationId, + organisationOnly, + projectId, +}) => { + const orgLink = `/organisation/${organisationId}/settings?tab=custom-fields` + const projectLink = `/project/${projectId}/settings?tab=custom-fields` + + if (organisationOnly) { + return ( + + You can create Organisation Custom Fields in your{' '} + + Organisation Settings + + . + + ) + } + + return ( + + You can create Custom Fields in your{' '} + + Organisation Settings + {' '} + or{' '} + + Project Settings + + . + + ) +} + +export default RedirectCreateCustomFields diff --git a/frontend/web/components/metadata/SupportedContentTypesSelect.tsx b/frontend/web/components/metadata/SupportedContentTypesSelect.tsx index f34e80502f65..99a555dec50b 100644 --- a/frontend/web/components/metadata/SupportedContentTypesSelect.tsx +++ b/frontend/web/components/metadata/SupportedContentTypesSelect.tsx @@ -1,6 +1,6 @@ import React, { FC, useEffect, useState } from 'react' import { useGetSupportedContentTypeQuery } from 'common/services/useSupportedContentType' -import { ContentType, MetadataModelField } from 'common/types/responses' +import { ContentType, MetadataFieldModelField } from 'common/types/responses' import InputGroup from 'components/base/forms/InputGroup' import ContentTypesMetadataFieldTable from './ContentTypesMetadataFieldTable' @@ -8,7 +8,7 @@ type SupportedContentTypesSelectType = { organisationId: string isEdit: boolean getMetadataContentTypes: (m: SelectContentTypesType[]) => void - metadataModelFieldList: MetadataModelField[] + metadataModelFieldList: MetadataFieldModelField[] } export type SelectContentTypesType = { diff --git a/frontend/web/components/modals/CreateMetadataField.tsx b/frontend/web/components/modals/CreateMetadataField.tsx index d23faaaebb48..fbe87e994d72 100644 --- a/frontend/web/components/modals/CreateMetadataField.tsx +++ b/frontend/web/components/modals/CreateMetadataField.tsx @@ -7,10 +7,12 @@ import SupportedContentTypesSelect, { } from 'components/metadata/SupportedContentTypesSelect' import { + metadataService, useCreateMetadataFieldMutation, useGetMetadataFieldQuery, useUpdateMetadataFieldMutation, } from 'common/services/useMetadataField' +import { getStore } from 'common/store' import { useGetSupportedContentTypeQuery } from 'common/services/useSupportedContentType' @@ -21,7 +23,7 @@ import { } from 'common/services/useMetadataModelField' import { ContentType, - MetadataModelField, + MetadataFieldModelField, isRequiredFor, } from 'common/types/responses' import ErrorMessage from 'components/ErrorMessage' @@ -29,12 +31,17 @@ import ErrorMessage from 'components/ErrorMessage' type CreateMetadataFieldType = { id?: string isEdit: boolean - metadataModelFieldList?: MetadataModelField[] + metadataModelFieldList?: MetadataFieldModelField[] onComplete?: () => void organisationId: string + projectId?: string } -type QueryBody = Omit +type QueryBody = { + content_type: number | string + field: number + is_required_for: isRequiredFor[] +} type Query = { body: QueryBody @@ -48,7 +55,8 @@ type MetadataType = { label: string } -type metadataFieldUpdatedSelectListType = MetadataModelField & { +type metadataFieldUpdatedSelectListType = MetadataFieldModelField & { + field: number removed: boolean new: boolean } @@ -64,6 +72,7 @@ const CreateMetadataField: FC = ({ metadataModelFieldList, onComplete, organisationId, + projectId, }) => { const metadataTypes: MetadataType[] = [ { id: 1, label: 'int', value: 'int' }, @@ -80,12 +89,9 @@ const CreateMetadataField: FC = ({ const { data: supportedContentTypes } = useGetSupportedContentTypeQuery({ organisation_id: `${organisationId}`, }) - const [ - createMetadataField, - { error: errorCreating, isLoading: creating, isSuccess: created }, - ] = useCreateMetadataFieldMutation() - const [updateMetadataField, { isLoading: updating, isSuccess: updated }] = - useUpdateMetadataFieldMutation() + const [createMetadataField, { error: errorCreating }] = + useCreateMetadataFieldMutation() + const [updateMetadataField] = useUpdateMetadataFieldMutation() const [createMetadataModelField] = useCreateMetadataModelFieldMutation() const [updateMetadataModelField] = useUpdateMetadataModelFieldMutation() @@ -96,7 +102,9 @@ const CreateMetadataField: FC = ({ Utils.getContentType( supportedContentTypes, 'model', - MetadataContentType.ORGANISATION, + projectId + ? MetadataContentType.PROJECT + : MetadataContentType.ORGANISATION, ) useEffect(() => { @@ -113,20 +121,6 @@ const CreateMetadataField: FC = ({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [data, isLoading]) - useEffect(() => { - if (!updating && updated) { - onComplete?.() - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [updating, updated]) - - useEffect(() => { - if (created && !creating) { - onComplete?.() - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [creating, created]) - const [typeValue, setTypeValue] = useState() const [name, setName] = useState('') const [description, setDescription] = useState('') @@ -151,7 +145,9 @@ const CreateMetadataField: FC = ({ ? ([ { content_type: metadataContentType.id, - object_id: parseInt(organisationId), + object_id: projectId + ? parseInt(projectId) + : parseInt(organisationId), } as isRequiredFor, ] as isRequiredFor[]) : [], @@ -167,19 +163,21 @@ const CreateMetadataField: FC = ({ return query } - const save = () => { - if (isEdit) { - updateMetadataField({ - body: { - description, - name, - organisation: organisationId, - type: `${typeValue?.value}`, - }, - id: id!, - }).then(() => { + const save = async () => { + try { + if (isEdit) { + await updateMetadataField({ + body: { + description, + name, + organisation: organisationId, + type: `${typeValue?.value}`, + ...(projectId ? { project: parseInt(projectId) } : {}), + }, + id: id!, + }).unwrap() if (metadataFieldSelectList.length) { - Promise.all( + await Promise.all( metadataFieldSelectList.map(async (m) => { const query = generateDataQuery( m.value, @@ -188,61 +186,69 @@ const CreateMetadataField: FC = ({ 0, true, ) - await createMetadataModelField(query) + await createMetadataModelField(query).unwrap() }), ) } if (metadataUpdatedSelectList.length) { - Promise.all( + await Promise.all( metadataUpdatedSelectList?.map( async (m: metadataFieldUpdatedSelectListType) => { const query = generateDataQuery( m.content_type, m.field, !!m.is_required_for, - parseInt(m.id), + m.id, m.new, ) if (!m.removed && !m.new) { - await updateMetadataModelField(query) + await updateMetadataModelField(query).unwrap() } else if (m.removed) { await deleteMetadataModelField({ id: m.id, organisation_id: organisationId, - }) + }).unwrap() } else if (m.new) { const newQuery = { ...query } delete newQuery.id - await createMetadataModelField(newQuery) + await createMetadataModelField(newQuery).unwrap() } }, ), ) } - closeModal() - }) - } else { - createMetadataField({ - body: { - description, - name, - organisation: organisationId, - type: `${typeValue?.value}`, - }, - }).then((res) => { - Promise.all( - metadataFieldSelectList.map(async (m) => { - const query = generateDataQuery( - m.value, - res?.data.id, - !!m?.isRequired, - 0, - true, - ) - await createMetadataModelField(query) - }), - ) - }) + } else { + const res = await createMetadataField({ + body: { + description, + name, + organisation: organisationId, + type: `${typeValue?.value}`, + ...(projectId ? { project: parseInt(projectId) } : {}), + }, + }).unwrap() + if (res?.id) { + await Promise.all( + metadataFieldSelectList.map(async (m) => { + const query = generateDataQuery( + m.value, + res.id, + !!m?.isRequired, + 0, + true, + ) + await createMetadataModelField(query).unwrap() + }), + ) + } + } + getStore().dispatch( + metadataService.util.invalidateTags([{ type: 'Metadata' }]), + ) + onComplete?.() + closeModal() + } catch (e) { + toast('Failed to save custom field', 'danger') } } @@ -313,12 +319,14 @@ const CreateMetadataField: FC = ({ if (isRequiredLength !== isRequired) { newMetadataFieldArray.push({ ...item1, + field: parseInt(id!), is_required_for: isRequired, }) } } else { newMetadataFieldArray.push({ ...item1, + field: parseInt(id!), new: false, removed: true, }) @@ -331,6 +339,7 @@ const CreateMetadataField: FC = ({ newMetadataFieldArray.push({ ...item1, content_type: item.value, + field: parseInt(id!), is_required_for: m?.isRequired, new: true, removed: false, diff --git a/frontend/web/components/modals/CreateSegment.tsx b/frontend/web/components/modals/CreateSegment.tsx index 642b1444a6ec..3022a9ff92c9 100644 --- a/frontend/web/components/modals/CreateSegment.tsx +++ b/frontend/web/components/modals/CreateSegment.tsx @@ -466,7 +466,7 @@ const CreateSegment: FC = ({ { diff --git a/frontend/web/components/modals/create-feature/index.js b/frontend/web/components/modals/create-feature/index.js index 2b40e5e02bd5..6dfd04e00fce 100644 --- a/frontend/web/components/modals/create-feature/index.js +++ b/frontend/web/components/modals/create-feature/index.js @@ -498,7 +498,11 @@ const Index = class extends Component { } parseError = (error) => { const { projectFlag } = this.props - let featureError = error?.message || error?.name?.[0] || error + let featureError = + error?.metadata?.flatMap((m) => m.non_field_errors ?? []).join('\n') || + error?.message || + error?.name?.[0] || + error let featureWarning = '' //Treat multivariate no changes as warnings if ( @@ -824,9 +828,6 @@ const Index = class extends Component { > {({ permission: projectAdmin }) => { this.state.skipSaveProjectFeature = !createFeature - const _hasMetadataRequired = - this.state.hasMetadataRequired && - !projectFlag.metadata?.length return (
@@ -1744,7 +1745,7 @@ const Index = class extends Component { isSaving || !projectFlag.name || invalid || - _hasMetadataRequired + this.state.hasMetadataRequired } > {isSaving @@ -1828,11 +1829,11 @@ const Index = class extends Component { } onHasMetadataRequiredChange={( hasMetadataRequired, - ) => + ) => { this.setState({ hasMetadataRequired, }) - } + }} featureError={ this.parseError(error).featureError } @@ -1850,7 +1851,9 @@ const Index = class extends Component { featureLimitPercentage={ this.state.featureLimitAlert.percentage } - hasMetadataRequired={_hasMetadataRequired} + hasMetadataRequired={ + this.state.hasMetadataRequired + } />
)} diff --git a/frontend/web/components/pages/CreateEnvironmentPage.tsx b/frontend/web/components/pages/CreateEnvironmentPage.tsx index 43ce829f02fd..efc5e6f2fde0 100644 --- a/frontend/web/components/pages/CreateEnvironmentPage.tsx +++ b/frontend/web/components/pages/CreateEnvironmentPage.tsx @@ -26,6 +26,7 @@ const CreateEnvironmentPage: React.FC = () => { const [name, setName] = useState('') const [description, setDescription] = useState() const [selectedEnv, setSelectedEnv] = useState() + const [hasMetadataRequired, setHasMetadataRequired] = useState(false) const inputRef = useRef(null) const history = useHistory() @@ -84,10 +85,37 @@ const CreateEnvironmentPage: React.FC = () => { permission='CREATE_ENVIRONMENT' id={projectId} > - {({ isLoading, permission }) => - isLoading ? ( - - ) : permission ? ( + {({ isLoading, permission }) => { + if (isLoading) { + return + } + if (!permission) { + return ( +
+

+ Check your project permissions +

+

+ Although you have been invited to this project, you are not + invited to any environments yet! +

+

+ Contact your project administrator asking them to either: +

    +
  • + Invite you to an environment (e.g. develop) by visiting{' '} + Environment settings +
    • +
  • + Grant permissions to create an environment under{' '} + Project settings. +
    • +
+

+
+ ) + } + return ( {({ createEnv, error, isSaving, project }) => (
{ /> )} - {error && ( - - - - )}
{Utils.getPlansPermission('METADATA') && envContentType?.id && ( @@ -183,19 +206,25 @@ const CreateEnvironmentPage: React.FC = () => { entity={envContentType.model} isCloningEnvironment onChange={setMetadata} + setHasMetadataRequired={setHasMetadataRequired} /> } /> )} + {error && ( + + + + )}
@@ -209,31 +238,8 @@ const CreateEnvironmentPage: React.FC = () => { )} - ) : ( -
-

- Check your project permissions -

-

- Although you have been invited to this project, you are not - invited to any environments yet! -

-

- Contact your project administrator asking them to either: -

    -
  • - Invite you to an environment (e.g. develop) by visiting{' '} - Environment settings -
    • -
  • - Grant permissions to create an environment under{' '} - Project settings. -
    • -
-

-
) - } + }}
) diff --git a/frontend/web/components/pages/project-settings/ProjectSettingsPage.tsx b/frontend/web/components/pages/project-settings/ProjectSettingsPage.tsx index 1871e382788d..73eb166dce7d 100644 --- a/frontend/web/components/pages/project-settings/ProjectSettingsPage.tsx +++ b/frontend/web/components/pages/project-settings/ProjectSettingsPage.tsx @@ -69,6 +69,9 @@ const ProjectSettingsPage = () => { // Derive data from project after all early returns const hasEnvironments = (environments?.results?.length || 0) > 0 const hasFeatureHealth = Utils.getFlagsmithHasFeature('feature_health') + const hasProjectCustomFields = Utils.getFlagsmithHasFeature( + 'project_custom_fields', + ) const organisationId = project.organisation const tabs: ProjectSettingsTab[] = [ @@ -111,8 +114,13 @@ const ProjectSettingsPage = () => { label: 'Permissions', }, { - component: , - isVisible: true, + component: ( + + ), + isVisible: hasProjectCustomFields, key: 'custom-fields', label: 'Custom Fields', }, diff --git a/frontend/web/components/pages/project-settings/tabs/CustomFieldsTab.tsx b/frontend/web/components/pages/project-settings/tabs/CustomFieldsTab.tsx index 864d87769c21..0128a6aa671e 100644 --- a/frontend/web/components/pages/project-settings/tabs/CustomFieldsTab.tsx +++ b/frontend/web/components/pages/project-settings/tabs/CustomFieldsTab.tsx @@ -1,12 +1,16 @@ import InfoMessage from 'components/InfoMessage' -import WarningMessage from 'components/WarningMessage' +import MetadataPage from 'components/metadata/MetadataPage' import React from 'react' type CustomFieldsTabProps = { organisationId: number + projectId: number } -export const CustomFieldsTab = ({ organisationId }: CustomFieldsTabProps) => { +export const CustomFieldsTab = ({ + organisationId, + projectId, +}: CustomFieldsTabProps) => { if (!organisationId) { return (
@@ -17,21 +21,9 @@ export const CustomFieldsTab = ({ organisationId }: CustomFieldsTabProps) => { return (
-
Custom Fields
- - - Custom fields have been moved to{' '} - - Organisation Settings - - . - - } +
)