Merge branch 'show-user-list-fe' into 'master'

Show User List Details

See merge request gitlab-org/gitlab!35369

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

 <script>
 import { s__, sprintf } from '~/locale';
 import timeagoMixin from '~/vue_shared/mixins/timeago';
-import { GlButton, GlModal, GlSprintf, GlTooltipDirective, GlModalDirective } from '@gitlab/ui';
+import {
+  GlButton,
+  GlButtonGroup,
+  GlModal,
+  GlSprintf,
+  GlTooltipDirective,
+  GlModalDirective,
+} from '@gitlab/ui';
 
 export default {
-  components: { GlButton, GlModal, GlSprintf },
+  components: { GlButton, GlButtonGroup, GlModal, GlSprintf },
   directives: { GlTooltip: GlTooltipDirective, GlModal: GlModalDirective },
   mixins: [timeagoMixin],
   props: {
@@ -80,15 +87,23 @@ export default {
         </span>
         <span data-testid="ffUserListIds" class="gl-str-truncated">{{ displayList(list) }}</span>
       </div>
-      <gl-button
-        v-gl-modal="$options.modal.id"
-        class="gl-align-self-start gl-mt-2"
-        category="secondary"
-        variant="danger"
-        icon="remove"
-        data-testid="delete-user-list"
-        @click="confirmDeleteList(list)"
-      />
+
+      <gl-button-group class="gl-align-self-start gl-mt-2">
+        <gl-button
+          :href="list.path"
+          category="secondary"
+          icon="pencil"
+          data-testid="edit-user-list"
+        />
+        <gl-button
+          v-gl-modal="$options.modal.id"
+          category="secondary"
+          variant="danger"
+          icon="remove"
+          data-testid="delete-user-list"
+          @click="confirmDeleteList(list)"
+        />
+      </gl-button-group>
     </div>
     <gl-modal
       :title="modalTitle"

ee/app/assets/javascripts/pages/projects/feature_flags_user_lists/show/index.js

+import Vue from 'vue';
+import Vuex from 'vuex';
+import UserList from 'ee/user_lists/components/user_list.vue';
+import createStore from 'ee/user_lists/store/show';
+
+Vue.use(Vuex);
+
+document.addEventListener('DOMContentLoaded', () => {
+  const el = document.getElementById('js-edit-user-list');
+  return new Vue({
+    el,
+    store: createStore(el.dataset),
+    render(h) {
+      const { emptyStatePath } = el.dataset;
+      return h(UserList, { props: { emptyStatePath } });
+    },
+  });
+});

ee/app/assets/javascripts/user_lists/components/user_list.vue

+<script>
+import { mapActions, mapState } from 'vuex';
+import { GlAlert, GlEmptyState, GlLoadingIcon, GlModalDirective as GlModal } from '@gitlab/ui';
+import { s__, __ } from '~/locale';
+import states from '../constants/show';
+
+const commonTableClasses = ['gl-py-5', 'gl-border-b-1', 'gl-border-b-solid', 'gl-border-gray-100'];
+
+export default {
+  components: {
+    GlAlert,
+    GlEmptyState,
+    GlLoadingIcon,
+  },
+  directives: {
+    GlModal,
+  },
+  props: {
+    emptyStatePath: {
+      required: true,
+      type: String,
+    },
+  },
+  translations: {
+    emptyStateTitle: s__('UserLists|There are no users'),
+    emptyStateDescription: s__(
+      'UserLists|Define a set of users to be used within feature flag strategies',
+    ),
+    userIdLabel: s__('UserLists|User IDs'),
+    userIdColumnHeader: s__('UserLists|User ID'),
+    errorMessage: __('Something went wrong on our end. Please try again!'),
+  },
+  classes: {
+    headerClasses: [
+      'gl-display-flex',
+      'gl-justify-content-space-between',
+      'gl-pb-5',
+      'gl-border-b-1',
+      'gl-border-b-solid',
+      'gl-border-gray-100',
+    ].join(' '),
+    tableHeaderClasses: commonTableClasses.join(' '),
+    tableRowClasses: [
+      ...commonTableClasses,
+      'gl-display-flex',
+      'gl-justify-content-space-between',
+      'gl-align-items-center',
+    ].join(' '),
+  },
+  modalId: 'add-userids-modal',
+  computed: {
+    ...mapState(['userList', 'userIds', 'state']),
+    name() {
+      return this.userList?.name ?? '';
+    },
+    hasUserIds() {
+      return this.userIds.length > 0;
+    },
+    isLoading() {
+      return this.state === states.LOADING;
+    },
+    hasError() {
+      return this.state === states.ERROR;
+    },
+  },
+  mounted() {
+    this.fetchUserList();
+  },
+  methods: {
+    ...mapActions(['fetchUserList', 'dismissErrorAlert']),
+  },
+};
+</script>
+<template>
+  <div>
+    <gl-alert v-if="hasError" variant="danger" @dismiss="dismissErrorAlert">
+      {{ $options.translations.errorMessage }}
+    </gl-alert>
+    <gl-loading-icon v-if="isLoading" size="xl" class="mt-5" />
+    <div v-else>
+      <div :class="$options.classes.headerClasses">
+        <div>
+          <h3>{{ name }}</h3>
+          <h4 class="gl-text-gray-700">{{ $options.translations.userIdLabel }}</h4>
+        </div>
+      </div>
+      <div v-if="hasUserIds">
+        <div :class="$options.classes.tableHeaderClasses">
+          {{ $options.translations.userIdColumnHeader }}
+        </div>
+        <div
+          v-for="id in userIds"
+          :key="id"
+          data-testid="user-id-row"
+          :class="$options.classes.tableRowClasses"
+        >
+          <span data-testid="user-id">{{ id }}</span>
+        </div>
+      </div>
+      <gl-empty-state
+        v-else
+        :title="$options.translations.emptyStateTitle"
+        :description="$options.translations.emptyStateDescription"
+        :svg-path="emptyStatePath"
+      />
+    </div>
+  </div>
+</template>

ee/app/assets/javascripts/user_lists/constants/show.js

+export default Object.freeze({
+  LOADING: 'LOADING',
+  SUCCESS: 'SUCCESS',
+  ERROR: 'ERROR',
+  ERROR_DISMISSED: 'ERROR_DISMISSED',
+});

ee/app/assets/javascripts/user_lists/store/show/actions.js

+import Api from 'ee/api';
+import * as types from './mutation_types';
+
+export const fetchUserList = ({ commit, state }) => {
+  commit(types.REQUEST_USER_LIST);
+  return Api.fetchFeatureFlagUserList(state.projectId, state.userListIid)
+    .then(response => commit(types.RECEIVE_USER_LIST_SUCCESS, response.data))
+    .catch(() => commit(types.RECEIVE_USER_LIST_ERROR));
+};
+
+export const dismissErrorAlert = ({ commit }) => commit(types.DISMISS_ERROR_ALERT);

ee/app/assets/javascripts/user_lists/store/show/index.js

+import Vuex from 'vuex';
+import createState from './state';
+import * as actions from './actions';
+import mutations from './mutations';
+
+export default initialState =>
+  new Vuex.Store({
+    actions,
+    mutations,
+    state: createState(initialState),
+  });

ee/app/assets/javascripts/user_lists/store/show/mutation_types.js

+export const REQUEST_USER_LIST = 'REQUEST_USER_LIST';
+export const RECEIVE_USER_LIST_SUCCESS = 'RECEIVE_USER_LIST_SUCCESS';
+export const RECEIVE_USER_LIST_ERROR = 'RECEIVE_USER_LIST_ERROR';
+
+export const DISMISS_ERROR_ALERT = 'DISMISS_ERROR_ALERT';

ee/app/assets/javascripts/user_lists/store/show/mutations.js

+import states from '../../constants/show';
+import * as types from './mutation_types';
+
+export default {
+  [types.REQUEST_USER_LIST](state) {
+    state.state = states.LOADING;
+  },
+  [types.RECEIVE_USER_LIST_SUCCESS](state, userList) {
+    state.state = states.SUCCESS;
+    state.userIds = userList.user_xids?.length > 0 ? userList.user_xids.split(',') : [];
+    state.userList = userList;
+  },
+  [types.RECEIVE_USER_LIST_ERROR](state) {
+    state.state = states.ERROR;
+  },
+  [types.DISMISS_ERROR_ALERT](state) {
+    state.state = states.ERROR_DISMISSED;
+  },
+};

ee/app/assets/javascripts/user_lists/store/show/state.js

+import states from '../../constants/show';
+
+export default ({ projectId = '', userListIid = '' }) => ({
+  state: states.LOADING,
+  projectId,
+  userListIid,
+  userIds: [],
+  userList: null,
+});

ee/app/controllers/projects/feature_flags_user_lists_controller.rb

 # frozen_string_literal: true
 
 class Projects::FeatureFlagsUserListsController < Projects::ApplicationController
-  before_action :check_feature_flag!
+  before_action :check_feature_flag!, only: [:new, :edit]
   before_action :authorize_admin_feature_flags_user_lists!
   before_action :user_list, only: [:edit, :show]
 

ee/app/views/projects/feature_flags_user_lists/show.html.haml

@@ -2,6 +2,6 @@
 - breadcrumb_title s_('FeatureFlags|List details')
 - page_title s_('FeatureFlags|Feature Flag User List Details')
 
-List details
-
-%p= @user_list.name
+#js-edit-user-list{ data: { project_id: @project.id,
+  user_list_iid: @user_list.iid,
+  empty_state_path: image_path('illustrations/feature_flag.svg') } }

ee/changelogs/unreleased/show-user-list-fe.yml

+---
+title: Show User List Details
+merge_request: 35369
+author:
+type: added

ee/lib/ee/api/entities/feature_flag/user_list.rb

@@ -5,6 +5,8 @@ module EE
     module Entities
       class FeatureFlag < Grape::Entity
         class UserList < Grape::Entity
+          include RequestAwareEntity
+
           expose :id
           expose :iid
           expose :project_id
@@ -12,6 +14,10 @@ module EE
           expose :updated_at
           expose :name
           expose :user_xids
+
+          expose :path do |list|
+            project_feature_flags_user_list_path(list.project, list)
+          end
         end
       end
     end

ee/spec/controllers/projects/feature_flags_user_lists_controller_spec.rb

@@ -140,14 +140,5 @@ RSpec.describe Projects::FeatureFlagsUserListsController do
 
       expect(response).to have_gitlab_http_status(:not_found)
     end
-
-    it 'returns not found when the feature flag is off' do
-      stub_feature_flags(feature_flag_user_lists: false)
-      list = create(:operations_feature_flag_user_list, project: project)
-
-      get(:show, params: request_params(iid: list.iid))
-
-      expect(response).to have_gitlab_http_status(:not_found)
-    end
   end
 end

ee/spec/frontend/feature_flags/components/user_lists_table_spec.js

@@ -49,6 +49,12 @@ describe('User Lists Table', () => {
     });
   });
 
