Commit 038922f4 authored by Jason Goodman's avatar Jason Goodman Committed by Shinya Maeda

Delete correct strategy when removed from new feature flag

Generate Vue key for strategies without an id
parent 34fce96f
<script> <script>
import Vue from 'vue'; import Vue from 'vue';
import { memoize, isString, cloneDeep, isNumber } from 'lodash'; import { memoize, isString, cloneDeep, isNumber, uniqueId } from 'lodash';
import { import {
GlDeprecatedButton, GlDeprecatedButton,
GlDeprecatedBadge as GlBadge, GlDeprecatedBadge as GlBadge,
...@@ -143,7 +143,6 @@ export default { ...@@ -143,7 +143,6 @@ export default {
supportsStrategies() { supportsStrategies() {
return this.glFeatures.featureFlagsNewVersion && this.version === NEW_VERSION_FLAG; return this.glFeatures.featureFlagsNewVersion && this.version === NEW_VERSION_FLAG;
}, },
canDeleteStrategy() { canDeleteStrategy() {
return this.formStrategies.length > 1; return this.formStrategies.length > 1;
}, },
...@@ -160,6 +159,14 @@ export default { ...@@ -160,6 +159,14 @@ export default {
} }
}, },
methods: { methods: {
keyFor(strategy) {
if (strategy.id) {
return strategy.id;
}
return uniqueId('strategy_');
},
addStrategy() { addStrategy() {
this.formStrategies.push({ name: '', parameters: {}, scopes: [] }); this.formStrategies.push({ name: '', parameters: {}, scopes: [] });
}, },
...@@ -321,7 +328,7 @@ export default { ...@@ -321,7 +328,7 @@ export default {
<div v-if="filteredStrategies.length > 0" data-testid="feature-flag-strategies"> <div v-if="filteredStrategies.length > 0" data-testid="feature-flag-strategies">
<strategy <strategy
v-for="(strategy, index) in filteredStrategies" v-for="(strategy, index) in filteredStrategies"
:key="strategy.id" :key="keyFor(strategy)"
:strategy="strategy" :strategy="strategy"
:index="index" :index="index"
:endpoint="environmentsEndpoint" :endpoint="environmentsEndpoint"
......
...@@ -273,7 +273,12 @@ export default { ...@@ -273,7 +273,12 @@ export default {
</div> </div>
<div class="align-self-end align-self-md-stretch order-first offset-md-0 order-md-0 ml-auto"> <div class="align-self-end align-self-md-stretch order-first offset-md-0 order-md-0 ml-auto">
<gl-deprecated-button v-if="canDelete" variant="danger" @click="$emit('delete')"> <gl-deprecated-button
v-if="canDelete"
data-testid="delete-strategy-button"
variant="danger"
@click="$emit('delete')"
>
<span class="d-md-none"> <span class="d-md-none">
{{ $options.translations.removeLabel }} {{ $options.translations.removeLabel }}
</span> </span>
......
---
title: Remove correct feature flag strategy when removing strategy that is not persisted
merge_request: 34889
author:
type: fixed
...@@ -50,6 +50,24 @@ RSpec.describe 'User creates feature flag', :js do ...@@ -50,6 +50,24 @@ RSpec.describe 'User creates feature flag', :js do
end end
end end
it 'removes the correct strategy when a strategy is deleted' do
visit(new_project_feature_flag_path(project))
click_button 'Add strategy'
within_strategy_row(1) do
select 'All users', from: 'Type'
end
within_strategy_row(2) do
select 'Percent rollout (logged in users)', from: 'Type'
end
within_strategy_row(1) do
delete_strategy_button.click
end
within_strategy_row(1) do
expect(page).to have_select('Type', selected: 'Percent rollout (logged in users)')
end
end
context 'with new version flags disabled' do context 'with new version flags disabled' do
before do before do
stub_feature_flags(feature_flags_new_version: false) stub_feature_flags(feature_flags_new_version: false)
......
...@@ -406,12 +406,12 @@ describe('feature flag form', () => { ...@@ -406,12 +406,12 @@ describe('feature flag form', () => {
strategies: [ strategies: [
{ {
type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT, type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
paramters: { percentage: '30' }, parameters: { percentage: '30' },
scopes: [], scopes: [],
}, },
{ {
type: ROLLOUT_STRATEGY_ALL_USERS, type: ROLLOUT_STRATEGY_ALL_USERS,
paramters: {}, parameters: {},
scopes: [{ environment_scope: 'review/*' }], scopes: [{ environment_scope: 'review/*' }],
}, },
], ],
...@@ -429,7 +429,7 @@ describe('feature flag form', () => { ...@@ -429,7 +429,7 @@ describe('feature flag form', () => {
expect(strategy.exists()).toBe(true); expect(strategy.exists()).toBe(true);
expect(strategy.props('strategy')).toEqual({ expect(strategy.props('strategy')).toEqual({
type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT, type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
paramters: { percentage: '30' }, parameters: { percentage: '30' },
scopes: [], scopes: [],
}); });
}); });
...@@ -446,7 +446,7 @@ describe('feature flag form', () => { ...@@ -446,7 +446,7 @@ describe('feature flag form', () => {
it('should remove a strategy on delete', () => { it('should remove a strategy on delete', () => {
const strategy = { const strategy = {
type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT, type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
paramters: { percentage: '30' }, parameters: { percentage: '30' },
scopes: [], scopes: [],
}; };
wrapper.find(Strategy).vm.$emit('delete'); wrapper.find(Strategy).vm.$emit('delete');
......
...@@ -60,6 +60,10 @@ module FeatureFlagHelpers ...@@ -60,6 +60,10 @@ module FeatureFlagHelpers
find('.js-feature-flag-edit-button') find('.js-feature-flag-edit-button')
end end
def delete_strategy_button
find("button[data-testid='delete-strategy-button']")
end
def status_toggle_button def status_toggle_button
find('.js-feature-flag-status button') find('.js-feature-flag-status button')
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment