diff --git a/api/features/serializers.py b/api/features/serializers.py index f0226642d94c..a00f597cf2f9 100644 --- a/api/features/serializers.py +++ b/api/features/serializers.py @@ -593,6 +593,10 @@ class Meta: fields = ("id", "feature", "environment") +class AssociatedFeaturesQuerySerializer(serializers.Serializer): + environment = serializers.IntegerField(required=False) + + class SDKFeatureStatesQuerySerializer(serializers.Serializer): feature = serializers.CharField( required=False, help_text="Name of the feature to get the state of" diff --git a/api/segments/views.py b/api/segments/views.py index 566dc9a6c1a1..c64e949d529a 100644 --- a/api/segments/views.py +++ b/api/segments/views.py @@ -10,8 +10,13 @@ from app.pagination import CustomPagination from edge_api.identities.models import EdgeIdentity from environments.identities.models import Identity +from environments.models import Environment from features.models import FeatureState -from features.serializers import SegmentAssociatedFeatureStateSerializer +from features.serializers import ( + AssociatedFeaturesQuerySerializer, + SegmentAssociatedFeatureStateSerializer, +) +from features.versioning.models import EnvironmentFeatureVersion from projects.permissions import VIEW_PROJECT from .models import Segment @@ -77,6 +82,7 @@ def get_queryset(self): return queryset + @swagger_auto_schema(query_serializer=AssociatedFeaturesQuerySerializer()) @action( detail=True, methods=["GET"], @@ -85,7 +91,22 @@ def get_queryset(self): ) def associated_features(self, request, *args, **kwargs): segment = self.get_object() - queryset = FeatureState.objects.filter(feature_segment__segment=segment) + + query_serializer = AssociatedFeaturesQuerySerializer(data=request.query_params) + query_serializer.is_valid(raise_exception=True) + + filter_kwargs = {"feature_segment__segment": segment} + if environment_id := query_serializer.validated_data.get("environment"): + environment = Environment.objects.get(pk=environment_id) + filter_kwargs["environment"] = environment + if environment.use_v2_feature_versioning: + filter_kwargs["environment_feature_version__in"] = ( + EnvironmentFeatureVersion.objects.get_latest_versions_by_environment_id( + environment_id + ) + ) + + queryset = FeatureState.objects.filter(**filter_kwargs) page = self.paginate_queryset(queryset) if page is not None: diff --git a/api/tests/unit/segments/test_unit_segments_views.py b/api/tests/unit/segments/test_unit_segments_views.py index 680a67ec7963..0f8f3f58e2db 100644 --- a/api/tests/unit/segments/test_unit_segments_views.py +++ b/api/tests/unit/segments/test_unit_segments_views.py @@ -18,7 +18,8 @@ from audit.models import AuditLog from audit.related_object_type import RelatedObjectType from environments.models import Environment -from features.models import Feature +from features.models import Feature, FeatureSegment, FeatureState +from features.versioning.models import EnvironmentFeatureVersion from metadata.models import Metadata, MetadataModelField from projects.models import Project from projects.permissions import MANAGE_SEGMENTS, VIEW_PROJECT @@ -323,6 +324,75 @@ def test_associated_features_returns_all_the_associated_features( assert response.json()["results"][0]["environment"] == environment.id +@pytest.mark.parametrize( + "client", + [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")], +) +def test_associated_features_returns_only_latest_versions_of_associated_features( + project: Project, + segment: Segment, + environment_v2_versioning: Environment, + client: APIClient, +) -> None: + # Given + # 2 features + feature_one = Feature.objects.create(project=project, name="feature_1") + feature_two = Feature.objects.create(project=project, name="feature_2") + + # Now let's create a version for each feature with a segment override + for feature in (feature_one, feature_two): + version = EnvironmentFeatureVersion.objects.create( + feature=feature, environment=environment_v2_versioning + ) + FeatureState.objects.create( + feature=feature, + environment=environment_v2_versioning, + environment_feature_version=version, + feature_segment=FeatureSegment.objects.create( + segment=segment, + environment=environment_v2_versioning, + feature=feature, + environment_feature_version=version, + ), + ) + version.publish() + + # And then let's create a third version for feature_one where we update the segment override + feature_1_version_3 = EnvironmentFeatureVersion.objects.create( + feature=feature_one, environment=environment_v2_versioning + ) + f1v3_segment_override_feature_state = feature_1_version_3.feature_states.get( + feature_segment__segment=segment + ) + f1v3_segment_override_feature_state.enabled = True + f1v3_segment_override_feature_state.save() + feature_1_version_3.publish() + + # And finally, let's create a third version for feature_two where we remove the segment override + feature_2_version_3 = EnvironmentFeatureVersion.objects.create( + feature=feature_two, environment=environment_v2_versioning + ) + feature_2_version_3.feature_states.filter(feature_segment__segment=segment).delete() + feature_2_version_3.publish() + + url = "%s?environment=%s" % ( + reverse( + "api-v1:projects:project-segments-associated-features", + args=[project.id, segment.id], + ), + environment_v2_versioning.id, + ) + + # When + response = client.get(url) + + # Then + assert response.json().get("count") == 1 + assert response.json()["results"][0]["id"] == f1v3_segment_override_feature_state.id + assert response.json()["results"][0]["feature"] == feature_one.id + assert response.json()["results"][0]["environment"] == environment_v2_versioning.id + + @pytest.mark.parametrize( "client", [lazy_fixture("admin_master_api_key_client"), lazy_fixture("admin_client")], diff --git a/frontend/web/components/modals/AssociatedSegmentOverrides.js b/frontend/web/components/modals/AssociatedSegmentOverrides.js index 06f48deb5c47..c59f42d7d1df 100644 --- a/frontend/web/components/modals/AssociatedSegmentOverrides.js +++ b/frontend/web/components/modals/AssociatedSegmentOverrides.js @@ -18,14 +18,22 @@ import Utils from 'common/utils/utils' class TheComponent extends Component { state = { isLoading: true, + selectedEnv: ProjectStore.getEnvs()?.[0]?.api_key, } componentDidMount() { this.fetch() } fetch = () => { + if (!this.state.selectedEnv) { + return + } _data .get( - `${Project.api}projects/${this.props.projectId}/segments/${this.props.id}/associated-features/`, + `${Project.api}projects/${this.props.projectId}/segments/${ + this.props.id + }/associated-features/?environment=${ProjectStore.getEnvironmentIdFromKey( + this.state.selectedEnv, + )}`, ) .then((v) => Promise.all( @@ -45,7 +53,7 @@ class TheComponent extends Component { (v) => v.id === e.environment, ) e.env = env - return env && env.name + return env && env.api_key }), ) .then((v) => { @@ -57,7 +65,7 @@ class TheComponent extends Component { }) const newItems = this.state.newItems || {} const selectedEnv = - this.state.selectedEnv || ProjectStore.getEnvs()[0].name + this.state.selectedEnv || ProjectStore.getEnvs()[0].api_key newItems[selectedEnv] = (newItems[selectedEnv] || []).filter( (newItem) => { const existingSegmentOverride = @@ -94,7 +102,7 @@ class TheComponent extends Component { (newItems && newItems[this.state.selectedEnv]) || [] const environment = ProjectStore.getEnvs().find( - (v) => v.name === this.state.selectedEnv, + (v) => v.api_key === this.state.selectedEnv, ) const selectedResults = selectedNewResults.concat( (results && results[this.state.selectedEnv]) || [], @@ -117,11 +125,7 @@ class TheComponent extends Component { ) - return this.state.isLoading ? ( -
- -
- ) : ( + return (
This shows the list of segment overrides associated with this segment. @@ -137,7 +141,7 @@ class TheComponent extends Component { .
@@ -145,62 +149,71 @@ class TheComponent extends Component { component={ - this.setState({ - selectedEnv: ProjectStore.getEnvs().find( - (v) => v.api_key === selectedEnv, - ).name, - }) + this.setState( + { + selectedEnv, + }, + this.fetch, + ) } /> } title='Environment' /> - this.setState({ search })} - filterRow={(row, search) => - row.feature.name.toLowerCase().includes(search.toLowerCase()) - } - className='no-pad panel-override' - title='Associated Features' - items={selectedResults} - renderNoResults={ - - {addOverride} -
- There are no segment overrides in this environment -
- - } - renderRow={(v) => ( -
-
- { - if (v.newSegmentOverrides) { - newItems[this.state.selectedEnv] = newItems[ - this.state.selectedEnv - ].filter((x) => x !== v) - this.setState({ - newItems, - }) - } - }} - id={this.props.id} - projectId={this.props.projectId} - environmentId={v.env.api_key} - readOnly={this.props.readOnly} - /> + + {this.state.isLoading ? ( +
+ +
+ ) : ( + this.setState({ search })} + filterRow={(row, search) => + row.feature.name.toLowerCase().includes(search.toLowerCase()) + } + className='no-pad panel-override' + title='Associated Features' + items={selectedResults} + renderNoResults={ + + {addOverride} +
+ There are no segment overrides in this environment +
+ + } + renderRow={(v) => ( +
+
+ + { + if (v.newSegmentOverrides) { + newItems[this.state.selectedEnv] = newItems[ + this.state.selectedEnv + ].filter((x) => x !== v) + this.setState({ + newItems, + }) + } + }} + id={this.props.id} + projectId={this.props.projectId} + environmentId={v.env.api_key} + readOnly={this.props.readOnly} + /> +
-
- )} - /> + )} + /> + )}
) @@ -300,7 +313,9 @@ export default class SegmentOverridesInner extends Component { segmentOverrides, updateSegments, } = this.props - const environment = ProjectStore.getEnvironment(environmentId) + const environment = ProjectStore.getEnvironment( + this.state.selectedEnvironment, + ) const changeRequest = Utils.changeRequestsEnabled( environment?.minimum_change_request_approvals, ) @@ -394,7 +409,7 @@ class SegmentOverridesInnerAdd extends Component { fetchTotalSegmentOverrides() { const { environmentId } = this.props - const env = ProjectStore.getEnvs().find((v) => v.name === environmentId) + const env = ProjectStore.getEnvs().find((v) => v.api_key === environmentId) if (!env) { return @@ -414,15 +429,17 @@ class SegmentOverridesInnerAdd extends Component { this.fetchTotalSegmentOverrides() } - componentDidUpdate(prevProps) { - if (prevProps.environmentId !== this.props.environmentId) { + componentDidUpdate(_, prevState) { + if (prevState.selectedEnv !== this.state.selectedEnv) { this.fetchTotalSegmentOverrides() } } render() { const { environmentId, id, ignoreFlags, projectId, readOnly } = this.props const addValue = (featureId, feature) => { - const env = ProjectStore.getEnvs().find((v) => v.name === environmentId) + const env = ProjectStore.getEnvs().find( + (v) => v.api_key === environmentId, + ) const item = { env, environment: environmentId, diff --git a/frontend/web/components/modals/CreateSegment.tsx b/frontend/web/components/modals/CreateSegment.tsx index c31e8fa27d0a..27d9e266671d 100644 --- a/frontend/web/components/modals/CreateSegment.tsx +++ b/frontend/web/components/modals/CreateSegment.tsx @@ -603,6 +603,7 @@ const CreateSegment: FC = ({ projectId={projectId} id={segment.id} readOnly={isReadOnly} + environmentId={environmentId} /> ) }}