+  describe('edit button', () => {
+    it('should link to the path for the user list', () => {
+      expect(wrapper.find('[data-testid="edit-user-list"]').attributes('href')).toBe(userList.path);
+    });
+  });
+
   describe('delete button', () => {
     it('should display the confirmation modal', () => {
       const modal = wrapper.find(GlModal);

ee/spec/frontend/feature_flags/mock_data.js

@@ -98,4 +98,5 @@ export const userList = {
   project_id: 1,
   created_at: '2020-02-04T08:13:10.507Z',
   updated_at: '2020-02-04T08:13:10.507Z',
+  path: '/path/to/user/list',
 };

ee/spec/frontend/user_lists/components/user_list_spec.js

+import Vue from 'vue';
+import Vuex from 'vuex';
+import { mount } from '@vue/test-utils';
+import { GlAlert, GlEmptyState, GlLoadingIcon } from '@gitlab/ui';
+import Api from 'ee/api';
+import { userList } from '../../feature_flags/mock_data';
+import createStore from 'ee/user_lists/store/show';
+import UserList from 'ee/user_lists/components/user_list.vue';
+
+jest.mock('ee/api');
+
+Vue.use(Vuex);
+
+describe('User List', () => {
+  let wrapper;
+
+  const findUserIds = () => wrapper.findAll('[data-testid="user-id"]');
+
+  const destroy = () => wrapper?.destroy();
+
+  const factory = () => {
+    destroy();
+
+    wrapper = mount(UserList, {
+      store: createStore({ projectId: '1', userListIid: '2' }),
+      propsData: {
+        emptyStatePath: '/empty_state.svg',
+      },
+    });
+  };
+
+  describe('loading', () => {
+    let resolveFn;
+
+    beforeEach(() => {
+      Api.fetchFeatureFlagUserList.mockReturnValue(
+        new Promise(resolve => {
+          resolveFn = resolve;
+        }),
+      );
+      factory();
+    });
+
+    afterEach(() => {
+      resolveFn();
+    });
+
+    it('shows a loading icon', () => {
+      expect(wrapper.find(GlLoadingIcon).exists()).toBe(true);
+    });
+  });
+
+  describe('success', () => {
+    let userIds;
+
+    beforeEach(() => {
+      userIds = userList.user_xids.split(',');
+      Api.fetchFeatureFlagUserList.mockResolvedValueOnce({ data: userList });
+      factory();
+
+      return wrapper.vm.$nextTick();
+    });
+
+    it('requests the user list on mount', () => {
+      expect(Api.fetchFeatureFlagUserList).toHaveBeenCalledWith('1', '2');
+    });
+
+    it('shows the list name', () => {
+      expect(wrapper.find('h3').text()).toBe(userList.name);
+    });
+
+    it('shows a row for every id', () => {
+      expect(wrapper.findAll('[data-testid="user-id-row"]')).toHaveLength(userIds.length);
+    });
+
+    it('shows one id on each row', () => {
+      findUserIds().wrappers.forEach((w, i) => expect(w.text()).toBe(userIds[i]));
+    });
+  });
+
+  describe('error', () => {
+    const findAlert = () => wrapper.find(GlAlert);
+
+    beforeEach(() => {
+      Api.fetchFeatureFlagUserList.mockRejectedValue();
+      factory();
+
+      return wrapper.vm.$nextTick();
+    });
+
+    it('displays the alert message', () => {
+      const alert = findAlert();
+      expect(alert.text()).toBe('Something went wrong on our end. Please try again!');
+    });
+
+    it('can dismiss the alert', async () => {
+      const alert = findAlert();
+      alert.find('button').trigger('click');
+
+      await wrapper.vm.$nextTick();
+
+      expect(alert.exists()).toBe(false);
+    });
+  });
+
+  describe('empty list', () => {
+    beforeEach(() => {
+      Api.fetchFeatureFlagUserList.mockResolvedValueOnce({ data: { ...userList, user_xids: '' } });
+      factory();
+
+      return wrapper.vm.$nextTick();
+    });
+
+    it('displays an empty state', () => {
+      expect(wrapper.find(GlEmptyState).exists()).toBe(true);
+    });
+  });
+});

ee/spec/frontend/user_lists/store/show/actions_spec.js

+import testAction from 'helpers/vuex_action_helper';
+import Api from 'ee/api';
+import createState from 'ee/user_lists/store/show/state';
+import * as types from 'ee/user_lists/store/show/mutation_types';
+import * as actions from 'ee/user_lists/store/show/actions';
+import { userList } from 'ee_jest/feature_flags/mock_data';
+
+jest.mock('ee/api');
+
+describe('User Lists Show Actions', () => {
+  let mockState;
+
+  beforeEach(() => {
+    mockState = createState({ projectId: '1', userListIid: '2' });
+  });
+
+  describe('fetchUserList', () => {
+    it('commits REQUEST_USER_LIST and RECEIVE_USER_LIST_SUCCESS on success', () => {
+      Api.fetchFeatureFlagUserList.mockResolvedValue({ data: userList });
+      return testAction(
+        actions.fetchUserList,
+        undefined,
+        mockState,
+        [
+          { type: types.REQUEST_USER_LIST },
+          { type: types.RECEIVE_USER_LIST_SUCCESS, payload: userList },
+        ],
+        [],
+        () => expect(Api.fetchFeatureFlagUserList).toHaveBeenCalledWith('1', '2'),
+      );
+    });
+
+    it('commits REQUEST_USER_LIST and RECEIVE_USER_LIST_ERROR on error', () => {
+      Api.fetchFeatureFlagUserList.mockRejectedValue({ message: 'fail' });
+      return testAction(
+        actions.fetchUserList,
+        undefined,
+        mockState,
+        [{ type: types.REQUEST_USER_LIST }, { type: types.RECEIVE_USER_LIST_ERROR }],
+        [],
+      );
+    });
+  });
+
+  describe('dismissErrorAlert', () => {
+    it('commits DISMISS_ERROR_ALERT', () => {
+      return testAction(
+        actions.dismissErrorAlert,
+        undefined,
+        mockState,
+        [{ type: types.DISMISS_ERROR_ALERT }],
+        [],
+      );
+    });
+  });
+});

ee/spec/frontend/user_lists/store/show/mutations_spec.js

+import createState from 'ee/user_lists/store/show/state';
+import mutations from 'ee/user_lists/store/show/mutations';
+import states from 'ee/user_lists/constants/show';
+import * as types from 'ee/user_lists/store/show/mutation_types';
+import { userList } from 'ee_jest/feature_flags/mock_data';
+
+describe('User Lists Show Mutations', () => {
+  let mockState;
+
+  beforeEach(() => {
+    mockState = createState({ projectId: '1', userListIid: '2' });
+  });
+
+  describe(types.REQUEST_USER_LIST, () => {
+    it('puts us in the loading state', () => {
+      mutations[types.REQUEST_USER_LIST](mockState);
+
+      expect(mockState.state).toBe(states.LOADING);
+    });
+  });
+
+  describe(types.RECEIVE_USER_LIST_SUCCESS, () => {
+    beforeEach(() => {
+      mutations[types.RECEIVE_USER_LIST_SUCCESS](mockState, userList);
+    });
+
+    it('sets the state to LOADED', () => {
+      expect(mockState.state).toBe(states.SUCCESS);
+    });
+
+    it('sets the active user list', () => {
+      expect(mockState.userList).toEqual(userList);
+    });
+
+    it('splits the user IDs into an Array', () => {
+      expect(mockState.userIds).toEqual(userList.user_xids.split(','));
+    });
+
+    it('sets user IDs to an empty Array if an empty string is received', () => {
+      mutations[types.RECEIVE_USER_LIST_SUCCESS](mockState, { ...userList, user_xids: '' });
+      expect(mockState.userIds).toEqual([]);
+    });
+  });
+  describe(types.RECEIVE_USER_LIST_ERROR, () => {
+    it('sets the state to error', () => {
+      mutations[types.RECEIVE_USER_LIST_ERROR](mockState);
+      expect(mockState.state).toBe(states.ERROR);
+    });
+  });
+});

ee/spec/requests/api/feature_flags_user_lists_spec.rb

@@ -58,7 +58,8 @@ RSpec.describe API::FeatureFlagsUserLists do
         'created_at' => user_list.created_at.as_json,
         'updated_at' => user_list.updated_at.as_json,
         'name' => 'list_a',
-        'user_xids' => 'user1'
+        'user_xids' => 'user1',
+        'path' => project_feature_flags_user_list_path(user_list.project, user_list)
       }])
     end
 
@@ -122,7 +123,8 @@ RSpec.describe API::FeatureFlagsUserLists do
         'iid' => list.iid,
         'project_id' => project.id,
         'created_at' => list.created_at.as_json,
-        'updated_at' => list.updated_at.as_json
+        'updated_at' => list.updated_at.as_json,
+        'path' => project_feature_flags_user_list_path(list.project, list)
       })
     end
 

locale/gitlab.pot

@@ -25172,6 +25172,18 @@ msgstr ""
 msgid "User was successfully updated."
 msgstr ""
 
+msgid "UserLists|Define a set of users to be used within feature flag strategies"
+msgstr ""
+
+msgid "UserLists|There are no users"
+msgstr ""
+
+msgid "UserLists|User ID"
+msgstr ""
+
+msgid "UserLists|User IDs"
+msgstr ""
+
 msgid "UserList|Delete %{name}?"
 msgstr ""