Merge branch 'delete-user-list-error-ui' into 'master'

Show Error Message When Delete Feature Flag User List Fails

See merge request gitlab-org/gitlab!36487

ee/app/assets/javascripts/feature_flags/components/feature_flags.vue

@@ -112,6 +112,7 @@ export default {
     ...mapState([
       FEATURE_FLAG_SCOPE,
       USER_LIST_SCOPE,
+      'alerts',
       'count',
       'pageInfo',
       'isLoading',
@@ -191,6 +192,7 @@ export default {
       'rotateInstanceId',
       'toggleFeatureFlag',
       'deleteUserList',
+      'clearAlert',
     ]),
     onChangeTab(scope) {
       this.scope = scope;
@@ -285,6 +287,16 @@ export default {
       </gl-sprintf>
     </gl-alert>
 
+    <gl-alert
+      v-for="(message, index) in alerts"
+      :key="index"
+      data-testid="serverErrors"
+      variant="danger"
+      @dismiss="clearAlert(index)"
+    >
+      {{ message }}
+    </gl-alert>
+
     <div v-if="shouldRenderTabs" class="top-area scrolling-tabs-container inner-page-scroll-tabs">
       <navigation-tabs :tabs="tabs" scope="featureflags" @onChangeTab="onChangeTab" />
     </div>

ee/app/assets/javascripts/feature_flags/store/modules/index/actions.js

@@ -72,14 +72,20 @@ export const deleteUserList = ({ state, dispatch }, list) => {
 
   return Api.deleteFeatureFlagUserList(state.projectId, list.iid)
     .then(() => dispatch('fetchUserLists'))
-    .catch(() => dispatch('receiveDeleteUserListError', list));
+    .catch(error =>
+      dispatch('receiveDeleteUserListError', {
+        list,
+        error: error?.response?.data ?? error,
+      }),
+    );
 };
 
 export const requestDeleteUserList = ({ commit }, list) =>
   commit(types.REQUEST_DELETE_USER_LIST, list);
 
-export const receiveDeleteUserListError = ({ commit }, list) =>
-  commit(types.RECEIVE_DELETE_USER_LIST_ERROR, list);
+export const receiveDeleteUserListError = ({ commit }, { error, list }) => {
+  commit(types.RECEIVE_DELETE_USER_LIST_ERROR, { error, list });
+};
 
 export const rotateInstanceId = ({ state, dispatch }) => {
   dispatch('requestRotateInstanceId');
@@ -96,5 +102,9 @@ export const receiveRotateInstanceIdSuccess = ({ commit }, response) =>
 export const receiveRotateInstanceIdError = ({ commit }) =>
   commit(types.RECEIVE_ROTATE_INSTANCE_ID_ERROR);
 
+export const clearAlert = ({ commit }, index) => {
+  commit(types.RECEIVE_CLEAR_ALERT, index);
+};
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

ee/app/assets/javascripts/feature_flags/store/modules/index/mutation_types.js

@@ -22,3 +22,5 @@ export const RECEIVE_UPDATE_FEATURE_FLAG_ERROR = 'RECEIVE_UPDATE_FEATURE_FLAG_ER
 export const REQUEST_ROTATE_INSTANCE_ID = 'REQUEST_ROTATE_INSTANCE_ID';
 export const RECEIVE_ROTATE_INSTANCE_ID_SUCCESS = 'RECEIVE_ROTATE_INSTANCE_ID_SUCCESS';
 export const RECEIVE_ROTATE_INSTANCE_ID_ERROR = 'RECEIVE_ROTATE_INSTANCE_ID_ERROR';
+
+export const RECEIVE_CLEAR_ALERT = 'RECEIVE_CLEAR_ALERT';

ee/app/assets/javascripts/feature_flags/store/modules/index/mutations.js

@@ -113,9 +113,13 @@ export default {
   [types.REQUEST_DELETE_USER_LIST](state, list) {
     state.userLists = state.userLists.filter(l => l !== list);
   },
-  [types.RECEIVE_DELETE_USER_LIST_ERROR](state, list) {
+  [types.RECEIVE_DELETE_USER_LIST_ERROR](state, { error, list }) {
     state.isLoading = false;
-    state.hasError = true;
+    state.hasError = false;
+    state.alerts = [].concat(error.message);
     state.userLists = state.userLists.concat(list).sort((l1, l2) => l1.iid - l2.iid);
   },
+  [types.RECEIVE_CLEAR_ALERT](state, index) {
+    state.alerts.splice(index, 1);
+  },
 };

ee/app/assets/javascripts/feature_flags/store/modules/index/state.js

@@ -3,6 +3,7 @@ import { FEATURE_FLAG_SCOPE, USER_LIST_SCOPE } from '../../../constants';
 export default () => ({
   [FEATURE_FLAG_SCOPE]: [],
   [USER_LIST_SCOPE]: [],
+  alerts: [],
   count: {},
   pageInfo: { [FEATURE_FLAG_SCOPE]: {}, [USER_LIST_SCOPE]: {} },
   isLoading: true,

ee/changelogs/unreleased/delete-user-list-error-ui.yml

+---
+title: Show error when attempting to delete feature flag user list that is in use
+merge_request: 36487
+author:
+type: fixed

ee/spec/features/projects/feature_flag_user_lists/user_deletes_feature_flag_user_list_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'User deletes feature flag user list', :js do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:developer) { create(:user) }
+
+  before do
+    project.add_developer(developer)
+    stub_licensed_features(feature_flags: true)
+    sign_in(developer)
+  end
+
+  context 'with a list' do
+    before do
+      create(:operations_feature_flag_user_list, project: project, name: 'My List')
+    end
+
+    it 'deletes the list' do
+      visit(project_feature_flags_path(project, scope: 'userLists'))
+
+      delete_user_list_button.click
+      delete_user_list_modal_confirmation_button.click
+
+      expect(page).to have_text('Lists 0')
+    end
+  end
+
+  context 'with a list that is in use' do
+    before do
+      list = create(:operations_feature_flag_user_list, project: project, name: 'My List')
+      feature_flag = create(:operations_feature_flag, :new_version_flag, project: project)
+      create(:operations_strategy, feature_flag: feature_flag, name: 'gitlabUserList', user_list: list)
+    end
+
+    it 'does not delete the list' do
+      visit(project_feature_flags_path(project, scope: 'userLists'))
+
+      delete_user_list_button.click
+      delete_user_list_modal_confirmation_button.click
+
+      expect(page).to have_text('User list is associated with a strategy')
+      expect(page).to have_text('Lists 1')
+      expect(page).to have_text('My List')
+
+      alert_dismiss_button.click
+
+      expect(page).not_to have_text('User list is associated with a strategy')
+    end
+  end
+
+  def delete_user_list_button
+    find("button[data-testid='delete-user-list']")
+  end
+
+  def delete_user_list_modal_confirmation_button
+    find("button[data-testid='modal-confirm']")
+  end
+
+  def alert_dismiss_button
+    find("div[data-testid='serverErrors'] button")
+  end
+end

ee/spec/frontend/feature_flags/store/index/actions_spec.js

@@ -22,6 +22,7 @@ import {
   fetchUserLists,
   deleteUserList,
   receiveDeleteUserListError,
+  clearAlert,
 } from 'ee/feature_flags/store/modules/index/actions';
 import { mapToScopesViewModel } from 'ee/feature_flags/store/modules/helpers';
 import state from 'ee/feature_flags/store/modules/index/state';
@@ -547,7 +548,7 @@ describe('Feature flags actions', () => {
 
     describe('error', () => {
       beforeEach(() => {
-        Api.deleteFeatureFlagUserList.mockRejectedValue();
+        Api.deleteFeatureFlagUserList.mockRejectedValue({ response: { data: 'some error' } });
       });
 
       it('should dispatch receiveDeleteUserListError', done => {
@@ -558,20 +559,44 @@ describe('Feature flags actions', () => {
           [],
           [
             { type: 'requestDeleteUserList', payload: userList },
-            { type: 'receiveDeleteUserListError', payload: userList },
+            {
+              type: 'receiveDeleteUserListError',
+              payload: { list: userList, error: 'some error' },
+            },
           ],
           done,
         );
       });
     });
   });
+
   describe('receiveDeleteUserListError', () => {
     it('should commit RECEIVE_DELETE_USER_LIST_ERROR with the given list', done => {
       testAction(
         receiveDeleteUserListError,
-        userList,
+        { list: userList, error: 'mock error' },
+        mockedState,
+        [
+          {
+            type: 'RECEIVE_DELETE_USER_LIST_ERROR',
+            payload: { list: userList, error: 'mock error' },
+          },
+        ],
+        [],
+        done,
+      );
+    });
+  });
+
+  describe('clearAlert', () => {
+    it('should commit RECEIVE_CLEAR_ALERT', done => {
+      const alertIndex = 3;
+
+      testAction(
+        clearAlert,
+        alertIndex,
         mockedState,
-        [{ type: 'RECEIVE_DELETE_USER_LIST_ERROR', payload: userList }],
+        [{ type: 'RECEIVE_CLEAR_ALERT', payload: alertIndex }],
         [],
         done,
       );

ee/spec/frontend/feature_flags/store/index/mutations_spec.js

@@ -281,6 +281,7 @@ describe('Feature flags store Mutations', () => {
       ]);
     });
   });
+
   describe('REQUEST_DELETE_USER_LIST', () => {
     beforeEach(() => {
       stateCopy.userLists = [userList];
@@ -291,18 +292,41 @@ describe('Feature flags store Mutations', () => {
       expect(stateCopy.userLists).not.toContain(userList);
     });
   });
+
   describe('RECEIVE_DELETE_USER_LIST_ERROR', () => {
     beforeEach(() => {
       stateCopy.userLists = [];
-      mutations[types.RECEIVE_DELETE_USER_LIST_ERROR](stateCopy, userList);
+      mutations[types.RECEIVE_DELETE_USER_LIST_ERROR](stateCopy, {
+        list: userList,
+        error: 'some error',
+      });
     });
 
-    it('should set isLoading to false and hasError to true', () => {
+    it('should set isLoading to false and hasError to false', () => {
       expect(stateCopy.isLoading).toBe(false);
-      expect(stateCopy.hasError).toBe(true);
+      expect(stateCopy.hasError).toBe(false);
     });
+
     it('should add the user list back to the list of user lists', () => {
       expect(stateCopy.userLists).toContain(userList);
     });
   });
+
+  describe('RECEIVE_CLEAR_ALERT', () => {
+    it('clears the alert', () => {
+      stateCopy.alerts = ['a server error'];
+
+      mutations[types.RECEIVE_CLEAR_ALERT](stateCopy, 0);
+
+      expect(stateCopy.alerts).toEqual([]);
+    });
+
+    it('clears the alert at the specified index', () => {
+      stateCopy.alerts = ['a server error', 'another error', 'final error'];
+
+      mutations[types.RECEIVE_CLEAR_ALERT](stateCopy, 1);
+
+      expect(stateCopy.alerts).toEqual(['a server error', 'final error']);
+    });
+  });
 });