diff --git a/.release-please-manifest.json b/.release-please-manifest.json index 5b7b34ce9594..6e02cab6aae2 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,3 +1,3 @@ { - ".": "2.114.0" + ".": "2.115.0" } \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f6223efde08..f109babde035 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,35 @@ # Changelog +## [2.115.0](https://github.com/Flagsmith/flagsmith/compare/v2.114.1...v2.115.0) (2024-05-15) + + +### Features + +* Add metadata fields to core entities (API) ([#3315](https://github.com/Flagsmith/flagsmith/issues/3315)) ([06eb8a4](https://github.com/Flagsmith/flagsmith/commit/06eb8a4754bf8afbff29974eb6e075953ca0352d)) + + +### Bug Fixes + +* add trailing slash to update group logic ([#3943](https://github.com/Flagsmith/flagsmith/issues/3943)) ([95b14d1](https://github.com/Flagsmith/flagsmith/commit/95b14d121d0a8d2419746ac5112b6ca78670f5d9)) +* changed the error message from custom_auth serializer ([#3924](https://github.com/Flagsmith/flagsmith/issues/3924)) ([185bd6a](https://github.com/Flagsmith/flagsmith/commit/185bd6a441af53fefec2426a891df0a1d140af58)) +* Create GitHub comment as table ([#3948](https://github.com/Flagsmith/flagsmith/issues/3948)) ([bf67b1d](https://github.com/Flagsmith/flagsmith/commit/bf67b1dc0ad3a34c38f8aabb8a5cfaf5f65863de)) +* Organisation ID is an object calling useHasPermission at organisation level ([#3950](https://github.com/Flagsmith/flagsmith/issues/3950)) ([1372917](https://github.com/Flagsmith/flagsmith/commit/13729173d0587c322272f603f1957908874f9375)) +* organisation id parsing ([#3954](https://github.com/Flagsmith/flagsmith/issues/3954)) ([aae116b](https://github.com/Flagsmith/flagsmith/commit/aae116bc6fd83837e6ead8bc681155c25149dcdc)) +* Scroll to top on path change ([#3926](https://github.com/Flagsmith/flagsmith/issues/3926)) ([1a2e793](https://github.com/Flagsmith/flagsmith/commit/1a2e793e9bb47922ad0c688c445a54d5ff2677db)) +* segment override link ([#3945](https://github.com/Flagsmith/flagsmith/issues/3945)) ([fc0cceb](https://github.com/Flagsmith/flagsmith/commit/fc0cceb40b881891878678c1c61cc0cc1148d090)) +* Validate and handle URL params ([#3932](https://github.com/Flagsmith/flagsmith/issues/3932)) ([7e1617f](https://github.com/Flagsmith/flagsmith/commit/7e1617f5bd4e754dbc7d17db957f4315c53c3fba)) +* **versioning:** prevent task from deleting all unrelated feature states / feature segments ([#3955](https://github.com/Flagsmith/flagsmith/issues/3955)) ([0ed5148](https://github.com/Flagsmith/flagsmith/commit/0ed5148183e21a74ad93aa9e0af47a08941cfed6)) + +## [2.114.1](https://github.com/Flagsmith/flagsmith/compare/v2.114.0...v2.114.1) (2024-05-14) + + +### Bug Fixes + +* Add multivariate values when cloning identities ([#3894](https://github.com/Flagsmith/flagsmith/issues/3894)) ([92e3e9f](https://github.com/Flagsmith/flagsmith/commit/92e3e9f55c25855cd0d1b7b7333184ab54385846)) +* Organisation id not numeric in organisation settings ([#3929](https://github.com/Flagsmith/flagsmith/issues/3929)) ([9e3746b](https://github.com/Flagsmith/flagsmith/commit/9e3746bd7dce9a8d2eac6bf616a73f2dd1f5b54e)) +* **versioning:** fix exception getting feature states for edge identity post v2 versioning migration ([#3916](https://github.com/Flagsmith/flagsmith/issues/3916)) ([132ef77](https://github.com/Flagsmith/flagsmith/commit/132ef77ee0d50c618c23431e3ea1b60aec3e5bf4)) +* **versioning:** handle mapping of environment to engine post v2 versioning migration ([#3913](https://github.com/Flagsmith/flagsmith/issues/3913)) ([75acd12](https://github.com/Flagsmith/flagsmith/commit/75acd12c632a9fe8b4a5af5a48a33d18a406e9d4)) + ## [2.114.0](https://github.com/Flagsmith/flagsmith/compare/v2.113.0...v2.114.0) (2024-05-10) diff --git a/README.md b/README.md index e56797673779..6a305147fc7c 100644 --- a/README.md +++ b/README.md @@ -75,3 +75,7 @@ REST calls to the API. - [Website](https://flagsmith.com/) - [Documentation](https://docs.flagsmith.com/) - If you have any questions about our projects you can email [support@flagsmith.com](mailto:support@flagsmith.com) + +## Acknowledgements + +Thank you to [Uffizzi](https://www.uffizzi.com) for providing ephemeral environments to preview pull requests. diff --git a/api/app/settings/common.py b/api/app/settings/common.py index 16204f9a64e5..1544fdc58923 100644 --- a/api/app/settings/common.py +++ b/api/app/settings/common.py @@ -1198,6 +1198,17 @@ ENABLE_API_USAGE_ALERTING = env.bool("ENABLE_API_USAGE_ALERTING", default=False) +# See DomainAuthMethods in flagsmith-auth-controller repository with auth_controller.models module +GLOBAL_DOMAIN_AUTH_METHODS = env.dict( + "GLOBAL_DOMAIN_AUTH_METHODS", + { + "EP": True, # Email / Password + "GO": True, # Google + "GH": True, # GitHub + "SAML": True, # Security Assertion Markup Language + }, +) + EDGE_V2_MIGRATION_READ_CAPACITY_BUDGET = env.int( "EDGE_V2_MIGRATION_READ_CAPACITY_BUDGET", default=0, diff --git a/api/conftest.py b/api/conftest.py index cfa1c70e124b..265a07ecb52d 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -336,7 +336,7 @@ def with_environment_permissions( """ def _with_environment_permissions( - permission_keys: list[str], + permission_keys: list[str] | None = None, environment_id: int | None = None, admin: bool = False, ) -> UserEnvironmentPermission: @@ -344,7 +344,8 @@ def _with_environment_permissions( uep, __ = UserEnvironmentPermission.objects.get_or_create( environment_id=environment_id, user=staff_user, defaults={"admin": admin} ) - uep.permissions.add(*permission_keys) + if permission_keys: + uep.permissions.add(*permission_keys) return uep @@ -384,7 +385,7 @@ def with_project_permissions( """ def _with_project_permissions( - permission_keys: list[str] = None, + permission_keys: list[str] | None = None, project_id: typing.Optional[int] = None, admin: bool = False, ) -> UserProjectPermission: @@ -552,6 +553,29 @@ def segment_featurestate(feature_segment, feature, environment): ) +@pytest.fixture() +def feature_with_value_segment( + feature_with_value: Feature, segment: Segment, environment: Environment +) -> FeatureSegment: + return FeatureSegment.objects.create( + feature=feature_with_value, segment=segment, environment=environment + ) + + +@pytest.fixture() +def segment_featurestate_and_feature_with_value( + feature_with_value_segment: FeatureSegment, + feature_with_value: Feature, + environment: Environment, +) -> FeatureState: + return FeatureState.objects.create( + feature_segment=feature_with_value_segment, + feature=feature_with_value, + environment=environment, + updated_at="2024-01-01 00:00:00", + ) + + @pytest.fixture() def environment_api_key(environment): return EnvironmentAPIKey.objects.create( @@ -658,23 +682,23 @@ def task_processor_synchronously(settings): @pytest.fixture() -def a_metadata_field(organisation): +def a_metadata_field(organisation: Organisation) -> MetadataField: return MetadataField.objects.create(name="a", type="int", organisation=organisation) @pytest.fixture() -def b_metadata_field(organisation): +def b_metadata_field(organisation: Organisation) -> MetadataField: return MetadataField.objects.create(name="b", type="str", organisation=organisation) @pytest.fixture() def required_a_environment_metadata_field( - organisation, - a_metadata_field, - environment, - project, - project_content_type, -): + organisation: Organisation, + a_metadata_field: MetadataField, + environment: Environment, + project: Project, + project_content_type: ContentType, +) -> MetadataModelField: environment_type = ContentType.objects.get_for_model(environment) model_field = MetadataModelField.objects.create( field=a_metadata_field, @@ -688,7 +712,119 @@ def required_a_environment_metadata_field( @pytest.fixture() -def optional_b_environment_metadata_field(organisation, b_metadata_field, environment): +def required_a_feature_metadata_field( + organisation: Organisation, + a_metadata_field: MetadataField, + feature_content_type: ContentType, + project: Project, + project_content_type: ContentType, +) -> MetadataModelField: + 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.id, model_field=model_field + ) + + return model_field + + +@pytest.fixture() +def required_a_feature_metadata_field_using_organisation_content_type( + organisation: Organisation, + a_metadata_field: MetadataField, + feature_content_type: ContentType, + project: Project, + organisation_content_type: ContentType, +) -> MetadataModelField: + model_field = MetadataModelField.objects.create( + field=a_metadata_field, + content_type=feature_content_type, + ) + + MetadataModelFieldRequirement.objects.create( + content_type=organisation_content_type, + object_id=organisation.id, + model_field=model_field, + ) + + return model_field + + +@pytest.fixture() +def required_a_segment_metadata_field( + organisation: Organisation, + a_metadata_field: MetadataField, + segment_content_type: ContentType, + project: Project, + project_content_type: ContentType, +) -> MetadataModelField: + 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 + ) + + return model_field + + +@pytest.fixture() +def required_a_segment_metadata_field_using_organisation_content_type( + organisation: Organisation, + a_metadata_field: MetadataField, + segment_content_type: ContentType, + project: Project, + organisation_content_type: ContentType, +) -> MetadataModelField: + 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, + ) + + return model_field + + +@pytest.fixture() +def optional_b_feature_metadata_field( + organisation: Organisation, b_metadata_field: MetadataField, feature: Feature +) -> MetadataModelField: + feature_type = ContentType.objects.get_for_model(feature) + + return MetadataModelField.objects.create( + field=b_metadata_field, + content_type=feature_type, + ) + + +@pytest.fixture() +def optional_b_segment_metadata_field( + organisation: Organisation, b_metadata_field: MetadataField, segment: Segment +) -> MetadataModelField: + segment_type = ContentType.objects.get_for_model(segment) + + return MetadataModelField.objects.create( + field=b_metadata_field, + content_type=segment_type, + ) + + +@pytest.fixture() +def optional_b_environment_metadata_field( + organisation: Organisation, + b_metadata_field: MetadataField, + environment: Environment, +) -> MetadataModelField: environment_type = ContentType.objects.get_for_model(environment) return MetadataModelField.objects.create( @@ -698,7 +834,10 @@ def optional_b_environment_metadata_field(organisation, b_metadata_field, enviro @pytest.fixture() -def environment_metadata_a(environment, required_a_environment_metadata_field): +def environment_metadata_a( + environment: Environment, + required_a_environment_metadata_field: MetadataModelField, +) -> Metadata: environment_type = ContentType.objects.get_for_model(environment) return Metadata.objects.create( object_id=environment.id, @@ -709,7 +848,10 @@ def environment_metadata_a(environment, required_a_environment_metadata_field): @pytest.fixture() -def environment_metadata_b(environment, optional_b_environment_metadata_field): +def environment_metadata_b( + environment: Environment, + optional_b_environment_metadata_field: MetadataModelField, +) -> Metadata: environment_type = ContentType.objects.get_for_model(environment) return Metadata.objects.create( object_id=environment.id, @@ -720,15 +862,30 @@ def environment_metadata_b(environment, optional_b_environment_metadata_field): @pytest.fixture() -def environment_content_type(): +def environment_content_type() -> ContentType: return ContentType.objects.get_for_model(Environment) @pytest.fixture() -def project_content_type(): +def feature_content_type() -> ContentType: + return ContentType.objects.get_for_model(Feature) + + +@pytest.fixture() +def segment_content_type() -> ContentType: + return ContentType.objects.get_for_model(Segment) + + +@pytest.fixture() +def project_content_type() -> ContentType: return ContentType.objects.get_for_model(Project) +@pytest.fixture() +def organisation_content_type() -> ContentType: + return ContentType.objects.get_for_model(Organisation) + + @pytest.fixture def manage_user_group_permission(db): return OrganisationPermissionModel.objects.get(key=MANAGE_USER_GROUPS) diff --git a/api/custom_auth/serializers.py b/api/custom_auth/serializers.py index fe680fb5af35..55bb43e595ae 100644 --- a/api/custom_auth/serializers.py +++ b/api/custom_auth/serializers.py @@ -40,7 +40,7 @@ class Meta(UserCreateSerializer.Meta): UniqueValidator( queryset=FFAdminUser.objects.all(), lookup="iexact", - message="Invalid email address.", + message="Email already exists. Please log in.", ) ] } diff --git a/api/edge_api/identities/models.py b/api/edge_api/identities/models.py index fd11379ca0e5..d67bcb5831c8 100644 --- a/api/edge_api/identities/models.py +++ b/api/edge_api/identities/models.py @@ -259,5 +259,6 @@ def clone_flag_states_from(self, source_identity: "EdgeIdentity") -> None: feature=feature_in_source.feature, feature_state_value=feature_in_source.feature_state_value, enabled=feature_in_source.enabled, + multivariate_feature_state_values=feature_in_source.multivariate_feature_state_values, ) self.add_feature_override(feature_state_target) diff --git a/api/environments/constants.py b/api/environments/constants.py new file mode 100644 index 000000000000..8f83d38c5b3e --- /dev/null +++ b/api/environments/constants.py @@ -0,0 +1,8 @@ +IDENTITY_INTEGRATIONS_RELATION_NAMES = [ + "amplitude_config", + "heap_config", + "mixpanel_config", + "rudderstack_config", + "segment_config", + "webhook_config", +] diff --git a/api/environments/managers.py b/api/environments/managers.py index bf9d7cf03338..fe0e0a4b51df 100644 --- a/api/environments/managers.py +++ b/api/environments/managers.py @@ -6,33 +6,21 @@ class EnvironmentManager(SoftDeleteManager): - def filter_for_document_builder(self, *args, **kwargs): + def filter_for_document_builder( + self, + *args, + extra_select_related: list[str] | None = None, + extra_prefetch_related: list[Prefetch | str] | None = None, + **kwargs, + ): return ( super() .select_related( "project", "project__organisation", - "amplitude_config", - "dynatrace_config", - "heap_config", - "mixpanel_config", - "rudderstack_config", - "segment_config", - "webhook_config", + *extra_select_related or (), ) .prefetch_related( - Prefetch( - "feature_states", - queryset=FeatureState.objects.select_related( - "feature", "feature_state_value" - ), - ), - Prefetch( - "feature_states__multivariate_feature_state_values", - queryset=MultivariateFeatureStateValue.objects.select_related( - "multivariate_feature_option" - ), - ), "project__segments", "project__segments__rules", "project__segments__rules__rules", @@ -55,6 +43,7 @@ def filter_for_document_builder(self, *args, **kwargs): "multivariate_feature_option" ), ), + *extra_prefetch_related or (), ) .filter(*args, **kwargs) ) diff --git a/api/environments/models.py b/api/environments/models.py index 99c65709d056..90d69b5af5be 100644 --- a/api/environments/models.py +++ b/api/environments/models.py @@ -1,6 +1,3 @@ -# -*- coding: utf-8 -*- -from __future__ import unicode_literals - import logging import typing from copy import deepcopy @@ -11,7 +8,7 @@ from django.contrib.contenttypes.fields import GenericRelation from django.core.cache import caches from django.db import models -from django.db.models import Q +from django.db.models import Prefetch, Q from django.utils import timezone from django.utils.translation import ugettext_lazy as _ from django_lifecycle import ( @@ -35,6 +32,7 @@ generate_client_api_key, generate_server_api_key, ) +from environments.constants import IDENTITY_INTEGRATIONS_RELATION_NAMES from environments.dynamodb import ( DynamoEnvironmentAPIKeyWrapper, DynamoEnvironmentV2Wrapper, @@ -43,10 +41,11 @@ from environments.exceptions import EnvironmentHeaderNotPresentError from environments.managers import EnvironmentManager from features.models import Feature, FeatureSegment, FeatureState +from features.multivariate.models import MultivariateFeatureStateValue from metadata.models import Metadata from projects.models import Project from segments.models import Segment -from util.mappers import map_environment_to_environment_document +from util.mappers import map_environment_to_sdk_document from webhooks.models import AbstractBaseExportableWebhookModel logger = logging.getLogger(__name__) @@ -207,11 +206,7 @@ def get_from_cache(cls, api_key): select_related_args = ( "project", "project__organisation", - "mixpanel_config", - "segment_config", - "amplitude_config", - "heap_config", - "dynatrace_config", + *IDENTITY_INTEGRATIONS_RELATION_NAMES, ) base_qs = cls.objects.select_related(*select_related_args).defer( "description" @@ -237,7 +232,24 @@ def write_environments_to_dynamodb( Q(id=environment_id) if environment_id else Q(project_id=project_id) ) environments = list( - cls.objects.filter_for_document_builder(environments_filter) + cls.objects.filter_for_document_builder( + environments_filter, + extra_select_related=IDENTITY_INTEGRATIONS_RELATION_NAMES, + extra_prefetch_related=[ + Prefetch( + "feature_states", + queryset=FeatureState.objects.select_related( + "feature", "feature_state_value" + ), + ), + Prefetch( + "feature_states__multivariate_feature_state_values", + queryset=MultivariateFeatureStateValue.objects.select_related( + "multivariate_feature_option" + ), + ), + ], + ) ) if not environments: return @@ -359,8 +371,40 @@ def _get_environment_document_from_db( cls, api_key: str, ) -> dict[str, typing.Any]: - environment = cls.objects.filter_for_document_builder(api_key=api_key).get() - return map_environment_to_environment_document(environment) + environment = cls.objects.filter_for_document_builder( + api_key=api_key, + extra_prefetch_related=[ + Prefetch( + "feature_states", + queryset=FeatureState.objects.select_related( + "feature", + "feature_state_value", + "identity", + "identity__environment", + ).prefetch_related( + Prefetch( + "identity__identity_features", + queryset=FeatureState.objects.select_related( + "feature", "feature_state_value", "environment" + ), + ), + Prefetch( + "identity__identity_features__multivariate_feature_state_values", + queryset=MultivariateFeatureStateValue.objects.select_related( + "multivariate_feature_option" + ), + ), + ), + ), + Prefetch( + "feature_states__multivariate_feature_state_values", + queryset=MultivariateFeatureStateValue.objects.select_related( + "multivariate_feature_option" + ), + ), + ], + ).get() + return map_environment_to_sdk_document(environment) def _get_environment(self): return self diff --git a/api/environments/sdk/schemas.py b/api/environments/sdk/schemas.py index 9741f4130df2..f96b656fe81d 100644 --- a/api/environments/sdk/schemas.py +++ b/api/environments/sdk/schemas.py @@ -1,14 +1,10 @@ from flag_engine.environments.models import EnvironmentModel +from environments.constants import IDENTITY_INTEGRATIONS_RELATION_NAMES from util.pydantic import exclude_model_fields SDKEnvironmentDocumentModel = exclude_model_fields( EnvironmentModel, - "amplitude_config", + *IDENTITY_INTEGRATIONS_RELATION_NAMES, "dynatrace_config", - "heap_config", - "mixpanel_config", - "rudderstack_config", - "segment_config", - "webhook_config", ) diff --git a/api/features/feature_external_resources/models.py b/api/features/feature_external_resources/models.py index e8f307230914..29860fb5a173 100644 --- a/api/features/feature_external_resources/models.py +++ b/api/features/feature_external_resources/models.py @@ -61,11 +61,12 @@ def execute_after_save_actions(self): feature_id=self.feature_id, identity_id__isnull=True ) feature_data: GithubData = generate_data( - github_configuration, - self.feature_id, - self.feature.name, - WebhookEventType.FEATURE_EXTERNAL_RESOURCE_ADDED.value, - feature_states, + github_configuration=github_configuration, + feature_id=self.feature_id, + feature_name=self.feature.name, + type=WebhookEventType.FEATURE_EXTERNAL_RESOURCE_ADDED.value, + feature_states=feature_states, + project_id=self.feature.project_id, ) call_github_app_webhook_for_feature_state.delay( diff --git a/api/features/models.py b/api/features/models.py index 6c86d8cf29ff..8c3db8b473f4 100644 --- a/api/features/models.py +++ b/api/features/models.py @@ -12,6 +12,7 @@ SoftDeleteExportableModel, abstract_base_auditable_model_factory, ) +from django.contrib.contenttypes.fields import GenericRelation from django.core.exceptions import ( NON_FIELD_ERRORS, ObjectDoesNotExist, @@ -75,6 +76,7 @@ ) from features.versioning.models import EnvironmentFeatureVersion from integrations.github.models import GithubConfiguration +from metadata.models import Metadata from projects.models import Project from projects.tags.models import Tag @@ -129,6 +131,8 @@ class Feature( objects = FeatureManager() + metadata = GenericRelation(Metadata) + class Meta: # Note: uniqueness index is added in explicit SQL in the migrations (See 0005, 0050) # TODO: after upgrade to Django 4.0 use UniqueConstraint() @@ -987,12 +991,17 @@ def copy_identity_feature_states( source_feature_id ].enabled - # Copy feature state value from source feature_state + if source_feature_state.feature.type == MULTIVARIATE: + mv_values = [ + mv_value.clone(feature_state=target_feature_state, persist=False) + for mv_value in source_feature_state.multivariate_feature_state_values.all() + ] + MultivariateFeatureStateValue.objects.bulk_create(mv_values) + target_feature_state.feature_state_value.copy_from( source_feature_state.feature_state_value ) - # Save changes to target feature_state target_feature_state.save() @hook(AFTER_SAVE) @@ -1005,7 +1014,6 @@ def create_github_comment(self) -> None: if ( not self.identity_id - and not self.feature_segment and self.feature.external_resources.exists() and self.environment.project.github_project.exists() and self.environment.project.organisation.github_config.exists() @@ -1023,6 +1031,7 @@ def create_github_comment(self) -> None: feature_name=self.feature.name, type=WebhookEventType.FLAG_UPDATED.value, feature_states=feature_states, + project_id=self.environment.project_id, ) if self.deleted_at is not None: diff --git a/api/features/serializers.py b/api/features/serializers.py index b5f5748988e0..242710c65de9 100644 --- a/api/features/serializers.py +++ b/api/features/serializers.py @@ -12,6 +12,7 @@ from environments.sdk.serializers_mixins import ( HideSensitiveFieldsSerializerMixin, ) +from metadata.serializers import MetadataSerializer, SerializerWithMetadata from projects.models import Project from users.serializers import ( UserIdsSerializer, @@ -296,17 +297,44 @@ def get_last_modified_in_current_environment( return getattr(instance, "last_modified_in_current_environment", None) -class ListFeatureSerializer(CreateFeatureSerializer): +class FeatureSerializerWithMetadata(SerializerWithMetadata, CreateFeatureSerializer): + metadata = MetadataSerializer(required=False, many=True) + + class Meta(CreateFeatureSerializer.Meta): + fields = CreateFeatureSerializer.Meta.fields + ("metadata",) + + def get_project(self, validated_data: dict = None) -> Project: + project = self.context.get("project") + if project: + return project + else: + raise serializers.ValidationError( + "Unable to retrieve project for metadata validation." + ) + + +class UpdateFeatureSerializerWithMetadata(FeatureSerializerWithMetadata): + """prevent users from changing certain values after creation""" + + class Meta(FeatureSerializerWithMetadata.Meta): + read_only_fields = FeatureSerializerWithMetadata.Meta.read_only_fields + ( + "default_enabled", + "initial_value", + "name", + ) + + +class ListFeatureSerializer(FeatureSerializerWithMetadata): # This exists purely to reduce the conflicts for the EE repository # which has some extra behaviour here to support Oracle DB. pass -class UpdateFeatureSerializer(CreateFeatureSerializer): +class UpdateFeatureSerializer(ListFeatureSerializer): """prevent users from changing certain values after creation""" - class Meta(CreateFeatureSerializer.Meta): - read_only_fields = CreateFeatureSerializer.Meta.read_only_fields + ( + class Meta(ListFeatureSerializer.Meta): + read_only_fields = ListFeatureSerializer.Meta.read_only_fields + ( "default_enabled", "initial_value", "name", diff --git a/api/features/versioning/tasks.py b/api/features/versioning/tasks.py index b706aa2d5674..8eaa00050df9 100644 --- a/api/features/versioning/tasks.py +++ b/api/features/versioning/tasks.py @@ -47,10 +47,10 @@ def disable_v2_versioning(environment_id: int) -> None: latest_feature_state_ids = [fs.id for fs in latest_feature_states] # delete any feature states and feature segments associated with older versions - FeatureState.objects.filter(identity_id__isnull=True).exclude( - id__in=latest_feature_state_ids - ).delete() - FeatureSegment.objects.exclude( + FeatureState.objects.filter( + identity_id__isnull=True, environment=environment + ).exclude(id__in=latest_feature_state_ids).delete() + FeatureSegment.objects.filter(environment=environment).exclude( feature_states__id__in=latest_feature_state_ids ).delete() @@ -60,7 +60,7 @@ def disable_v2_versioning(environment_id: int) -> None: version=1, live_from=timezone.now(), environment_feature_version=None ) FeatureSegment.objects.filter( - feature_states__id__in=latest_feature_state_ids + environment=environment, feature_states__id__in=latest_feature_state_ids ).update(environment_feature_version=None) EnvironmentFeatureVersion.objects.filter(environment=environment).delete() diff --git a/api/features/views.py b/api/features/views.py index 323bef8dd175..6ac0312b8ebe 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -107,8 +107,8 @@ class FeatureViewSet(viewsets.ModelViewSet): def get_serializer_class(self): return { "list": ListFeatureSerializer, - "retrieve": CreateFeatureSerializer, - "create": CreateFeatureSerializer, + "retrieve": ListFeatureSerializer, + "create": ListFeatureSerializer, "update": UpdateFeatureSerializer, "partial_update": UpdateFeatureSerializer, }.get(self.action, ProjectFeatureSerializer) @@ -131,7 +131,9 @@ def get_queryset(self): ), ), ) - .prefetch_related("multivariate_options", "owners", "tags", "group_owners") + .prefetch_related( + "multivariate_options", "owners", "tags", "group_owners", "metadata" + ) ) query_serializer = FeatureQuerySerializer(data=self.request.query_params) diff --git a/api/integrations/github/constants.py b/api/integrations/github/constants.py index e5ca5be737ff..713e6e73a0e9 100644 --- a/api/integrations/github/constants.py +++ b/api/integrations/github/constants.py @@ -1,8 +1,13 @@ GITHUB_API_URL = "https://api.github.com/" GITHUB_API_VERSION = "2022-11-28" -LINK_FEATURE_TEXT = "### This pull request is linked to a Flagsmith Feature (`%s`):\n" +LINK_FEATURE_TITLE = """**Flagsmith feature linked:** `%s` +Default Values:\n""" +FEATURE_TABLE_HEADER = """| Environment | Enabled | Value | Last Updated (UTC) | +| :--- | :----- | :------ | :------ |\n""" +FEATURE_TABLE_ROW = "| [%s](%s) | %s | %s | %s |\n" +LINK_SEGMENT_TITLE = "Segment `%s` values:\n" UNLINKED_FEATURE_TEXT = "### The feature flag `%s` was unlinked from the issue/PR" -UPDATED_FEATURE_TEXT = "### The Flagsmith Feature `%s` was updated in the environment " -LAST_UPDATED_FEATURE_TEXT = "Last Updated %s" +UPDATED_FEATURE_TEXT = "Flagsmith Feature `%s` has been updated:\n" DELETED_FEATURE_TEXT = "### The Feature Flag `%s` was deleted" +FEATURE_ENVIRONMENT_URL = "%s/project/%s/environment/%s/features?feature=%s&tab=%s" diff --git a/api/integrations/github/github.py b/api/integrations/github/github.py index fad30c2cd6c4..5608a209bddb 100644 --- a/api/integrations/github/github.py +++ b/api/integrations/github/github.py @@ -1,18 +1,21 @@ -import datetime import logging import typing from dataclasses import dataclass import requests +from core.helpers import get_current_site_url from django.conf import settings from features.models import FeatureState, FeatureStateValue from integrations.github.client import generate_token from integrations.github.constants import ( DELETED_FEATURE_TEXT, + FEATURE_ENVIRONMENT_URL, + FEATURE_TABLE_HEADER, + FEATURE_TABLE_ROW, GITHUB_API_URL, - LAST_UPDATED_FEATURE_TEXT, - LINK_FEATURE_TEXT, + LINK_FEATURE_TITLE, + LINK_SEGMENT_TITLE, UNLINKED_FEATURE_TEXT, UPDATED_FEATURE_TEXT, ) @@ -28,8 +31,9 @@ class GithubData: feature_id: int feature_name: str type: str - feature_states: typing.List[dict[str, typing.Any]] = None - url: str = None + feature_states: typing.List[dict[str, typing.Any]] | None = None + url: str | None = None + project_id: int | None = None @classmethod def from_dict(cls, data_dict: dict) -> "GithubData": @@ -65,6 +69,8 @@ def post_comment_to_github( def generate_body_comment( name: str, event_type: str, + project_id: int, + feature_id: int, feature_states: typing.List[typing.Dict[str, typing.Any]], ) -> str: @@ -78,25 +84,34 @@ def generate_body_comment( if is_removed: return delete_text - last_updated_string = LAST_UPDATED_FEATURE_TEXT % ( - datetime.datetime.now().strftime("%dth %b %Y %I:%M%p") - ) - updated_text = UPDATED_FEATURE_TEXT % (name) - - result = updated_text if is_update else LINK_FEATURE_TEXT % (name) + result = UPDATED_FEATURE_TEXT % (name) if is_update else LINK_FEATURE_TITLE % (name) + last_segment_name = "" + if len(feature_states) > 0 and not feature_states[0].get("segment_name"): + result += FEATURE_TABLE_HEADER - # if feature_states is None: for v in feature_states: feature_value = v.get("feature_state_value") - feature_value_type = v.get("feature_state_value_type") - - feature_value_string = f"\n{feature_value_type}\n```{feature_value if feature_value else 'No value.'}```\n\n" - - result += f"**{v['environment_name']}{' - ' + v['segment_name'] if v.get('segment_name') else ''}**\n" - result += f"- [{'x' if v['feature_value'] else ' '}] {'Enabled' if v['feature_value'] else 'Disabled'}" - result += feature_value_string + tab = "segment-overrides" if v.get("segment_name") is not None else "value" + environment_link_url = FEATURE_ENVIRONMENT_URL % ( + get_current_site_url(), + project_id, + v.get("environment_api_key"), + feature_id, + tab, + ) + if v.get("segment_name") is not None and v["segment_name"] != last_segment_name: + result += "\n" + LINK_SEGMENT_TITLE % (v["segment_name"]) + last_segment_name = v["segment_name"] + result += FEATURE_TABLE_HEADER + table_row = FEATURE_TABLE_ROW % ( + v["environment_name"], + environment_link_url, + "✅ Enabled" if v["enabled"] else "❌ Disabled", + f"`{feature_value}`" if feature_value else "", + v["last_updated"], + ) + result += table_row - result += last_updated_string return result @@ -109,8 +124,11 @@ def generate_data( feature_id: int, feature_name: str, type: str, - feature_states: typing.Union[list[FeatureState], list[FeatureStateValue]] = None, - url: str = None, + feature_states: ( + typing.Union[list[FeatureState], list[FeatureStateValue]] | None + ) = None, + url: str | None = None, + project_id: int | None = None, ) -> GithubData: if feature_states: @@ -126,7 +144,11 @@ def generate_data( feature_env_data["feature_state_value_type"] = feature_state_value_type if type is not WebhookEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value: feature_env_data["environment_name"] = feature_state.environment.name - feature_env_data["feature_value"] = feature_state.enabled + feature_env_data["enabled"] = feature_state.enabled + feature_env_data["last_updated"] = feature_state.updated_at + feature_env_data["environment_api_key"] = ( + feature_state.environment.api_key + ) if ( hasattr(feature_state, "feature_segment") and feature_state.feature_segment is not None @@ -147,4 +169,5 @@ def generate_data( else None ), feature_states=feature_states_list if feature_states else None, + project_id=project_id, ) diff --git a/api/integrations/github/tasks.py b/api/integrations/github/tasks.py index 89e301f95d0e..aba93e4e0621 100644 --- a/api/integrations/github/tasks.py +++ b/api/integrations/github/tasks.py @@ -24,12 +24,16 @@ class CallGithubData: def send_post_request(data: CallGithubData) -> None: feature_name = data.github_data.feature_name + feature_id = data.github_data.feature_id + project_id = data.github_data.project_id event_type = data.event_type feature_states = feature_states = ( data.github_data.feature_states if data.github_data.feature_states else None ) installation_id = data.github_data.installation_id - body = generate_body_comment(feature_name, event_type, feature_states) + body = generate_body_comment( + feature_name, event_type, project_id, feature_id, feature_states + ) if ( event_type == WebhookEventType.FLAG_UPDATED.value diff --git a/api/integrations/integration.py b/api/integrations/integration.py index faecf80aebff..f6702f86af4b 100644 --- a/api/integrations/integration.py +++ b/api/integrations/integration.py @@ -1,5 +1,6 @@ from typing import Type, TypedDict +from environments.constants import IDENTITY_INTEGRATIONS_RELATION_NAMES from integrations.amplitude.amplitude import AmplitudeWrapper from integrations.common.wrapper import AbstractBaseIdentityIntegrationWrapper from integrations.heap.heap import HeapWrapper @@ -16,13 +17,24 @@ class IntegrationConfig(TypedDict): IDENTITY_INTEGRATIONS: list[IntegrationConfig] = [ {"relation_name": "amplitude_config", "wrapper": AmplitudeWrapper}, - {"relation_name": "segment_config", "wrapper": SegmentWrapper}, {"relation_name": "heap_config", "wrapper": HeapWrapper}, {"relation_name": "mixpanel_config", "wrapper": MixpanelWrapper}, - {"relation_name": "webhook_config", "wrapper": WebhookWrapper}, {"relation_name": "rudderstack_config", "wrapper": RudderstackWrapper}, + {"relation_name": "segment_config", "wrapper": SegmentWrapper}, + {"relation_name": "webhook_config", "wrapper": WebhookWrapper}, ] +assert set(IDENTITY_INTEGRATIONS_RELATION_NAMES) == ( + _configured_integrations := { + integration_config["relation_name"] + for integration_config in IDENTITY_INTEGRATIONS + } +), ( + "Check that `environments.constants.IDENTITY_INTEGRATIONS_RELATION_NAMES` and " + "`integration.integration.IDENTITY_INTEGRATIONS` contain the same values. \n" + f"Unconfigured integrations: {set(IDENTITY_INTEGRATIONS_RELATION_NAMES) - _configured_integrations}" +) + def identify_integrations(identity, all_feature_states, trait_models=None): for integration in IDENTITY_INTEGRATIONS: diff --git a/api/metadata/models.py b/api/metadata/models.py index ccdd376780c8..26b228a5e283 100644 --- a/api/metadata/models.py +++ b/api/metadata/models.py @@ -6,22 +6,16 @@ from django.db import models from organisations.models import Organisation -from projects.models import Project from .fields import GenericObjectID FIELD_VALUE_MAX_LENGTH = 2000 -METADATA_SUPPORTED_MODELS = ["environment"] - # A map of model name to a function that takes the object id and returns the organisation_id SUPPORTED_REQUIREMENTS_MAPPING = { - "environment": { - "organisation": lambda org_id: org_id, - "project": lambda project_id: Project.objects.get( - id=project_id - ).organisation_id, - } + "environment": ["organisation", "project"], + "feature": ["organisation", "project"], + "segment": ["organisation", "project"], } diff --git a/api/metadata/serializers.py b/api/metadata/serializers.py index 3d5f9a1e6f50..a6fdddf89582 100644 --- a/api/metadata/serializers.py +++ b/api/metadata/serializers.py @@ -9,7 +9,6 @@ ) from .models import ( - SUPPORTED_REQUIREMENTS_MAPPING, Metadata, MetadataField, MetadataModelField, @@ -55,21 +54,13 @@ class Meta: def validate(self, data): data = super().validate(data) for requirement in data.get("is_required_for", []): - try: - get_org_id_func = SUPPORTED_REQUIREMENTS_MAPPING[ - data["content_type"].model - ][requirement["content_type"].model] - except KeyError: - raise serializers.ValidationError( - "Invalid requirement for model {}".format( - data["content_type"].model - ) - ) - - if ( - get_org_id_func(requirement["object_id"]) - != data["field"].organisation_id - ): + org_id = ( + requirement["content_type"] + .model_class() + .objects.get(id=requirement["object_id"]) + .organisation_id + ) + if org_id != data["field"].organisation_id: raise serializers.ValidationError( "The requirement organisation does not match the field organisation" ) diff --git a/api/metadata/views.py b/api/metadata/views.py index 098e266318dc..49680ee866ec 100644 --- a/api/metadata/views.py +++ b/api/metadata/views.py @@ -1,13 +1,14 @@ +from itertools import chain + from django.contrib.contenttypes.models import ContentType from django.utils.decorators import method_decorator from drf_yasg.utils import swagger_auto_schema -from rest_framework import viewsets +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 .models import ( - METADATA_SUPPORTED_MODELS, SUPPORTED_REQUIREMENTS_MAPPING, MetadataField, MetadataModelField, @@ -81,7 +82,13 @@ def get_queryset(self): url_path="supported-content-types", ) def supported_content_types(self, request, organisation_pk=None): - qs = ContentType.objects.filter(model__in=METADATA_SUPPORTED_MODELS) + need_content_type_of = list( + chain.from_iterable( + (key, *value) for key, value in SUPPORTED_REQUIREMENTS_MAPPING.items() + ) + ) + + qs = ContentType.objects.filter(model__in=need_content_type_of) serializer = ContentTypeSerializer(qs, many=True) return Response(serializer.data) @@ -100,11 +107,16 @@ def supported_required_for_models(self, request, organisation_pk=None): serializer = SupportedRequiredForModelQuerySerializer(data=request.query_params) serializer.is_valid(raise_exception=True) - qs = ContentType.objects.filter( - model__in=SUPPORTED_REQUIREMENTS_MAPPING.get( - serializer.data["model_name"], {} - ).keys() + supported_models = SUPPORTED_REQUIREMENTS_MAPPING.get( + serializer.data["model_name"], [] ) + if not supported_models: + return Response( + {"message": "No supported models found for the given model name."}, + status=status.HTTP_404_NOT_FOUND, + ) + + qs = ContentType.objects.filter(model__in=supported_models) serializer = ContentTypeSerializer(qs, many=True) return Response(serializer.data) diff --git a/api/segments/models.py b/api/segments/models.py index 78ddd68ae31d..dea6f980a7ea 100644 --- a/api/segments/models.py +++ b/api/segments/models.py @@ -7,6 +7,7 @@ abstract_base_auditable_model_factory, ) from django.conf import settings +from django.contrib.contenttypes.fields import GenericRelation from django.core.exceptions import ValidationError from django.db import models from flag_engine.segments import constants @@ -18,6 +19,7 @@ ) from audit.related_object_type import RelatedObjectType from features.models import Feature +from metadata.models import Metadata from projects.models import Project logger = logging.getLogger(__name__) @@ -43,6 +45,8 @@ class Segment( Feature, on_delete=models.CASCADE, related_name="segments", null=True ) + metadata = GenericRelation(Metadata) + class Meta: ordering = ("id",) # explicit ordering to prevent pagination warnings diff --git a/api/segments/serializers.py b/api/segments/serializers.py index f5364c408449..1909ddebafd7 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -1,12 +1,15 @@ import typing from django.conf import settings +from django.contrib.contenttypes.models import ContentType from flag_engine.segments.constants import PERCENTAGE_SPLIT from rest_framework import serializers from rest_framework.exceptions import ValidationError from rest_framework.serializers import ListSerializer from rest_framework_recursive.fields import RecursiveField +from metadata.models import Metadata +from metadata.serializers import MetadataSerializer, SerializerWithMetadata from projects.models import Project from segments.models import Condition, Segment, SegmentRule @@ -40,25 +43,32 @@ class Meta: fields = ("id", "type", "rules", "conditions", "delete") -class SegmentSerializer(serializers.ModelSerializer): +class SegmentSerializer(serializers.ModelSerializer, SerializerWithMetadata): rules = RuleSerializer(many=True) + metadata = MetadataSerializer(required=False, many=True) class Meta: model = Segment fields = "__all__" def validate(self, attrs): + attrs = super().validate(attrs) + self.validate_required_metadata(attrs) if not attrs.get("rules"): raise ValidationError( {"rules": "Segment cannot be created without any rules."} ) return attrs + def get_project(self, validated_data: dict = None) -> Project: + return validated_data.get("project") + def create(self, validated_data): project = validated_data["project"] self.validate_project_segment_limit(project) rules_data = validated_data.pop("rules", []) + metadata_data = validated_data.pop("metadata", []) self.validate_segment_rules_conditions_limit(rules_data) # create segment with nested rules and conditions @@ -66,13 +76,16 @@ def create(self, validated_data): self._update_or_create_segment_rules( rules_data, segment=segment, is_create=True ) + self._update_or_create_metadata(metadata_data, segment=segment) return segment def update(self, instance, validated_data): # use the initial data since we need the ids included to determine which to update & which to create rules_data = self.initial_data.pop("rules", []) + metadata_data = validated_data.pop("metadata", []) self.validate_segment_rules_conditions_limit(rules_data) self._update_segment_rules(rules_data, segment=instance) + self._update_or_create_metadata(metadata_data, segment=instance) # remove rules from validated data to prevent error trying to create segment with nested rules del validated_data["rules"] return super().update(instance, validated_data) @@ -156,6 +169,28 @@ def _update_or_create_segment_rules( child_rules, rule=child_rule, is_create=is_create ) + def _update_or_create_metadata( + self, metadata_data: typing.Dict, segment: typing.Optional[Segment] = None + ) -> None: + if len(metadata_data) == 0: + Metadata.objects.filter(object_id=segment.id).delete() + return + if metadata_data is not None: + for metadata_item in metadata_data: + metadata_model_field = metadata_item.pop("model_field", None) + if metadata_item.get("delete"): + Metadata.objects.filter(model_field=metadata_model_field).delete() + continue + + Metadata.objects.update_or_create( + model_field=metadata_model_field, + defaults={ + **metadata_item, + "content_type": ContentType.objects.get_for_model(Segment), + "object_id": segment.id, + }, + ) + @staticmethod def _update_or_create_segment_rule( rule_data: dict, segment: Segment = None, rule: SegmentRule = None diff --git a/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py b/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py index 89d6cf4ba739..3d547801d135 100644 --- a/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py +++ b/api/tests/integration/edge_api/identities/test_edge_identity_featurestates_viewset.py @@ -5,7 +5,13 @@ import pytest from core.constants import BOOLEAN, INTEGER, STRING from django.urls import reverse -from flag_engine.features.models import FeatureModel, FeatureStateModel +from flag_engine.features.models import ( + FeatureModel, + FeatureStateModel, + MultivariateFeatureOptionModel, + MultivariateFeatureStateValueList, + MultivariateFeatureStateValueModel, +) from mypy_boto3_dynamodb.service_resource import Table from pytest_lazyfixture import lazy_fixture from rest_framework import status @@ -18,6 +24,7 @@ IdentityModel, ) from features.models import Feature +from features.multivariate.models import MultivariateFeatureOption from projects.models import Project from tests.integration.helpers import create_mv_option_with_api from util.mappers.engine import map_feature_to_engine @@ -1082,9 +1089,24 @@ def features_for_identity_clone_flag_states_from( project ) + mv_feature = Feature.objects.create( + type="MULTIVARIATE", + name="mv_feature", + initial_value="foo", + project=project, + ) + + mv_variant_1 = MultivariateFeatureOption.objects.create( + feature=mv_feature, + default_percentage_allocation=0, + type=STRING, + string_value="bar", + ) + feature_model_1: FeatureModel = map_feature_to_engine(feature=feature_1) feature_model_2: FeatureModel = map_feature_to_engine(feature=feature_2) feature_model_3: FeatureModel = map_feature_to_engine(feature=feature_3) + mv_feature_model: FeatureModel = map_feature_to_engine(feature=mv_feature) source_identity: EdgeIdentity = create_identity(identifier="source_identity") target_identity: EdgeIdentity = create_identity(identifier="target_identity") @@ -1105,6 +1127,21 @@ def features_for_identity_clone_flag_states_from( feature_state_value=source_feature_state_2_value, ) + source_mv_feature_state = FeatureStateModel( + feature=mv_feature_model, + environment_id=dynamo_enabled_environment, + enabled=True, + multivariate_feature_state_values=MultivariateFeatureStateValueList(), + ) + source_mv_feature_state.multivariate_feature_state_values.append( + MultivariateFeatureStateValueModel( + multivariate_feature_option=MultivariateFeatureOptionModel( + value=mv_variant_1.value + ), + percentage_allocation=100, + ) + ) + target_feature_state_2_value = "Target Identity value for feature 2" target_feature_state_2 = FeatureStateModel( feature=feature_model_2, @@ -1122,6 +1159,7 @@ def features_for_identity_clone_flag_states_from( # Add feature states for features 1 and 2 to source identity source_identity.add_feature_override(feature_state=source_feature_state_1) source_identity.add_feature_override(feature_state=source_feature_state_2) + source_identity.add_feature_override(feature_state=source_mv_feature_state) # Add feature states for features 2 and 3 to target identity. target_identity.add_feature_override(feature_state=target_feature_state_2) @@ -1156,7 +1194,7 @@ def features_for_identity_clone_flag_states_from( response = clone_identity_feature_states_response.json() # Target identity contains only the 2 cloned overridden features states and 1 environment feature state - assert len(response) == 3 + assert len(response) == 4 assert response[0]["feature"]["id"] == feature_1.id assert response[0]["enabled"] == source_feature_state_1.enabled @@ -1172,3 +1210,14 @@ def features_for_identity_clone_flag_states_from( assert response[2]["enabled"] == feature_3.default_enabled assert response[2]["feature_state_value"] == feature_3.initial_value assert response[2]["overridden_by"] is None + + assert response[3]["feature"]["id"] == mv_feature.id + assert response[3]["enabled"] == source_mv_feature_state.enabled + assert response[3]["feature_state_value"] == mv_variant_1.value + assert ( + response[3]["multivariate_feature_state_values"][0][ + "multivariate_feature_option" + ]["value"] + == mv_variant_1.value + ) + assert response[3]["overridden_by"] == "IDENTITY" diff --git a/api/tests/types.py b/api/tests/types.py index b87b5e3924a1..bcc790d8a796 100644 --- a/api/tests/types.py +++ b/api/tests/types.py @@ -5,11 +5,11 @@ from projects.models import UserProjectPermission WithProjectPermissionsCallable = Callable[ - [list[str], int | None, bool], UserProjectPermission + [list[str] | None, int | None, bool], UserProjectPermission ] WithOrganisationPermissionsCallable = Callable[ [list[str], int | None], UserOrganisationPermission ] WithEnvironmentPermissionsCallable = Callable[ - [list[str], int | None, bool], UserEnvironmentPermission + [list[str] | None, int | None, bool], UserEnvironmentPermission ] diff --git a/api/tests/unit/custom_auth/test_unit_custom_auth_serializer.py b/api/tests/unit/custom_auth/test_unit_custom_auth_serializer.py index a00089baaed2..127f5569a3b9 100644 --- a/api/tests/unit/custom_auth/test_unit_custom_auth_serializer.py +++ b/api/tests/unit/custom_auth/test_unit_custom_auth_serializer.py @@ -28,7 +28,7 @@ def test_CustomUserCreateSerializer_does_case_insensitive_lookup_with_email(db): # When assert serializer.is_valid() is False - assert serializer.errors["email"][0] == "Invalid email address." + assert serializer.errors["email"][0] == "Email already exists. Please log in." def test_CustomUserCreateSerializer_calls_is_authentication_method_valid_correctly_if_auth_controller_is_installed( diff --git a/api/tests/unit/environments/identities/test_unit_identities_feature_states_views.py b/api/tests/unit/environments/identities/test_unit_identities_feature_states_views.py index 4f7d2327a0a8..a2d3c3185c03 100644 --- a/api/tests/unit/environments/identities/test_unit_identities_feature_states_views.py +++ b/api/tests/unit/environments/identities/test_unit_identities_feature_states_views.py @@ -1,6 +1,7 @@ import json import pytest +from core.constants import STRING from django.test import Client from django.urls import reverse from rest_framework import status @@ -12,6 +13,10 @@ VIEW_ENVIRONMENT, ) from features.models import Feature, FeatureState, FeatureStateValue +from features.multivariate.models import ( + MultivariateFeatureOption, + MultivariateFeatureStateValue, +) from projects.models import Project from tests.unit.environments.helpers import get_environment_user_client @@ -136,6 +141,20 @@ def features_for_identity_clone_flag_states_from( project ) + mv_feature = Feature.objects.create( + type="MULTIVARIATE", + name="mv_feature", + initial_value="foo", + project=project, + ) + + mv_variant_1 = MultivariateFeatureOption.objects.create( + feature=mv_feature, + default_percentage_allocation=0, + type=STRING, + string_value="bar", + ) + source_identity: Identity = Identity.objects.create( identifier="source_identity", environment=environment ) @@ -165,6 +184,18 @@ def features_for_identity_clone_flag_states_from( string_value=source_feature_state_2_value ) + source_mv_feature_state: FeatureState = FeatureState.objects.create( + feature=mv_feature, + environment=environment, + identity=source_identity, + enabled=True, + ) + MultivariateFeatureStateValue.objects.create( + feature_state=source_mv_feature_state, + multivariate_feature_option=mv_variant_1, + percentage_allocation=100, + ) + target_feature_state_2: FeatureState = FeatureState.objects.create( feature=feature_2, environment=environment, @@ -201,7 +232,7 @@ def features_for_identity_clone_flag_states_from( response = clone_identity_feature_states_response.json() # Target identity contains only the 2 cloned overridden features states and 1 environment feature state - assert len(response) == 3 + assert len(response) == 4 # Assert cloned data is correct assert response[0]["feature"]["id"] == feature_1.id @@ -219,6 +250,17 @@ def features_for_identity_clone_flag_states_from( assert response[2]["feature_state_value"] == feature_3.initial_value assert response[2]["overridden_by"] is None + assert response[3]["feature"]["id"] == mv_feature.id + assert response[3]["enabled"] == source_mv_feature_state.enabled + assert response[3]["feature_state_value"] == mv_variant_1.value + assert ( + response[3]["multivariate_feature_state_values"][0][ + "multivariate_feature_option" + ]["value"] + == mv_variant_1.value + ) + assert response[3]["overridden_by"] == "IDENTITY" + # Target identity feature 3 override has been removed assert not FeatureState.objects.filter( feature=feature_3, diff --git a/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py b/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py index 70192c304ec2..c14b976d5bcd 100644 --- a/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py +++ b/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py @@ -1,19 +1,36 @@ +from typing import TYPE_CHECKING + from core.constants import FLAGSMITH_UPDATED_AT_HEADER from django.urls import reverse from flag_engine.segments.constants import EQUAL from rest_framework import status from rest_framework.test import APIClient +from environments.identities.models import Identity from environments.models import Environment, EnvironmentAPIKey from features.feature_types import MULTIVARIATE -from features.models import Feature, FeatureSegment, FeatureState +from features.models import ( + STRING, + Feature, + FeatureSegment, + FeatureState, + FeatureStateValue, +) from features.multivariate.models import MultivariateFeatureOption from segments.models import Condition, Segment, SegmentRule +if TYPE_CHECKING: + from pytest_django import DjangoAssertNumQueries + + from organisations.models import Organisation + from projects.models import Project + def test_get_environment_document( - organisation_one, organisation_one_project_one, django_assert_num_queries -): + organisation_one: "Organisation", + organisation_one_project_one: "Project", + django_assert_num_queries: "DjangoAssertNumQueries", +) -> None: # Given project = organisation_one_project_one @@ -57,6 +74,21 @@ def test_get_environment_document( enabled=True, ) + # Add identity overrides + identity = Identity.objects.create( + environment=environment, + identifier=f"identity_{i}", + ) + identity_feature_state = FeatureState.objects.create( + identity=identity, + feature=feature, + environment=environment, + ) + FeatureStateValue.objects.filter(feature_state=identity_feature_state).update( + string_value="overridden", + type=STRING, + ) + for i in range(10): mv_feature = Feature.objects.create( name=f"mv_feature_{i}", project=project, type=MULTIVARIATE @@ -76,7 +108,7 @@ def test_get_environment_document( url = reverse("api-v1:environment-document") # When - with django_assert_num_queries(13): + with django_assert_num_queries(15): response = client.get(url) # Then @@ -88,8 +120,9 @@ def test_get_environment_document( def test_get_environment_document_fails_with_invalid_key( - organisation_one, organisation_one_project_one -): + organisation_one: "Organisation", + organisation_one_project_one: "Project", +) -> None: # Given project = organisation_one_project_one diff --git a/api/tests/unit/features/feature_segments/test_unit_feature_segments_models.py b/api/tests/unit/features/feature_segments/test_unit_feature_segments_models.py index 34222c6f3f3b..abf719247417 100644 --- a/api/tests/unit/features/feature_segments/test_unit_feature_segments_models.py +++ b/api/tests/unit/features/feature_segments/test_unit_feature_segments_models.py @@ -1,158 +1,115 @@ -import pytest -from core.constants import STRING -from django.test import TestCase -from flag_engine.segments.constants import EQUAL - -from environments.identities.models import Identity -from environments.identities.traits.models import Trait from environments.models import Environment from features.models import Feature, FeatureSegment -from organisations.models import Organisation from projects.models import Project -from segments.models import Condition, Segment, SegmentRule - - -@pytest.mark.django_db -class FeatureSegmentTest(TestCase): - def setUp(self) -> None: - self.organisation = Organisation.objects.create(name="Test org") - self.project = Project.objects.create( - name="Test project", organisation=self.organisation - ) - self.environment = Environment.objects.create( - name="Test environment", project=self.project - ) - - self.initial_value = "test" - self.remote_config = Feature.objects.create( - name="Remote Config", - initial_value="test", - project=self.project, - ) - - self.segment = Segment.objects.create(name="Test segment", project=self.project) - segment_rule = SegmentRule.objects.create( - segment=self.segment, type=SegmentRule.ALL_RULE - ) - - self.condition_property = "test_property" - self.condition_value = "test_value" - Condition.objects.create( - property=self.condition_property, - value=self.condition_value, - operator=EQUAL, - rule=segment_rule, - ) - - self.matching_identity = Identity.objects.create( - identifier="user_1", environment=self.environment - ) - Trait.objects.create( - identity=self.matching_identity, - trait_key=self.condition_property, - value_type=STRING, - string_value=self.condition_value, - ) - - self.not_matching_identity = Identity.objects.create( - identifier="user_2", environment=self.environment - ) - - def test_feature_segment_is_less_than_other_if_priority_lower(self): - # Given - feature_segment_1 = FeatureSegment.objects.create( - feature=self.remote_config, - segment=self.segment, - environment=self.environment, - priority=1, - ) - - another_segment = Segment.objects.create( - name="Another segment", project=self.project - ) - feature_segment_2 = FeatureSegment.objects.create( - feature=self.remote_config, - segment=another_segment, - environment=self.environment, - priority=2, - ) - - # When - result = feature_segment_2 < feature_segment_1 - - # Then - assert result - - def test_feature_segments_are_created_with_correct_priority(self): - # Given - 5 feature segments - - # 2 with the same feature, environment but a different segment - another_segment = Segment.objects.create( - name="Another segment", project=self.project - ) - feature_segment_1 = FeatureSegment.objects.create( - feature=self.remote_config, - segment=self.segment, - environment=self.environment, - ) - - feature_segment_2 = FeatureSegment.objects.create( - feature=self.remote_config, - segment=another_segment, - environment=self.environment, - ) - - # 1 with the same feature but a different environment - another_environment = Environment.objects.create( - name="Another environment", project=self.project - ) - feature_segment_3 = FeatureSegment.objects.create( - feature=self.remote_config, - segment=self.segment, - environment=another_environment, - ) - - # 1 with the same environment but a different feature - another_feature = Feature.objects.create( - name="Another feature", project=self.project - ) - feature_segment_4 = FeatureSegment.objects.create( - feature=another_feature, segment=self.segment, environment=self.environment - ) - - # 1 with a different feature and a different environment - feature_segment_5 = FeatureSegment.objects.create( - feature=another_feature, - segment=self.segment, - environment=another_environment, - ) - - # Then - # the two with the same feature and environment are created with ascending priorities - assert feature_segment_1.priority == 0 - assert feature_segment_2.priority == 1 - - # the ones with different combinations of features and environments are all created with a priority of 0 - assert feature_segment_3.priority == 0 - assert feature_segment_4.priority == 0 - assert feature_segment_5.priority == 0 - - def test_clone_creates_a_new_object(self): - # Given - feature_segment = FeatureSegment.objects.create( - feature=self.remote_config, - segment=self.segment, - environment=self.environment, - priority=1, - ) - new_env = Environment.objects.create( - name="Test environment New", project=self.project - ) - - # When - feature_segment_clone = feature_segment.clone(new_env) - - # Then - assert feature_segment_clone.id != feature_segment.id - assert feature_segment_clone.priority == feature_segment.priority - assert feature_segment_clone.environment.id == new_env.id +from segments.models import Segment + + +def test_feature_segment_is_less_than_other_if_priority_lower( + feature: Feature, + environment: Environment, + segment: Segment, + project: Project, +) -> None: + # Given + feature_segment_1 = FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=environment, + priority=1, + ) + + another_segment = Segment.objects.create(name="Another segment", project=project) + feature_segment_2 = FeatureSegment.objects.create( + feature=feature, + segment=another_segment, + environment=environment, + priority=2, + ) + + # When + result = feature_segment_2 < feature_segment_1 + + # Then + assert result is True + + +def test_feature_segments_are_created_with_correct_priority( + feature: Feature, + environment: Environment, + segment: Segment, + project: Project, +) -> None: + # Given - 5 feature segments + + # 2 with the same feature and environment but a different segment + another_segment = Segment.objects.create(name="Another segment", project=project) + feature_segment_1 = FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=environment, + ) + + feature_segment_2 = FeatureSegment.objects.create( + feature=feature, + segment=another_segment, + environment=environment, + ) + + # 1 with the same feature but a different environment + another_environment = Environment.objects.create( + name="Another environment", project=project + ) + feature_segment_3 = FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=another_environment, + ) + + # 1 with the same environment but a different feature + another_feature = Feature.objects.create(name="Another feature", project=project) + feature_segment_4 = FeatureSegment.objects.create( + feature=another_feature, segment=segment, environment=environment + ) + + # 1 with a different feature and a different environment + feature_segment_5 = FeatureSegment.objects.create( + feature=another_feature, + segment=segment, + environment=another_environment, + ) + + # Then + # the two with the same feature and environment are created with ascending priorities + assert feature_segment_1.priority == 0 + assert feature_segment_2.priority == 1 + + # the ones with different combinations of features and environments are all created with a priority of 0 + assert feature_segment_3.priority == 0 + assert feature_segment_4.priority == 0 + assert feature_segment_5.priority == 0 + + +def test_clone_creates_a_new_object( + feature: Feature, + environment: Environment, + segment: Segment, + project: Project, +) -> None: + # Given + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=environment, + priority=1, + ) + new_environment = Environment.objects.create( + name="Test environment New", project=project + ) + + # When + feature_segment_clone = feature_segment.clone(new_environment) + + # Then + assert feature_segment_clone.id != feature_segment.id + assert feature_segment_clone.priority == feature_segment.priority + assert feature_segment_clone.environment.id == new_environment.id diff --git a/api/tests/unit/features/test_unit_feature_external_resources_views.py b/api/tests/unit/features/test_unit_feature_external_resources_views.py index 6df4cc47393b..1cd5ea031a53 100644 --- a/api/tests/unit/features/test_unit_feature_external_resources_views.py +++ b/api/tests/unit/features/test_unit_feature_external_resources_views.py @@ -1,10 +1,8 @@ -import datetime - import pytest import simplejson as json from django.core.serializers.json import DjangoJSONEncoder from django.urls import reverse -from freezegun import freeze_time +from pytest_mock import MockerFixture from rest_framework import status from rest_framework.test import APIClient @@ -37,24 +35,44 @@ def json(self): return MockResponse(json_data={"data": "data"}, status_code=200) -@freeze_time("2024-01-01") def test_create_feature_external_resource( admin_client_new: APIClient, feature_with_value: Feature, + segment_featurestate_and_feature_with_value: FeatureState, + environment: Environment, project: Project, github_configuration: GithubConfiguration, github_repository: GithubRepository, - mocker, + mocker: MockerFixture, ) -> None: # Given mock_generate_token = mocker.patch( "integrations.github.github.generate_token", ) + mock_generate_token.return_value = "mocked_token" github_request_mock = mocker.patch( "requests.post", side_effect=mocked_requests_post ) - datetime_now = datetime.datetime.now() + + feature_state = FeatureState.objects.filter(feature=feature_with_value).first() + + expected_comment_body = ( + "**Flagsmith feature linked:** `feature_with_value`\n" + "Default Values:\n" + "| Environment | Enabled | Value | Last Updated (UTC) |\n" + "| :--- | :----- | :------ | :------ |\n" + f"| [Test Environment](https://example.com/project/{project.id}/" + f"environment/{environment.api_key}/features?feature={feature_with_value.id}&tab=value) " + f"| ❌ Disabled | `value` | {feature_state.updated_at} |\n" + "\n" + "Segment `segment` values:\n" + "| Environment | Enabled | Value | Last Updated (UTC) |\n" + "| :--- | :----- | :------ | :------ |\n" + f"| [Test Environment](https://example.com/project/{project.id}/" + f"environment/{environment.api_key}/features?feature={feature_with_value.id}&tab=segment-overrides) " + f"| ❌ Disabled | `value` | {segment_featurestate_and_feature_with_value.updated_at} |\n" + ) feature_external_resource_data = { "type": "GITHUB_ISSUE", @@ -76,9 +94,7 @@ def test_create_feature_external_resource( # Then github_request_mock.assert_called_with( "https://api.github.com/repos/repoowner/repo-name/issues/35/comments", - json={ - "body": f"### This pull request is linked to a Flagsmith Feature (`feature_with_value`):\n**Test Environment**\n- [ ] Disabled\nunicode\n```value```\n\nLast Updated {datetime_now.strftime('%dth %b %Y %I:%M%p')}" # noqa E501 - }, + json={"body": f"{expected_comment_body}"}, headers={ "Accept": "application/vnd.github.v3+json", "Authorization": "Bearer mocked_token", @@ -320,9 +336,10 @@ def test_create_github_comment_on_feature_state_updated( # noqa: C901 with_environment_permissions: WithEnvironmentPermissionsCallable, feature_external_resource: FeatureExternalResource, feature: Feature, + project: Project, github_configuration: GithubConfiguration, github_repository: GithubRepository, - mocker, + mocker: MockerFixture, environment: Environment, event_type: str, ) -> None: @@ -412,4 +429,5 @@ def test_create_github_comment_on_feature_state_updated( # noqa: C901 feature_name=feature.name, type=WebhookEventType.FLAG_UPDATED.value, feature_states=[feature_state], + project_id=project.id, ) diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index e63ac8cb6e9d..c579d1ab0af9 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -13,6 +13,7 @@ from freezegun import freeze_time from pytest_django import DjangoAssertNumQueries from pytest_django.fixtures import SettingsWrapper +from pytest_lazyfixture import lazy_fixture from pytest_mock import MockerFixture from rest_framework import status from rest_framework.test import APIClient @@ -36,6 +37,7 @@ from features.multivariate.models import MultivariateFeatureOption from features.value_types import BOOLEAN, INTEGER, STRING from features.versioning.models import EnvironmentFeatureVersion +from metadata.models import MetadataModelField from organisations.models import Organisation, OrganisationRole from projects.models import Project, UserProjectPermission from projects.permissions import CREATE_FEATURE, VIEW_PROJECT @@ -2294,6 +2296,138 @@ def test_cannot_update_feature_of_a_feature_state( ) +@pytest.mark.parametrize( + "client", + [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")], +) +def test_create_feature_without_required_metadata_returns_400( + project: Project, + client: APIClient, + required_a_feature_metadata_field: MetadataModelField, +) -> None: + # Given + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + description = "This is the description" + data = { + "name": "Test feature", + "description": description, + } + + # When + response = client.post(url, data=json.dumps(data), content_type="application/json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +@pytest.mark.parametrize( + "client", + [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")], +) +def test_create_feature_with_optional_metadata_returns_201( + project: Project, + client: APIClient, + optional_b_feature_metadata_field: MetadataModelField, +) -> None: + # Given + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + description = "This is the description" + field_value = 10 + data = { + "name": "Test feature", + "description": description, + "metadata": [ + { + "model_field": optional_b_feature_metadata_field.id, + "field_value": field_value, + }, + ], + } + + # When + response = client.post(url, data=json.dumps(data), content_type="application/json") + + # Then + assert response.status_code == status.HTTP_201_CREATED + assert ( + response.json()["metadata"][0]["model_field"] + == optional_b_feature_metadata_field.id + ) + assert response.json()["metadata"][0]["field_value"] == str(field_value) + + +@pytest.mark.parametrize( + "client", + [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")], +) +def test_create_feature_with_required_metadata_returns_201( + project: Project, + client: APIClient, + required_a_feature_metadata_field: MetadataModelField, +) -> None: + # Given + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + description = "This is the description" + field_value = 10 + data = { + "name": "Test feature", + "description": description, + "metadata": [ + { + "model_field": required_a_feature_metadata_field.id, + "field_value": field_value, + }, + ], + } + + # When + response = client.post(url, data=json.dumps(data), content_type="application/json") + + # Then + assert response.status_code == status.HTTP_201_CREATED + assert ( + response.json()["metadata"][0]["model_field"] + == required_a_feature_metadata_field.id + ) + assert response.json()["metadata"][0]["field_value"] == str(field_value) + + +@pytest.mark.parametrize( + "client", + [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")], +) +def test_create_feature_with_required_metadata_using_organisation_content_typereturns_201( + project: Project, + client: APIClient, + required_a_feature_metadata_field_using_organisation_content_type: MetadataModelField, +) -> None: + # Given + url = reverse("api-v1:projects:project-features-list", args=[project.id]) + description = "This is the description" + field_value = 10 + data = { + "name": "Test feature", + "description": description, + "metadata": [ + { + "model_field": required_a_feature_metadata_field_using_organisation_content_type.id, + "field_value": field_value, + }, + ], + } + + # When + response = client.post(url, data=json.dumps(data), content_type="application/json") + + # Then + assert response.status_code == status.HTTP_201_CREATED + assert ( + response.json()["metadata"][0]["model_field"] + == required_a_feature_metadata_field_using_organisation_content_type.id + ) + assert response.json()["metadata"][0]["field_value"] == str(field_value) + + def test_create_segment_override__using_simple_feature_state_viewset__allows_manage_segment_overrides( staff_client: APIClient, with_environment_permissions: WithEnvironmentPermissionsCallable, @@ -2466,7 +2600,7 @@ def test_list_features_n_plus_1( v1_feature_state.clone(env=environment, version=i, live_from=timezone.now()) # When - with django_assert_num_queries(16): + with django_assert_num_queries(17): response = staff_client.get(url) # Then @@ -2674,7 +2808,7 @@ def test_list_features_with_feature_state( url = f"{base_url}?environment={environment.id}" # When - with django_assert_num_queries(16): + with django_assert_num_queries(17): response = staff_client.get(url) # Then @@ -2968,7 +3102,7 @@ def test_feature_list_last_modified_values( Feature.objects.create(name=f"feature_{i}", project=project) # When - with django_assert_num_queries(18): # TODO: reduce this number of queries! + with django_assert_num_queries(19): # TODO: reduce this number of queries! response = staff_client.get(url) # Then diff --git a/api/tests/unit/features/versioning/test_unit_versioning_tasks.py b/api/tests/unit/features/versioning/test_unit_versioning_tasks.py index f5569b31cfa6..dd587e24bf51 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_tasks.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_tasks.py @@ -12,6 +12,7 @@ from features.versioning.versioning_service import ( get_environment_flags_queryset, ) +from projects.models import Project from segments.models import Segment from users.models import FFAdminUser from webhooks.webhooks import WebhookEventType @@ -37,6 +38,7 @@ def test_enable_v2_versioning( def test_disable_v2_versioning( environment_v2_versioning: Environment, + project: Project, feature: Feature, segment: Segment, staff_user: FFAdminUser, @@ -85,6 +87,21 @@ def test_disable_v2_versioning( environment=environment_v2_versioning, ) + # Finally, let's create another environment and confirm its + # feature states are unaffected. + unaffected_environment = Environment.objects.create( + name="Unaffected environment", project=project + ) + FeatureState.objects.create( + feature=feature, + environment=unaffected_environment, + feature_segment=FeatureSegment.objects.create( + segment=segment, + feature=feature, + environment=unaffected_environment, + ), + ) + # When disable_v2_versioning(environment_v2_versioning.id) environment_v2_versioning.refresh_from_db() @@ -114,6 +131,9 @@ def test_disable_v2_versioning( is True ) + assert unaffected_environment.feature_states.count() == 2 + assert unaffected_environment.feature_segments.count() == 1 + def test_trigger_update_version_webhooks( environment_v2_versioning: Environment, feature: Feature, mocker: MockerFixture diff --git a/api/tests/unit/metadata/test_views.py b/api/tests/unit/metadata/test_views.py index 18680ce0d07d..2dc750d0d3f5 100644 --- a/api/tests/unit/metadata/test_views.py +++ b/api/tests/unit/metadata/test_views.py @@ -1,11 +1,19 @@ import json +from itertools import chain from django.contrib.contenttypes.models import ContentType from django.urls import reverse from rest_framework import status +from rest_framework.test import APIClient -from metadata.models import MetadataModelField, MetadataModelFieldRequirement -from metadata.views import METADATA_SUPPORTED_MODELS +from metadata.models import ( + MetadataField, + MetadataModelField, + MetadataModelFieldRequirement, +) +from metadata.views import SUPPORTED_REQUIREMENTS_MAPPING +from organisations.models import Organisation +from projects.models import Project def test_can_create_metadata_field(admin_client, organisation): @@ -267,15 +275,14 @@ def test_can_not_update_model_metadata_field_from_other_organisation( assert response.status_code == status.HTTP_404_NOT_FOUND -def test_create_model_metadata_field( - admin_client, - a_metadata_field, - organisation, - environment, - project_content_type, - environment_content_type, - project, -): +def test_create_model_metadata_field_for_environments( + admin_client: APIClient, + a_metadata_field: MetadataField, + organisation: Organisation, + project_content_type: ContentType, + environment_content_type: ContentType, + project: Project, +) -> None: # Given url = reverse( "api-v1:organisations:metadata-model-fields-list", args=[organisation.id] @@ -301,6 +308,72 @@ def test_create_model_metadata_field( } +def test_create_model_metadata_field_for_features( + admin_client: APIClient, + a_metadata_field: MetadataField, + organisation: Organisation, + project_content_type: ContentType, + feature_content_type: ContentType, + project: Project, +) -> None: + # Given + url = reverse( + "api-v1:organisations:metadata-model-fields-list", args=[organisation.id] + ) + data = { + "field": a_metadata_field.id, + "is_required_for": [ + {"content_type": project_content_type.id, "object_id": project.id} + ], + "content_type": feature_content_type.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()["field"] == a_metadata_field.id + assert response.json()["is_required_for"][0] == { + "content_type": project_content_type.id, + "object_id": project.id, + } + + +def test_create_model_metadata_field_for_segments( + admin_client: APIClient, + a_metadata_field: MetadataField, + organisation: Organisation, + project_content_type: ContentType, + segment_content_type: ContentType, + project: Project, +) -> None: + # Given + url = reverse( + "api-v1:organisations:metadata-model-fields-list", args=[organisation.id] + ) + data = { + "field": a_metadata_field.id, + "is_required_for": [ + {"content_type": project_content_type.id, "object_id": project.id} + ], + "content_type": segment_content_type.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()["field"] == a_metadata_field.id + assert response.json()["is_required_for"][0] == { + "content_type": project_content_type.id, + "object_id": project.id, + } + + def test_can_not_create_model_metadata_field_using_field_from_other_organisation( admin_client, environment_metadata_field_different_org, organisation, project ): @@ -322,22 +395,31 @@ def test_can_not_create_model_metadata_field_using_field_from_other_organisation assert response.status_code == status.HTTP_403_FORBIDDEN -def test_get_supported_content_type(admin_client, organisation): +def test_get_supported_content_type( + admin_client: APIClient, organisation: Organisation +): # Given url = reverse( "api-v1:organisations:metadata-model-fields-supported-content-types", args=[organisation.id], ) + + supported_models = list( + chain.from_iterable( + (key, *value) for key, value in SUPPORTED_REQUIREMENTS_MAPPING.items() + ) + ) + # When response = admin_client.get(url) # Then assert response.status_code == status.HTTP_200_OK - assert len(response.json()) == len(METADATA_SUPPORTED_MODELS) - assert set(content_type["model"] for content_type in response.json()) == set( - METADATA_SUPPORTED_MODELS - ) + response_models = set(content_type["model"] for content_type in response.json()) + + for model in response_models: + assert model in supported_models def test_get_supported_required_for_models(admin_client, organisation): diff --git a/api/tests/unit/segments/test_unit_segments_permissions.py b/api/tests/unit/segments/test_unit_segments_permissions.py index 5f842d56eeeb..b338c96b1d9f 100644 --- a/api/tests/unit/segments/test_unit_segments_permissions.py +++ b/api/tests/unit/segments/test_unit_segments_permissions.py @@ -1,17 +1,17 @@ import uuid -from unittest import TestCase, mock - -import pytest +from unittest import mock from environments.identities.models import Identity from environments.models import Environment -from environments.permissions.models import UserEnvironmentPermission -from organisations.models import Organisation from permissions.models import PermissionModel from projects.models import Project, UserProjectPermission from projects.permissions import VIEW_PROJECT from segments.models import Segment from segments.permissions import SegmentPermissions +from tests.types import ( + WithEnvironmentPermissionsCallable, + WithProjectPermissionsCallable, +) from users.models import FFAdminUser mock_request = mock.MagicMock() @@ -20,121 +20,153 @@ segment_permissions = SegmentPermissions() -@pytest.mark.django_db -class SegmentPermissionsTestCase(TestCase): - def setUp(self) -> None: - organisation = Organisation.objects.create(name="Test org") - self.project = Project.objects.create( - name="Test project", organisation=organisation - ) - self.segment = Segment.objects.create(name="Test segment", project=self.project) +def test_staff_user_has_permission(staff_user: FFAdminUser, project: Project) -> None: + # Given + mock_request = mock.MagicMock() + mock_request.user = staff_user + mock_request.query_params = {} + mock_view = mock.MagicMock() + mock_view.kwargs = {"project_pk": project.id} + segment_permissions = SegmentPermissions() + + # When + results = [] + for action in ("list", "create"): + mock_view.action = action + results.append(segment_permissions.has_permission(mock_request, mock_view)) - self.project_admin = FFAdminUser.objects.create(email="project_admin@test.com") - mock_view.kwargs = {"project_pk": self.project.id} + # Then + assert all(results) - mock_request.query_params = {} - self.project_user = FFAdminUser.objects.create(email="user@test.com") +def test_project_admin_has_object_permission( + staff_user: FFAdminUser, + project: Project, + with_project_permissions: WithProjectPermissionsCallable, + segment: Segment, +) -> None: + # Given + with_project_permissions(admin=True) + mock_request = mock.MagicMock() + mock_request.user = staff_user + mock_request.query_params = {} + mock_view = mock.MagicMock() + mock_view.kwargs = {"project_pk": project.id} + segment_permissions = SegmentPermissions() - user_project_permissions = UserProjectPermission.objects.create( - project=self.project, user=self.project_user + # When + results = [] + for action in ("update", "destroy", "retrieve"): + mock_view.action = action + results.append( + segment_permissions.has_object_permission(mock_request, mock_view, segment) ) - user_project_permissions.add_permission(VIEW_PROJECT) - def test_project_admin_has_permission(self): - # Given - mock_request.user = self.project_admin + # Then + assert all(results) - # When - results = [] - for action in ("list", "create"): - mock_view.action = action - results.append(segment_permissions.has_permission(mock_request, mock_view)) - # then - assert all(results) +def test_project_user_has_list_permission( + project: Project, + staff_user: FFAdminUser, + with_project_permissions: WithProjectPermissionsCallable, +) -> None: + # Given + mock_request = mock.MagicMock() + mock_request.user = staff_user + mock_request.query_params = {} + mock_view = mock.MagicMock() + mock_view.kwargs = {"project_pk": project.id} + mock_view.detail = False + with_project_permissions([VIEW_PROJECT]) + segment_permissions = SegmentPermissions() - def test_project_admin_has_object_permission(self): - # Given - UserProjectPermission.objects.create( - user=self.project_admin, project=self.project, admin=True - ) - mock_request.user = self.project_admin - - # When - results = [] - for action in ("update", "destroy", "retrieve"): - mock_view.action = action - results.append( - segment_permissions.has_object_permission( - mock_request, mock_view, self.segment - ) - ) - - # then - assert all(results) - - def test_project_user_has_list_permission(self): - # Given - mock_request.user = self.project_user - mock_view.detail = False - - # When - mock_view.action = "list" - result = segment_permissions.has_permission(mock_request, mock_view) + # When + mock_view.action = "list" + result = segment_permissions.has_permission(mock_request, mock_view) + + # Then + assert result is True + + +def test_project_user_has_no_create_permission( + project: Project, + staff_user: FFAdminUser, + with_project_permissions: WithProjectPermissionsCallable, +) -> None: + # Given + mock_request = mock.MagicMock() + mock_request.user = staff_user + mock_request.query_params = {} + mock_view = mock.MagicMock() + mock_view.kwargs = {"project_pk": project.id} + mock_view.detail = False + with_project_permissions([VIEW_PROJECT]) + segment_permissions = SegmentPermissions() + + # When + mock_view.action = "create" + result = segment_permissions.has_permission(mock_request, mock_view) + + # Then + assert result is False - # Then - assert result - def test_project_user_has_no_create_permission(self): - # Given - mock_request.user = self.project_user - mock_view.detail = False +def test_project_user_has_object_permission( + project: Project, + staff_user: FFAdminUser, + with_project_permissions: WithProjectPermissionsCallable, + segment: Segment, +) -> None: + # Given + mock_request = mock.MagicMock() + mock_request.user = staff_user + mock_request.query_params = {} + mock_view = mock.MagicMock() + mock_view.kwargs = {"project_pk": project.id} + with_project_permissions([VIEW_PROJECT]) + segment_permissions = SegmentPermissions() - # When - mock_view.action = "create" - result = segment_permissions.has_permission(mock_request, mock_view) + # When + for action, expected_result in ( + ("retrieve", True), + ("destroy", False), + ("update", False), + ("partial_update", False), + ): + mock_view.action = action # Then - assert not result - - def test_project_user_has_object_permission(self): - # Given - mock_request.user = self.project_user - - # When - for action, expected_result in ( - ("retrieve", True), - ("destroy", False), - ("update", False), - ("partial_update", False), - ): - mock_view.action = action - # Then - assert ( - segment_permissions.has_object_permission( - mock_request, mock_view, self.segment - ) - == expected_result - ) - - def test_environment_admin_can_get_segments_for_an_identity(self): - # Given - environment = Environment.objects.create( - name="Test environment", project=self.project + assert ( + segment_permissions.has_object_permission(mock_request, mock_view, segment) + == expected_result ) - identity = Identity.objects.create(identifier="test", environment=environment) - user = FFAdminUser.objects.create(email="environment_admin@test.com") - UserEnvironmentPermission.objects.create( - user=user, admin=True, environment=environment - ) - mock_request.query_params["identity"] = identity.id - # When - result = segment_permissions.has_permission(mock_request, mock_view) - # Then - assert result +def test_environment_admin_can_get_segments_for_an_identity( + project: Project, + staff_user: FFAdminUser, + with_environment_permissions: WithEnvironmentPermissionsCallable, + segment: Segment, + environment: Environment, + identity: Identity, +) -> None: + # Given + mock_request = mock.MagicMock() + mock_request.user = staff_user + mock_request.query_params = {} + mock_view = mock.MagicMock() + mock_view.kwargs = {"project_pk": project.id} + with_environment_permissions(admin=True) + identity = Identity.objects.create(identifier="test", environment=environment) + mock_request.query_params["identity"] = identity.id + segment_permissions = SegmentPermissions() + + # When + result = segment_permissions.has_permission(mock_request, mock_view) + + # Then + assert result def test_user_with_view_project_permission_can_list_segments_for_an_identity( diff --git a/api/tests/unit/segments/test_unit_segments_views.py b/api/tests/unit/segments/test_unit_segments_views.py index d990bc4dc4f2..74fa7ae22127 100644 --- a/api/tests/unit/segments/test_unit_segments_views.py +++ b/api/tests/unit/segments/test_unit_segments_views.py @@ -15,6 +15,7 @@ from audit.related_object_type import RelatedObjectType from environments.models import Environment from features.models import Feature +from metadata.models import MetadataModelField from projects.models import Project from projects.permissions import MANAGE_SEGMENTS, VIEW_PROJECT from segments.models import Condition, Segment, SegmentRule, WhitelistedSegment @@ -336,8 +337,8 @@ def test_get_segment_by_uuid(client, project, segment): @pytest.mark.parametrize( "client, num_queries", [ - (lazy_fixture("admin_master_api_key_client"), 11), - (lazy_fixture("admin_client"), 10), + (lazy_fixture("admin_master_api_key_client"), 16), + (lazy_fixture("admin_client"), 15), ], ) def test_list_segments(django_assert_num_queries, project, client, num_queries): @@ -608,6 +609,108 @@ def test_update_segment_delete_existing_rule(project, client, segment, segment_r assert segment_rule.conditions.count() == 0 +@pytest.mark.parametrize( + "client", + [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")], +) +def test_create_segment_with_required_metadata_returns_201( + project: Project, + client: APIClient, + required_a_segment_metadata_field: MetadataModelField, +) -> None: + # Given + url = reverse("api-v1:projects:project-segments-list", args=[project.id]) + description = "This is the description" + field_value = 10 + data = { + "name": "Test Segment", + "description": description, + "project": project.id, + "rules": [{"type": "ALL", "rules": [], "conditions": []}], + "metadata": [ + { + "model_field": required_a_segment_metadata_field.id, + "field_value": field_value, + }, + ], + } + + # When + response = client.post(url, data=json.dumps(data), content_type="application/json") + + # Then + assert response.status_code == status.HTTP_201_CREATED + assert ( + response.json()["metadata"][0]["model_field"] + == required_a_segment_metadata_field.id + ) + assert response.json()["metadata"][0]["field_value"] == str(field_value) + + +@pytest.mark.parametrize( + "client", + [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")], +) +def test_create_segment_with_required_metadata_using_organisation_content_type_returns_201( + project: Project, + client: APIClient, + required_a_segment_metadata_field_using_organisation_content_type: MetadataModelField, +) -> None: + # Given + url = reverse("api-v1:projects:project-segments-list", args=[project.id]) + description = "This is the description" + field_value = 10 + data = { + "name": "Test Segment", + "description": description, + "project": project.id, + "rules": [{"type": "ALL", "rules": [], "conditions": []}], + "metadata": [ + { + "model_field": required_a_segment_metadata_field_using_organisation_content_type.id, + "field_value": field_value, + }, + ], + } + + # When + response = client.post(url, data=json.dumps(data), content_type="application/json") + + # Then + assert response.status_code == status.HTTP_201_CREATED + assert ( + response.json()["metadata"][0]["model_field"] + == required_a_segment_metadata_field_using_organisation_content_type.id + ) + assert response.json()["metadata"][0]["field_value"] == str(field_value) + + +@pytest.mark.parametrize( + "client", + [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")], +) +def test_create_segment_without_required_metadata_returns_400( + project: Project, + client: APIClient, + required_a_segment_metadata_field: MetadataModelField, +) -> None: + # Given + url = reverse("api-v1:projects:project-segments-list", args=[project.id]) + description = "This is the description" + data = { + "name": "Test Segment", + "description": description, + "project": project.id, + "rules": [{"type": "ALL", "rules": [], "conditions": []}], + } + + # When + response = client.post(url, data=json.dumps(data), content_type="application/json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + + def test_update_segment_obeys_max_conditions( project: Project, admin_client: APIClient, @@ -683,6 +786,44 @@ def test_update_segment_obeys_max_conditions( assert nested_rule.conditions.count() == 1 +@pytest.mark.parametrize( + "client", + [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")], +) +def test_create_segment_with_optional_metadata_returns_201( + project: Project, + client: APIClient, + optional_b_segment_metadata_field: MetadataModelField, +) -> None: + # Given + url = reverse("api-v1:projects:project-segments-list", args=[project.id]) + description = "This is the description" + field_value = 10 + data = { + "name": "Test Segment", + "description": description, + "project": project.id, + "rules": [{"type": "ALL", "rules": [], "conditions": []}], + "metadata": [ + { + "model_field": optional_b_segment_metadata_field.id, + "field_value": field_value, + }, + ], + } + + # When + response = client.post(url, data=json.dumps(data), content_type="application/json") + + # Then + assert response.status_code == status.HTTP_201_CREATED + assert ( + response.json()["metadata"][0]["model_field"] + == optional_b_segment_metadata_field.id + ) + assert response.json()["metadata"][0]["field_value"] == str(field_value) + + def test_update_segment_evades_max_conditions_when_whitelisted( project: Project, admin_client: APIClient, @@ -809,7 +950,6 @@ def test_create_segment_obeys_max_conditions( assert response.json() == { "segment": "The segment has 11 conditions, which exceeds the maximum condition count of 10." } - assert Segment.objects.count() == 0 diff --git a/api/tests/unit/util/mappers/test_unit_mappers_sdk.py b/api/tests/unit/util/mappers/test_unit_mappers_sdk.py new file mode 100644 index 000000000000..7363f1e840db --- /dev/null +++ b/api/tests/unit/util/mappers/test_unit_mappers_sdk.py @@ -0,0 +1,108 @@ +from typing import TYPE_CHECKING + +import pytest + +from environments.identities.models import Identity +from util.mappers.sdk import map_environment_to_sdk_document + +if TYPE_CHECKING: # pragma: no cover + from pytest_mock import MockerFixture + + from environments.models import Environment + from features.models import FeatureState + + +@pytest.fixture() +def identity_without_overrides(environment): + return Identity.objects.create( + identifier="test_identity_without_overrides", environment=environment + ) + + +def test_map_environment_to_sdk_document__return_expected( + mocker: "MockerFixture", + environment: "Environment", + feature_state: "FeatureState", + identity: Identity, + identity_featurestate: "FeatureState", + identity_without_overrides: Identity, +) -> None: + # Given + expected_overridden_value = "some-overridden-value" + expected_api_key = environment.api_key + + identity_featurestate.feature_state_value.string_value = expected_overridden_value + identity_featurestate.feature_state_value.save() + identity_featurestate.enabled = True + identity_featurestate.save() + + # When + result = map_environment_to_sdk_document(environment) + + # Then + assert result == { + "allow_client_traits": True, + "api_key": expected_api_key, + "feature_states": [ + { + "django_id": feature_state.pk, + "enabled": False, + "feature": { + "id": feature_state.feature.pk, + "name": "Test Feature1", + "type": "STANDARD", + }, + "feature_segment": None, + "feature_state_value": None, + "featurestate_uuid": feature_state.uuid, + "multivariate_feature_state_values": [], + } + ], + "identity_overrides": [ + { + "composite_key": identity.composite_key, + "created_date": identity.created_date, + "django_id": identity.pk, + "environment_api_key": expected_api_key, + "identifier": identity.identifier, + "identity_features": [ + { + "django_id": identity_featurestate.pk, + "enabled": True, + "feature": { + "id": identity_featurestate.feature.pk, + "name": identity_featurestate.feature.name, + "type": identity_featurestate.feature.type, + }, + "feature_segment": None, + "feature_state_value": expected_overridden_value, + "featurestate_uuid": identity_featurestate.uuid, + "multivariate_feature_state_values": [], + } + ], + "identity_traits": [], + "identity_uuid": mocker.ANY, + } + ], + "hide_disabled_flags": None, + "hide_sensitive_data": False, + "id": environment.pk, + "name": "Test Environment", + "project": { + "enable_realtime_updates": False, + "hide_disabled_flags": False, + "id": environment.project.pk, + "name": "Test Project", + "organisation": { + "feature_analytics": False, + "id": environment.project.organisation.pk, + "name": "Test Org", + "persist_trait_data": True, + "stop_serving_flags": False, + }, + "segments": [], + "server_key_only_feature_ids": [], + }, + "updated_at": environment.updated_at, + "use_identity_composite_key_for_hashing": True, + } diff --git a/api/util/mappers/__init__.py b/api/util/mappers/__init__.py index 995f02308be2..6b5f3d2a7f41 100644 --- a/api/util/mappers/__init__.py +++ b/api/util/mappers/__init__.py @@ -14,6 +14,7 @@ map_identity_to_engine, map_mv_option_to_engine, ) +from util.mappers.sdk import map_environment_to_sdk_document __all__ = ( "map_engine_feature_state_to_identity_override", @@ -21,6 +22,7 @@ "map_environment_api_key_to_environment_api_key_document", "map_environment_to_environment_document", "map_environment_to_environment_v2_document", + "map_environment_to_sdk_document", "map_feature_state_to_engine", "map_feature_to_engine", "map_identity_changeset_to_identity_override_changeset", diff --git a/api/util/mappers/dynamodb.py b/api/util/mappers/dynamodb.py index 95a0850f9d70..b4da6bcdb903 100644 --- a/api/util/mappers/dynamodb.py +++ b/api/util/mappers/dynamodb.py @@ -48,6 +48,7 @@ def map_environment_to_environment_document( field_name: _map_value_to_document_value(value) for field_name, value in map_environment_to_engine( environment, + with_integrations=True, ) } diff --git a/api/util/mappers/engine.py b/api/util/mappers/engine.py index 5730dc9846c2..243b1fff6eec 100644 --- a/api/util/mappers/engine.py +++ b/api/util/mappers/engine.py @@ -24,7 +24,9 @@ SegmentRuleModel, ) -if TYPE_CHECKING: +from environments.constants import IDENTITY_INTEGRATIONS_RELATION_NAMES + +if TYPE_CHECKING: # pragma: no cover from environments.identities.models import Identity, Trait from environments.models import Environment, EnvironmentAPIKey from features.models import Feature, FeatureSegment, FeatureState @@ -174,6 +176,8 @@ def map_mv_option_to_engine( def map_environment_to_engine( environment: "Environment", + *, + with_integrations: bool = True, ) -> EnvironmentModel: """ Maps Core API's `environments.models.Environment` model instance to the @@ -215,22 +219,14 @@ def map_environment_to_engine( } # Read integrations. - integration_configs: dict[str, Optional["EnvironmentIntegrationModel"]] = {} - for attr_name in ( - "amplitude_config", - "dynatrace_config", - "heap_config", - "mixpanel_config", - "rudderstack_config", - "segment_config", - ): - integration_config = getattr(environment, attr_name, None) - if integration_config and not integration_config.deleted: - integration_configs[attr_name] = integration_config - - webhook_config: Optional["WebhookConfiguration"] = getattr( - environment, "webhook_config", None - ) + integration_configs: dict[ + str, "EnvironmentIntegrationModel | WebhookConfiguration | None" + ] = {} + if with_integrations: + for attr_name in IDENTITY_INTEGRATIONS_RELATION_NAMES: + integration_config = getattr(environment, attr_name, None) + if integration_config and not integration_config.deleted: + integration_configs[attr_name] = integration_config # No reading from ORM past this point! @@ -291,9 +287,6 @@ def map_environment_to_engine( amplitude_config_model = map_integration_to_engine( integration_configs.pop("amplitude_config", None), ) - dynatrace_config_model = map_integration_to_engine( - integration_configs.pop("dynatrace_config", None), - ) heap_config_model = map_integration_to_engine( integration_configs.pop("heap_config", None), ) @@ -306,13 +299,8 @@ def map_environment_to_engine( segment_config_model = map_integration_to_engine( integration_configs.pop("segment_config", None), ) - - webhook_config_model = ( - map_webhook_config_to_engine( - webhook_config, - ) - if webhook_config and not webhook_config.deleted - else None + webhook_config_model = map_webhook_config_to_engine( + integration_configs.pop("webhook_config", None), ) return EnvironmentModel( @@ -333,7 +321,6 @@ def map_environment_to_engine( # # Integrations: amplitude_config=amplitude_config_model, - dynatrace_config=dynatrace_config_model, heap_config=heap_config_model, mixpanel_config=mixpanel_config_model, rudderstack_config=rudderstack_config_model, diff --git a/api/util/mappers/sdk.py b/api/util/mappers/sdk.py new file mode 100644 index 000000000000..bc94def7ae8f --- /dev/null +++ b/api/util/mappers/sdk.py @@ -0,0 +1,51 @@ +from typing import TYPE_CHECKING, TypeAlias + +from environments.constants import IDENTITY_INTEGRATIONS_RELATION_NAMES +from util.mappers.engine import ( + map_environment_to_engine, + map_identity_to_engine, +) + +if TYPE_CHECKING: # pragma: no cover + from environments.models import Environment + + +SDKDocumentValue: TypeAlias = dict[str, "SDKDocumentValue"] | str | bool | None | float +SDKDocument: TypeAlias = dict[str, SDKDocumentValue] + +SDK_DOCUMENT_EXCLUDE = [ + *IDENTITY_INTEGRATIONS_RELATION_NAMES, + "dynatrace_config", +] + + +def map_environment_to_sdk_document(environment: "Environment") -> SDKDocument: + """ + Map an `environments.models.Environment` instance to an SDK document + used by SDKs with local evaluation mode. + + It's virtually the same data that gets indexed in DynamoDB, + except it presents identity overrides and omits integrations configurations. + """ + # Read relationships. + identities_with_overrides = {} + for feature_state in environment.feature_states.all(): + if (identity_id := feature_state.identity_id) and ( + identity_id not in identities_with_overrides + ): + identities_with_overrides[identity_id] = feature_state.identity + + # Get the engine data. + engine_environment = map_environment_to_engine(environment, with_integrations=False) + + # No reading from ORM past this point! + + # Prepare relationships. + engine_environment.identity_overrides = [ + map_identity_to_engine(identity, with_traits=False) + for identity in identities_with_overrides.values() + ] + + return engine_environment.model_dump( + exclude=SDK_DOCUMENT_EXCLUDE, + ) diff --git a/docs/docs/deployment/hosting/kubernetes.md b/docs/docs/deployment/hosting/kubernetes.md index 74da289e0b74..2ae3e0a04357 100644 --- a/docs/docs/deployment/hosting/kubernetes.md +++ b/docs/docs/deployment/hosting/kubernetes.md @@ -381,7 +381,7 @@ The following table lists the configurable parameters of the chart and their def | `taskProcessor.affinity` | | `{}` | | `taskProcessor.podSecurityContext` | | `{}` | | `taskProcessor.defaultPodSecurityContext.enabled` | whether to use the default security context | `true` | -| `postgresql.enabled` | if `true`, creates in-cluster PostgreSQL database | `true` | +| `devPostgresql.enabled` | if `true`, creates in-cluster PostgreSQL database | `true` | | `postgresql.serviceAccount.enabled` | creates a serviceaccount for the postgres pod | `true` | | `nameOverride` | | `flagsmith-postgres` | | `postgresqlDatabase` | | `flagsmith` | @@ -478,7 +478,7 @@ The following table lists the configurable parameters of the chart and their def ## Key upgrade notes -- [0.20.0](https://artifacthub.io/packages/helm/flagsmith/flagsmith/0.20.0): upgrades the bundled in-cluster Postgres. +- [0.37.0](https://artifacthub.io/packages/helm/flagsmith/flagsmith/0.37.0): upgrades the bundled in-cluster Postgres. This makes no effort to preserve data in the bundled in-cluster Postgres if it is in use. This also renames the bundled in-cluster Postgres to have `dev-postgresql` in the name, to signify that it exists such that the chart can be deployed self-contained, but that this Postgres instance is treated as disposable. All Flagsmith installations for diff --git a/docs/docs/deployment/overview.md b/docs/docs/deployment/overview.md index 1133f837dd20..d4406e545ecc 100644 --- a/docs/docs/deployment/overview.md +++ b/docs/docs/deployment/overview.md @@ -185,21 +185,14 @@ The list of the flags and remote config we're currently using in production is b | Flag Name | Description | Text Value | | ------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------- | -| `4eyes` | Whether to enable the Change Requests feature | None | | `announcement` | Shows an announcement at the top of the app | None | | `butter_bar` | Show html in a butter bar for certain users | None | -| `case_sensitive_flags` | Enables the project setting to allow case sensitive flags | None | -| `compare_environments` | Compare feature flag changes across environments | None | -| `configure_hide_sensitive_data` | If the value is true, the hide sensitive data switch will be displayed in the environment settings. | None | | `dark_mode` | Enables Dark Mode in UI See Below | None | | `default_environment_names_for_new_project` | Names of default environments to create when creating a new project (e.g. `["Development", "Production"]`) | None | | `disable_create_org` | Turning this on will prevent users from creating any additional organisations | None | | `disable_users_as_reviewers` | If enabled, this flag will hide the Assigned users section in the Change Requests and in the Create Change Request modal in the Features page. | None | | `enable_metadata` | If enabled, metadata can be handled | None | -| `feature_name_regex` | Enables the project setting to add a regex matcher to validate feature names | None | | `feature_versioning` | Opt into feature versioning for your environment | None | -| `flag_analytics` | Flag usage chart - requires additional infrastructure ([See here](/deployment/overview#flag-analytics)) | None | -| `force_2fa` | Enables the organisation setting to force 2 factor authentication | None | | `integration_data` | Integration config for different providers | [See Below](#integration_data) | | `mailing_list` | Determines if mailing list consent is shown on signup | None | | `max_api_calls_alert` | If enabled, shows an alert message in the top banner when the organization is over a 70% of its API calls limit | None | @@ -207,15 +200,9 @@ The list of the flags and remote config we're currently using in production is b | `oauth_google` | Google login key | [See Below](#oauth_google) | | `payments_enabled` | Determines whether to show payment UI / seats | None | | `plan_based_access` | Controls rbac and 2f based on plans | None | -| `rotate_api_token` | Enables the ability to rotate a user's access token | [See Below](#oauth_google) | | `saml` | Enables SAML authentication | [See](/system-administration/authentication/SAML) | -| `segment_associated_features` | Enables the ability to see features associated with a segment | None | | `segment_operators` | Determines what rules are shown when creating a segment | [See Below](#segment_operators) | -| `serverside_sdk_keys` | Enable Server-side Environment Keys | None | -| `show_role_management` | Show role management tab in OrganisationalSettingsPage | None | | `sso_idp` | For self hosted, this will automatically redirect to the pre configured IdP | None | -| `tag_environments` | Enables an environment setting to add a UI hint to your environments (e.g. for prod) | None | -| `usage_chart` | Organisation Analytics usage chart - | None | | `verify_seats_limit_for_invite_links` | Determines whether to show los invite links | None | ### `integration_data` @@ -225,7 +212,7 @@ The list of the flags and remote config we're currently using in production is b "datadog": { "perEnvironment": false, "image": "/static/images/integrations/datadog.svg", - "docs": "https://docs.flagsmith.com/integrations/apm/datadog/", + "docs": "https://docs.flagsmith.com/integrations/apm/datadog", "fields": [ { "key": "base_url", @@ -235,6 +222,12 @@ The list of the flags and remote config we're currently using in production is b "key": "api_key", "label": "API Key", "hidden": true + }, + { + "key": "use_custom_source", + "label": "Use Custom Source", + "inputType": "checkbox", + "default": true } ], "tags": ["logging"], @@ -244,7 +237,7 @@ The list of the flags and remote config we're currently using in production is b "dynatrace": { "perEnvironment": true, "image": "/static/images/integrations/dynatrace.svg", - "docs": "https://docs.flagsmith.com/integrations/apm/dynatrace/", + "docs": "https://docs.flagsmith.com/integrations/apm/dynatrace", "fields": [ { "key": "base_url", @@ -264,11 +257,20 @@ The list of the flags and remote config we're currently using in production is b "title": "Dynatrace", "description": "Sends events to Dynatrace for when flags are created, updated and removed. Logs are tagged with the environment they came from e.g. production." }, + "jira": { + "perEnvironment": false, + "image": "https://docs.flagsmith.com/img/integrations/jira/jira-logo.svg", + "docs": "https://docs.flagsmith.com/integrations/project-management/jira", + "external": true, + "title": "Jira", + "description": "View your Flagsmith Flags inside Jira." + }, + "slack": { "perEnvironment": true, "isOauth": true, "image": "/static/images/integrations/slack.svg", - "docs": "https://docs.flagsmith.com/integrations/slack/", + "docs": "https://docs.flagsmith.com/integrations/slack", "tags": ["messaging"], "title": "Slack", "description": "Sends messages to Slack when flags are created, updated and removed. Logs are tagged with the environment they came from e.g. production." @@ -276,7 +278,7 @@ The list of the flags and remote config we're currently using in production is b "amplitude": { "perEnvironment": true, "image": "/static/images/integrations/amplitude.svg", - "docs": "https://docs.flagsmith.com/integrations/analytics/amplitude/", + "docs": "https://docs.flagsmith.com/integrations/analytics/amplitude", "fields": [ { "key": "api_key", @@ -388,184 +390,6 @@ The list of the flags and remote config we're currently using in production is b "image": "/static/images/integrations/mp.svg", "docs": "https://docs.flagsmith.com/integrations/analytics/mixpanel", "fields": [ - { - "datadog": { - "perEnvironment": false, - "image": "/static/images/integrations/datadog.svg", - "docs": "https://docs.flagsmith.com/integrations/apm/datadog/", - "fields": [ - { - "key": "base_url", - "label": "Base URL" - }, - { - "key": "api_key", - "label": "API Key", - "hidden": true - } - ], - "tags": ["logging"], - "title": "Datadog", - "description": "Sends events to Datadog for when flags are created, updated and removed. Logs are tagged with the environment they came from e.g. production." - }, - "dynatrace": { - "perEnvironment": true, - "image": "/static/images/integrations/dynatrace.svg", - "docs": "https://docs.flagsmith.com/integrations/apm/dynatrace/", - "fields": [ - { - "key": "base_url", - "label": "Base URL" - }, - { - "key": "api_key", - "label": "API Key", - "hidden": true - }, - { - "key": "entity_selector", - "label": "Entity Selector" - } - ], - "tags": ["logging"], - "title": "Dynatrace", - "description": "Sends events to Dynatrace for when flags are created, updated and removed. Logs are tagged with the environment they came from e.g. production." - }, - "slack": { - "perEnvironment": true, - "isOauth": true, - "image": "/static/images/integrations/slack.svg", - "docs": "https://docs.flagsmith.com/integrations/slack/", - "tags": ["messaging"], - "title": "Slack", - "description": "Sends messages to Slack when flags are created, updated and removed. Logs are tagged with the environment they came from e.g. production." - }, - "amplitude": { - "perEnvironment": true, - "image": "/static/images/integrations/amplitude.svg", - "docs": "https://docs.flagsmith.com/integrations/analytics/amplitude/", - "fields": [ - { - "key": "api_key", - "label": "API Key", - "hidden": true - }, - { - "key": "base_url", - "label": "Base URL" - } - ], - "tags": ["analytics"], - "title": "Amplitude", - "description": "Sends data on what flags served to each identity." - }, - "new-relic": { - "perEnvironment": false, - "image": "/static/images/integrations/new_relic.svg", - "docs": "https://docs.flagsmith.com/integrations/apm/newrelic", - "fields": [ - { - "key": "base_url", - "label": "New Relic Base URL" - }, - { - "key": "api_key", - "label": "New Relic API Key", - "hidden": true - }, - { - "key": "app_id", - "label": "New Relic Application ID" - } - ], - "tags": ["analytics"], - "title": "New Relic", - "description": "Sends events to New Relic for when flags are created, updated and removed." - }, - "segment": { - "perEnvironment": true, - "image": "/static/images/integrations/segment.svg", - "docs": "https://docs.flagsmith.com/integrations/analytics/segment", - "fields": [ - { - "key": "api_key", - "label": "API Key", - "hidden": true - } - ], - "tags": ["analytics"], - "title": "Segment", - "description": "Sends data on what flags served to each identity." - }, - "rudderstack": { - "perEnvironment": true, - "image": "/static/images/integrations/rudderstack.svg", - "docs": "https://docs.flagsmith.com/integrations/analytics/rudderstack", - "fields": [ - { - "key": "base_url", - "label": "Rudderstack Data Plane URL" - }, - { - "key": "api_key", - "label": "API Key", - "hidden": true - } - ], - "tags": ["analytics"], - "title": "Rudderstack", - "description": "Sends data on what flags served to each identity." - }, - "webhook": { - "perEnvironment": true, - "image": "/static/images/integrations/webhooks.svg", - "docs": "https://docs.flagsmith.com/integrations/webhook", - "fields": [ - { - "key": "url", - "label": "Your Webhook URL Endpoint" - }, - { - "key": "secret", - "label": "Your Webhook Secret", - "hidden": true - } - ], - "tags": ["analytics"], - "title": "Webhook", - "description": "Sends data on what flags served to each identity to a Webhook Endpoint you provide." - }, - "heap": { - "perEnvironment": true, - "image": "/static/images/integrations/heap.svg", - "docs": "https://docs.flagsmith.com/integrations/analytics/heap", - "fields": [ - { - "key": "api_key", - "label": "API Key", - "hidden": true - } - ], - "tags": ["analytics"], - "title": "Heap Analytics", - "description": "Sends data on what flags served to each identity." - }, - "mixpanel": { - "perEnvironment": true, - "image": "/static/images/integrations/mp.svg", - "docs": "https://docs.flagsmith.com/integrations/analytics/mixpanel", - "fields": [ - { - "key": "api_key", - "label": "Project Token", - "hidden": true - } - ], - "tags": ["analytics"], - "title": "Mixpanel", - "description": "Sends data on what flags served to each identity." - } - }, { "key": "api_key", "label": "Project Token", @@ -635,6 +459,11 @@ The list of the flags and remote config we're currently using in production is b "label": "SemVer <=", "append": ":semver" }, + { + "value": "MODULO", + "label": "Modulo", + "valuePlaceholder": "Divisor|Remainder" + }, { "value": "CONTAINS", "label": "Contains" diff --git a/frontend/common/constants.ts b/frontend/common/constants.ts index 3438e4f41fdb..aac41d387a82 100644 --- a/frontend/common/constants.ts +++ b/frontend/common/constants.ts @@ -342,6 +342,12 @@ export default { 'category': 'Organisation', 'event': 'Updated user role', }, + UPGRADE: (plan: string) => { + return { + 'category': 'Upgrade', + 'event': `Upgrade ${plan}`, + } + }, 'VIEW_FEATURE': { 'category': 'Features', 'event': 'Feature viewed' }, 'VIEW_SEGMENT': { 'category': 'Segment', 'event': 'Segment viewed' }, 'VIEW_USER_FEATURE': { diff --git a/frontend/common/services/useGroup.ts b/frontend/common/services/useGroup.ts index 46c9f76bf81a..b5b4535d2178 100644 --- a/frontend/common/services/useGroup.ts +++ b/frontend/common/services/useGroup.ts @@ -109,7 +109,7 @@ export const groupService = service const { data, error } = await baseQuery({ body: query.data, method: 'PUT', - url: `organisations/${query.orgId}/groups/${query.data.id}`, + url: `organisations/${query.orgId}/groups/${query.data.id}/`, }) if (error) { return { error } diff --git a/frontend/common/stores/account-store.js b/frontend/common/stores/account-store.js index b14197acc99f..acfefa41ae85 100644 --- a/frontend/common/stores/account-store.js +++ b/frontend/common/stores/account-store.js @@ -307,10 +307,10 @@ const controller = { path: '/organisation/:organisationId', strict: false, })?.params?.organisationId - const orgId = pathID || cookiedID + const orgId = parseInt(pathID || cookiedID) || undefined if (orgId) { const foundOrganisation = user.organisations.find( - (v) => `${v.id}` === orgId, + (v) => v.id === orgId, ) if (foundOrganisation) { store.organisation = foundOrganisation @@ -372,6 +372,7 @@ const controller = { store.model.organisations[idx] = res try { if (res && res.subscription && res.subscription.plan) { + API.trackEvent(Constants.events.UPGRADE(res.subscription.plan)) API.postEvent(res.subscription.plan, 'chargebee') } } catch (e) {} @@ -396,7 +397,6 @@ const store = Object.assign({}, BaseStore, { } return ( - Utils.getFlagsmithHasFeature('force_2fa') && store.getOrganisations() && store.getOrganisations().find((o) => o.force_2fa) ) diff --git a/frontend/common/stores/project-store.js b/frontend/common/stores/project-store.js index 0fe419143906..10bea02bd1bd 100644 --- a/frontend/common/stores/project-store.js +++ b/frontend/common/stores/project-store.js @@ -96,7 +96,12 @@ const controller = { }) }, getProject: (id, cb, force) => { - if (force) { + if (!id) { + if (!getIsWidget()) { + !force && AsyncStorage.removeItem('lastEnv') + document.location.href = '/404' + } + } else if (force) { store.loading() return Promise.all([ diff --git a/frontend/common/utils/utils.tsx b/frontend/common/utils/utils.tsx index bb725e32a8c0..c46f77915a39 100644 --- a/frontend/common/utils/utils.tsx +++ b/frontend/common/utils/utils.tsx @@ -416,10 +416,9 @@ const Utils = Object.assign({}, require('./base/_utils'), { } return !!( - !Utils.getFlagsmithHasFeature('show_edge_identity_overrides') || - (project && - project.use_edge_identities && - !project.show_edge_identity_overrides_for_feature) + project && + project.use_edge_identities && + !project.show_edge_identity_overrides_for_feature ) }, diff --git a/frontend/web/components/AdminAPIKeys.js b/frontend/web/components/AdminAPIKeys.js index fd5351ea3970..22589592733a 100644 --- a/frontend/web/components/AdminAPIKeys.js +++ b/frontend/web/components/AdminAPIKeys.js @@ -30,9 +30,7 @@ export class CreateAPIKey extends PureComponent { } componentDidMount() { - Utils.getFlagsmithHasFeature('show_role_management') && - this.props.isEdit && - this.getApiKeyByPrefix(this.props.prefix) + this.props.isEdit && this.getApiKeyByPrefix(this.props.prefix) } submit = () => { @@ -52,18 +50,17 @@ export class CreateAPIKey extends PureComponent { isSaving: false, key: res.key, }) - Utils.getFlagsmithHasFeature('show_role_management') && - Promise.all( - this.state.roles.map((role) => - createRoleMasterApiKey(getStore(), { - body: { master_api_key: res.id }, - org_id: AccountStore.getOrganisation().id, - role_id: role.id, - }).then(() => { - toast('Role API Key was Created') - }), - ), - ) + Promise.all( + this.state.roles.map((role) => + createRoleMasterApiKey(getStore(), { + body: { master_api_key: res.id }, + org_id: AccountStore.getOrganisation().id, + role_id: role.id, + }).then(() => { + toast('Role API Key was Created') + }), + ), + ) this.props.onSuccess() }) @@ -156,9 +153,6 @@ export class CreateAPIKey extends PureComponent { const { expiry_date, is_admin, roles, showRoles } = this.state const buttonText = this.props.isEdit ? 'Update' : 'Create' const buttonSavingText = this.props.isEdit ? 'Updating' : 'Creating' - const showRoleManagementEnabled = Utils.getFlagsmithHasFeature( - 'show_role_management', - ) return ( <>
@@ -179,76 +173,68 @@ export class CreateAPIKey extends PureComponent { placeholder='e.g. Admin API Key' /> - {showRoleManagementEnabled && ( - <> - - - { - this.setState({ - is_admin: !is_admin, - }) - }} - checked={is_admin} - /> - - {!is_admin && ( - <> - - - {roles?.map((r) => ( - - this.removeRoleApiKey(r.id, this.props.isEdit) - } - className='chip' - > - {r.name} - - - - - ))} - - - - {Utils.getFlagsmithHasFeature( - 'show_role_management', - ) && ( -
- v.id)} - onAdd={(role) => - this.addRole(role, this.props.isEdit) - } - onRemove={(roleId) => - this.removeRoleApiKey(roleId, this.props.isEdit) - } - isOpen={showRoles} - onToggle={() => - this.setState({ showRoles: !showRoles }) - } + {r.name} + + -
- )} - - - )} - - )} + + + ))} + + + +
+ v.id)} + onAdd={(role) => + this.addRole(role, this.props.isEdit) + } + onRemove={(roleId) => + this.removeRoleApiKey(roleId, this.props.isEdit) + } + isOpen={showRoles} + onToggle={() => + this.setState({ showRoles: !showRoles }) + } + /> +
+ + + )} +
@@ -406,9 +392,6 @@ export default class AdminAPIKeys extends PureComponent { render() { const apiKeys = this.state.apiKeys && this.state.apiKeys.results - const showRoleManagementEnabled = Utils.getFlagsmithHasFeature( - 'show_role_management', - ) return (
-
{`${ - showRoleManagementEnabled ? 'Manage' : 'Terraform' - } API Keys`}
+
{`${'Manage'} API Keys`}

- {`${ - showRoleManagementEnabled ? '' : 'Terraform' - } API keys are used to authenticate with the Admin API.`} + {`API keys are used to authenticate with the Admin API.`}

{this.state.isLoading && ( @@ -470,10 +447,7 @@ export default class AdminAPIKeys extends PureComponent { - showRoleManagementEnabled && - this.editAPIKey(v.name, v.id, v.prefix) - } + onClick={() => this.editAPIKey(v.name, v.id, v.prefix)} >
{v.name}
@@ -510,7 +484,7 @@ export default class AdminAPIKeys extends PureComponent { > - -
- } - type='text' - title='Assign roles' - tooltip='Assigns what role the user/group will have' - inputProps={{ - className: 'full-width', - style: { minHeight: 80 }, - }} - className='full-width' - placeholder='Add an optional description...' - /> - - )} - {Utils.getFlagsmithHasFeature('show_role_management') && - level !== 'environment' && - level !== 'project' && ( -
- v.role)} - onAdd={addRole} - onRemove={removeOwner} - isOpen={showRoles} - onToggle={() => setShowRoles(!showRoles)} - /> -
- )} + {r.name} + + + + + ))} + + +
+ } + type='text' + title='Assign roles' + tooltip='Assigns what role the user/group will have' + inputProps={{ + className: 'full-width', + style: { minHeight: 80 }, + }} + className='full-width' + placeholder='Add an optional description...' + /> + + )} + {level !== 'environment' && level !== 'project' && ( +
+ v.role)} + onAdd={addRole} + onRemove={removeOwner} + isOpen={showRoles} + onToggle={() => setShowRoles(!showRoles)} + /> +
+ )}
) }), @@ -1111,84 +1102,82 @@ const EditPermissions: FC = (props) => { - {Utils.getFlagsmithHasFeature('show_role_management') && ( - - {hasRbacPermission ? ( - <> - -
{roleTabTitle}
- - -
- Roles -
-
- Description -
+ + {hasRbacPermission ? ( + <> + +
{roleTabTitle}
+ + +
+ Roles +
+
+ Description +
+ + } + renderRow={(role: Role) => ( + + editRolePermissions(role)} + className='table-column px-3' + style={{ + width: rolesWidths[0], + }} + > + {role.name} - } - renderRow={(role: Role) => ( editRolePermissions(role)} + style={{ + width: rolesWidths[1], + }} > - editRolePermissions(role)} - className='table-column px-3' - style={{ - width: rolesWidths[0], - }} - > - {role.name} - - editRolePermissions(role)} - style={{ - width: rolesWidths[1], - }} - > - {role.description} - + {role.description} - )} - renderNoResults={ - -
- - {`You currently have no roles with ${level} permissions.`} - -
- - } - isLoading={false} - /> - - ) : ( -
- - To use role features you have to upgrade your - plan. - -
- )} - - )} + + )} + renderNoResults={ + +
+ + {`You currently have no roles with ${level} permissions.`} + +
+ + } + isLoading={false} + /> + + ) : ( +
+ + To use role features you have to upgrade your + plan. + +
+ )} + ) diff --git a/frontend/web/components/ScrollToTop.tsx b/frontend/web/components/ScrollToTop.tsx new file mode 100644 index 000000000000..ee8eb410cd2e --- /dev/null +++ b/frontend/web/components/ScrollToTop.tsx @@ -0,0 +1,16 @@ +import { FC, useEffect } from 'react' +import { withRouter } from 'react-router-dom' +import { RouteComponentProps } from 'react-router' + +type ScrollToTopType = RouteComponentProps & {} + +const ScrollToTop: FC = (props) => { + const pathname = props.location.pathname + useEffect(() => { + window.scrollTo(0, 0) + }, [pathname]) + + return null +} + +export default withRouter(ScrollToTop) diff --git a/frontend/web/components/SegmentOverrides.js b/frontend/web/components/SegmentOverrides.js index de44830e115d..4b23f530a320 100644 --- a/frontend/web/components/SegmentOverrides.js +++ b/frontend/web/components/SegmentOverrides.js @@ -213,7 +213,7 @@ const SegmentOverrideInner = class Override extends React.Component { theme='text' disabled={!permission} target='_blank' - href={`${document.location.origin}/project/${this.props.projectId}/environment/${this.props.environmentId}/segments?id=${v.segment}`} + href={`${document.location.origin}/project/${this.props.projectId}/segments?id=${v.segment}`} className='btn btn-with-icon' > diff --git a/frontend/web/components/base/higher-order/ParameterizedRoute.tsx b/frontend/web/components/base/higher-order/ParameterizedRoute.tsx new file mode 100644 index 000000000000..c1ca292e5e8d --- /dev/null +++ b/frontend/web/components/base/higher-order/ParameterizedRoute.tsx @@ -0,0 +1,49 @@ +import NotFoundPage from 'components/pages/NotFoundPage' +import React from 'react' +import { RouteComponentProps, Route } from 'react-router-dom' + +type ParameterizedRouteType = { + component: React.ComponentType + [key: string]: any +} + +export const ParameterizedRoute = ({ + component: Component, + ...props +}: ParameterizedRouteType) => { + const { organisationId, projectId } = props.computedMatch.params + + const parsedOrganisationId = organisationId && parseInt(organisationId) + const parsedProjectId = projectId && parseInt(projectId) + + // Handle the case where the parameters are invalid + if ( + (projectId && isNaN(parseInt(projectId))) || + (organisationId && isNaN(parseInt(organisationId))) + ) { + return + } + + if (!projectId && !organisationId) { + return + } + + return ( + ( + + )} + /> + ) +} diff --git a/frontend/web/components/diff/DiffChangeRequest.tsx b/frontend/web/components/diff/DiffChangeRequest.tsx index c13d98e1d766..15d7f4170c61 100644 --- a/frontend/web/components/diff/DiffChangeRequest.tsx +++ b/frontend/web/components/diff/DiffChangeRequest.tsx @@ -35,15 +35,13 @@ const DiffChangeRequest: FC = ({ ) } return ( -
- -
+ ) } diff --git a/frontend/web/components/import-export/ImportPage.tsx b/frontend/web/components/import-export/ImportPage.tsx index dedd4d92fff0..e98a890362c6 100644 --- a/frontend/web/components/import-export/ImportPage.tsx +++ b/frontend/web/components/import-export/ImportPage.tsx @@ -37,10 +37,6 @@ const ImportPage: FC = ({ const [createLaunchDarklyProjectImport, { data, isSuccess }] = useCreateLaunchDarklyProjectImportMutation() - const hasFlagsmithImport = Utils.getFlagsmithHasFeature( - 'flagsmith_import_export', - ) - const { data: status, isSuccess: statusLoaded, @@ -231,20 +227,16 @@ const ImportPage: FC = ({ )}
- {Utils.getFlagsmithHasFeature('flagsmith_import_export') ? ( - - -
- -
- - -
{launchDarklyImport}
- - - ) : ( - launchDarklyImport - )} + + +
+ +
+ + +
{launchDarklyImport}
+ +
) diff --git a/frontend/web/components/modals/CreateSegment.tsx b/frontend/web/components/modals/CreateSegment.tsx index e8711f160610..b164e1277b01 100644 --- a/frontend/web/components/modals/CreateSegment.tsx +++ b/frontend/web/components/modals/CreateSegment.tsx @@ -312,18 +312,16 @@ const CreateSegment: FC = ({ )} - {!readOnly && Utils.getFlagsmithHasFeature('not_operator') && ( -
addRule('NONE')} className='text-center'> - -
- )} +
addRule('NONE')} className='text-center'> + +
diff --git a/frontend/web/components/modals/Payment.js b/frontend/web/components/modals/Payment.js index 9fce5b53eb73..b2f5a95f799d 100644 --- a/frontend/web/components/modals/Payment.js +++ b/frontend/web/components/modals/Payment.js @@ -12,10 +12,7 @@ const PaymentButton = (props) => { AccountStore.getOrganisation().id, ) - if ( - Utils.getFlagsmithHasFeature('upgrade_subscription') && - activeSubscription - ) { + if (activeSubscription) { return ( { diff --git a/frontend/web/components/pages/ChangeRequestPage.js b/frontend/web/components/pages/ChangeRequestPage.js index beda37f69130..9d69cbfc97e6 100644 --- a/frontend/web/components/pages/ChangeRequestPage.js +++ b/frontend/web/components/pages/ChangeRequestPage.js @@ -347,167 +347,144 @@ const ChangeRequestsPage = class extends Component { : 'Unknown user'}

{changeRequest.description}

-
-
- {isScheduled && ( -
- - This feature change{' '} - {changeRequest?.committedAt - ? 'is scheduled to' - : 'will'}{' '} - go live at{' '} - {scheduledDate.format('Do MMM YYYY hh:mma')} - {changeRequest?.committed_at - ? ' unless it is edited or deleted' - : ' if it is approved and published'} - . - {!!changeRequest?.committedAt && - 'You can still edit / remove the change request before this date.'} - -
- )} - - {!Utils.getFlagsmithHasFeature( - 'disable_users_as_reviewers', - ) && ( -
- - this.setState({ showUsers: true }) - } + {isScheduled && ( +
+ + This feature change{' '} + {changeRequest?.committedAt ? 'is scheduled to' : 'will'}{' '} + go live at {scheduledDate.format('Do MMM YYYY hh:mma')} + {changeRequest?.committed_at + ? ' unless it is edited or deleted' + : ' if it is approved and published'} + . + {!!changeRequest?.committedAt && + 'You can still edit / remove the change request before this date.'} + +
+ )} + + {!Utils.getFlagsmithHasFeature( + 'disable_users_as_reviewers', + ) && ( +
+ this.setState({ showUsers: true })} + > + Assigned users + + + {ownerUsers.length !== 0 && + ownerUsers.map((u) => ( + this.removeOwner(u.id)} + className='chip' + style={{ + marginBottom: 4, + marginTop: 4, + }} + > + + {u.first_name} {u.last_name} + + + + + + ))} + + v.id)} + onAdd={this.addOwner} + onRemove={this.removeOwner} + isOpen={this.state.showUsers} + onToggle={() => + this.setState({ + showUsers: !this.state.showUsers, + }) + } + /> +
+ )} +
+ this.setState({ showGroups: true })} + > + Assigned groups + + + {!!ownerGroups?.length && + ownerGroups.map((g) => ( + this.removeOwner(g.id, false)} + className='chip' + style={{ + marginBottom: 4, + marginTop: 4, + }} > - Assigned users - - - {ownerUsers.length !== 0 && - ownerUsers.map((u) => ( - this.removeOwner(u.id)} - className='chip' - style={{ - marginBottom: 4, - marginTop: 4, - }} - > - - {u.first_name} {u.last_name} - - - - - - ))} + + {g.name} + + + + - v.id) - } - onAdd={this.addOwner} - onRemove={this.removeOwner} - isOpen={this.state.showUsers} - onToggle={() => - this.setState({ - showUsers: !this.state.showUsers, - }) - } - /> -
- )} -
- - this.setState({ showGroups: true }) - } - > - Assigned groups - - - {!!ownerGroups?.length && - ownerGroups.map((g) => ( - - this.removeOwner(g.id, false) - } - className='chip' - style={{ - marginBottom: 4, - marginTop: 4, - }} - > - - {g.name} - - - - - - ))} - - v.id) - } - onAdd={this.addOwner} - onRemove={this.removeOwner} - isOpen={this.state.showGroups} - onToggle={() => - this.setState({ - showGroups: !this.state.showGroups, - }) - } - /> -
- - } - /> + ))} + + v.id)} + onAdd={this.addOwner} + onRemove={this.removeOwner} + isOpen={this.state.showGroups} + onToggle={() => + this.setState({ + showGroups: !this.state.showGroups, + }) + } + /> +
+ + } + /> - ) : ( - + {!isYourChangeRequest && Utils.renderWithPermission( @@ -576,10 +553,6 @@ const ChangeRequestsPage = class extends Component { )} - - -
-
)} diff --git a/frontend/web/components/pages/EnvironmentSettingsPage.js b/frontend/web/components/pages/EnvironmentSettingsPage.js index 3a391c7adaf3..2ecb2f3a011d 100644 --- a/frontend/web/components/pages/EnvironmentSettingsPage.js +++ b/frontend/web/components/pages/EnvironmentSettingsPage.js @@ -45,34 +45,35 @@ const EnvironmentSettingsPage = class extends Component { componentDidMount = () => { API.trackPage(Constants.pages.ENVIRONMENT_SETTINGS) - if (Utils.getFlagsmithHasFeature('show_role_management')) { - const env = ProjectStore.getEnvs().find( - (v) => v.api_key === this.props.match.params.environmentId, - ) - this.setState({ env }) - getRoles( + this.getEnvironment() + this.props.getWebhooks() + } + + getEnvironment = () => { + const env = ProjectStore.getEnvs().find( + (v) => v.api_key === this.props.match.params.environmentId, + ) + this.setState({ env }) + getRoles( + getStore(), + { organisation_id: AccountStore.getOrganisation().id }, + { forceRefetch: true }, + ).then((roles) => { + getRolesEnvironmentPermissions( getStore(), - { organisation_id: AccountStore.getOrganisation().id }, + { + env_id: env.id, + organisation_id: AccountStore.getOrganisation().id, + role_id: roles.data.results[0].id, + }, { forceRefetch: true }, - ).then((roles) => { - getRolesEnvironmentPermissions( - getStore(), - { - env_id: env.id, - organisation_id: AccountStore.getOrganisation().id, - role_id: roles.data.results[0].id, - }, - { forceRefetch: true }, - ).then((res) => { - const matchingItems = roles.data.results.filter((item1) => - res.data.results.some((item2) => item2.role === item1.id), - ) - this.setState({ roles: matchingItems }) - }) + ).then((res) => { + const matchingItems = roles.data.results.filter((item1) => + res.data.results.some((item2) => item2.role === item1.id), + ) + this.setState({ roles: matchingItems }) }) - } - - this.props.getWebhooks() + }) } onSave = () => { @@ -85,6 +86,12 @@ const EnvironmentSettingsPage = class extends Component { ) { AppActions.getProject(this.props.match.params.projectId) } + if ( + this.props.match.params.environmentId !== + prevProps.match.params.environmentId + ) { + this.getEnvironment() + } } onRemove = () => { @@ -238,8 +245,10 @@ const EnvironmentSettingsPage = class extends Component { api_key: this.props.match.params.environmentId, }) if ( - env && - typeof this.state.minimum_change_request_approvals === 'undefined' + (env && + typeof this.state.minimum_change_request_approvals === + 'undefined') || + this.state.env?.api_key !== this.props.match.params.environmentId ) { setTimeout(() => { this.setState({ diff --git a/frontend/web/components/pages/FeaturesPage.js b/frontend/web/components/pages/FeaturesPage.js index 4232b7ee5e89..cd61e877d7fb 100644 --- a/frontend/web/components/pages/FeaturesPage.js +++ b/frontend/web/components/pages/FeaturesPage.js @@ -219,11 +219,7 @@ const FeaturesPage = class extends Component { render() { const { environmentId, projectId } = this.props.match.params const readOnly = Utils.getFlagsmithHasFeature('read_only_mode') - const enabledStateFilter = Utils.getFlagsmithHasFeature( - 'feature_enabled_state_filter', - ) const environment = ProjectStore.getEnvironment(environmentId) - const ownersFilter = Utils.getFlagsmithHasFeature('owners_filter') return (
- {enabledStateFilter && ( - { - this.setState( - { - is_enabled: enabled, - value_search: valueSearch, - }, - this.filter, - ) - }} - /> - )} - {ownersFilter && ( - { - FeatureListStore.isLoading = true - this.setState( - { - owners: owners, - }, - this.filter, - ) - }} - /> - )} - {ownersFilter && ( - { - FeatureListStore.isLoading = true - this.setState( - { - group_owners: group_owners, - }, - this.filter, - ) - }} - /> - )} + { + this.setState( + { + is_enabled: enabled, + value_search: valueSearch, + }, + this.filter, + ) + }} + /> + { + FeatureListStore.isLoading = true + this.setState( + { + owners: owners, + }, + this.filter, + ) + }} + /> + { + FeatureListStore.isLoading = true + this.setState( + { + group_owners: group_owners, + }, + this.filter, + ) + }} + />
- {Utils.getFlagsmithHasFeature('force_2fa') && ( -
- - {!force2faPermission ? ( - - } - > - To access this feature please upgrade - your account to scaleup or higher." - - ) : ( - - )} -
Enforce 2FA
- -

- Enabling this setting forces users within - the organisation to setup 2 factor - security. -

-
- )} +
+ + {!force2faPermission ? ( + + } + > + To access this feature please upgrade + your account to scaleup or higher." + + ) : ( + + )} +
Enforce 2FA
+ +

+ Enabling this setting forces users within + the organisation to setup 2 factor security. +

+
{Utils.getFlagsmithHasFeature( 'restrict_project_create_to_admin', ) && ( diff --git a/frontend/web/components/pages/ProjectSettingsPage.js b/frontend/web/components/pages/ProjectSettingsPage.js index 97d4cb2c9b75..054551a14617 100644 --- a/frontend/web/components/pages/ProjectSettingsPage.js +++ b/frontend/web/components/pages/ProjectSettingsPage.js @@ -49,28 +49,26 @@ const ProjectSettingsPage = class extends Component { componentDidMount = () => { API.trackPage(Constants.pages.PROJECT_SETTINGS) - if (Utils.getFlagsmithHasFeature('show_role_management')) { - getRoles( + getRoles( + getStore(), + { organisation_id: AccountStore.getOrganisation().id }, + { forceRefetch: true }, + ).then((roles) => { + getRolesProjectPermissions( getStore(), - { organisation_id: AccountStore.getOrganisation().id }, + { + organisation_id: AccountStore.getOrganisation().id, + project_id: this.props.match.params.projectId, + role_id: roles.data.results[0].id, + }, { forceRefetch: true }, - ).then((roles) => { - getRolesProjectPermissions( - getStore(), - { - organisation_id: AccountStore.getOrganisation().id, - project_id: this.props.match.params.projectId, - role_id: roles.data.results[0].id, - }, - { forceRefetch: true }, - ).then((res) => { - const matchingItems = roles.data.results.filter((item1) => - res.data.results.some((item2) => item2.role === item1.id), - ) - this.setState({ roles: matchingItems }) - }) + ).then((res) => { + const matchingItems = roles.data.results.filter((item1) => + res.data.results.some((item2) => item2.role === item1.id), + ) + this.setState({ roles: matchingItems }) }) - } + }) } onSave = () => { @@ -618,16 +616,13 @@ const ProjectSettingsPage = class extends Component { /> )} - {!!ProjectStore.getEnvs()?.length && - Utils.getFlagsmithHasFeature( - 'flagsmith_import_export', - ) && ( - - - - )} + {!!ProjectStore.getEnvs()?.length && ( + + + + )} }
diff --git a/frontend/web/components/pages/UserPage.js b/frontend/web/components/pages/UserPage.js index 69d7756fc648..0c13325f8059 100644 --- a/frontend/web/components/pages/UserPage.js +++ b/frontend/web/components/pages/UserPage.js @@ -320,10 +320,6 @@ const UserPage = class extends Component { render() { const { actualFlags } = this.state const { environmentId, projectId } = this.props.match.params - const enabledStateFilter = Utils.getFlagsmithHasFeature( - 'feature_enabled_state_filter', - ) - const ownersFilter = Utils.getFlagsmithHasFeature('owners_filter') const preventAddTrait = !AccountStore.getOrganisation().persist_trait_data return (
@@ -536,70 +532,62 @@ const UserPage = class extends Component { ) }} /> - {enabledStateFilter && ( - { - this.setState( - { - is_enabled: enabled, - value_search: valueSearch, - }, - this.filter, - ) - }} - /> - )} - {ownersFilter && ( - { - FeatureListStore.isLoading = true - this.setState( - { - owners: owners, - }, - this.filter, - ) - }} - /> - )} - {ownersFilter && ( - { - FeatureListStore.isLoading = true - this.setState( - { - group_owners: - group_owners, - }, - this.filter, - ) - }} - /> - )} + { + this.setState( + { + is_enabled: enabled, + value_search: valueSearch, + }, + this.filter, + ) + }} + /> + { + FeatureListStore.isLoading = true + this.setState( + { + owners: owners, + }, + this.filter, + ) + }} + /> + { + FeatureListStore.isLoading = true + this.setState( + { + group_owners: group_owners, + }, + this.filter, + ) + }} + /> = ({ (overSeats && (!verifySeatsLimit || !autoSeats)) || (!autoSeats && usedSeats) return ( -
+
= ({ />
- {Utils.getFlagsmithHasFeature('show_role_management') && ( - - {hasRbacPermission ? ( - <> - - - ) : ( -
- - To use role features you have to - upgrade your plan. - -
- )} - - )} + + {hasRbacPermission ? ( + <> + + + ) : ( +
+ + To use role features you have to + upgrade your plan. + +
+ )} +
)} diff --git a/frontend/web/routes.js b/frontend/web/routes.js index 2c1fbf473380..1370fd8a529b 100644 --- a/frontend/web/routes.js +++ b/frontend/web/routes.js @@ -38,6 +38,7 @@ import OrganisationsPage from './components/pages/OrganisationsPage' import UsersAndPermissionsPage from './components/pages/UsersAndPermissionsPage' import ProjectRedirectPage from './components/pages/ProjectRedirectPage' import SDKKeysPage from './components/SDKKeysPage' +import { ParameterizedRoute } from './components/base/higher-order/ParameterizedRoute' export default ( @@ -56,27 +57,27 @@ export default ( exact component={PasswordResetPage} /> - - - - - - - - - - - - - - - + - - - - - - - - - + - - - * { + margin: 0; + } &.react-diff-marker--added { background-color: $success-alfa-16 !important; diff --git a/version.txt b/version.txt index 2c047762ff63..a6629883a5e9 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -2.114.0 +2.115.0