Merge branch '346939-frontend-issues-list-save-sort-order' into 'master'

Save sort order on group/project issues list refactor

See merge request gitlab-org/gitlab!79532

app/assets/javascripts/issues/list/components/issues_list_app.vue

@@ -28,7 +28,6 @@ import {
   MAX_LIST_SIZE,
   PAGE_SIZE,
   PARAM_DUE_DATE,
-  PARAM_SORT,
   PARAM_STATE,
   RELATIVE_POSITION_ASC,
   TOKEN_TYPE_ASSIGNEE,
@@ -69,6 +68,7 @@ import {
 } from '~/vue_shared/components/filtered_search_bar/constants';
 import eventHub from '../eventhub';
 import reorderIssuesMutation from '../queries/reorder_issues.mutation.graphql';
+import setSortPreferenceMutation from '../queries/set_sort_preference.mutation.graphql';
 import searchLabelsQuery from '../queries/search_labels.query.graphql';
 import searchMilestonesQuery from '../queries/search_milestones.query.graphql';
 import searchUsersQuery from '../queries/search_users.query.graphql';
@@ -140,6 +140,9 @@ export default {
     initialEmail: {
       default: '',
     },
+    initialSort: {
+      default: '',
+    },
     isAnonymousSearchDisabled: {
       default: false,
     },
@@ -181,7 +184,12 @@ export default {
   data() {
     const state = getParameterByName(PARAM_STATE);
     const defaultSortKey = state === IssuableStates.Closed ? UPDATED_DESC : CREATED_DESC;
-    let sortKey = getSortKey(getParameterByName(PARAM_SORT)) || defaultSortKey;
+    const dashboardSortKey = getSortKey(this.initialSort);
+    const graphQLSortKey = this.initialSort?.toUpperCase();
+
+    // The initial sort is an old enum value when it is saved on the dashboard issues page.
+    // The initial sort is a GraphQL enum value when it is saved on the Vue issues list page.
+    let sortKey = dashboardSortKey || graphQLSortKey || defaultSortKey;
 
     if (this.isIssueRepositioningDisabled && sortKey === RELATIVE_POSITION_ASC) {
       this.showIssueRepositioningMessage();
@@ -608,6 +616,25 @@ export default {
         this.pageParams = getInitialPageParams(sortKey);
       }
       this.sortKey = sortKey;
+
+      if (this.isSignedIn) {
+        this.saveSortPreference(sortKey);
+      }
+    },
+    saveSortPreference(sortKey) {
+      this.$apollo
+        .mutate({
+          mutation: setSortPreferenceMutation,
+          variables: { input: { issuesSort: sortKey } },
+        })
+        .then(({ data }) => {
+          if (data.userPreferencesUpdate.errors.length) {
+            throw new Error(data.userPreferencesUpdate.errors);
+          }
+        })
+        .catch((error) => {
+          Sentry.captureException(error);
+        });
     },
     showAnonymousSearchingMessage() {
       createFlash({

app/assets/javascripts/issues/list/index.js

@@ -99,6 +99,7 @@ export function mountIssuesListApp() {
     hasMultipleIssueAssigneesFeature,
     importCsvIssuesPath,
     initialEmail,
+    initialSort,
     isAnonymousSearchDisabled,
     isIssueRepositioningDisabled,
     isProject,
@@ -133,6 +134,7 @@ export function mountIssuesListApp() {
       hasIssueWeightsFeature: parseBoolean(hasIssueWeightsFeature),
       hasIterationsFeature: parseBoolean(hasIterationsFeature),
       hasMultipleIssueAssigneesFeature: parseBoolean(hasMultipleIssueAssigneesFeature),
+      initialSort,
       isAnonymousSearchDisabled: parseBoolean(isAnonymousSearchDisabled),
       isIssueRepositioningDisabled: parseBoolean(isIssueRepositioningDisabled),
       isProject: parseBoolean(isProject),

app/assets/javascripts/issues/list/queries/set_sort_preference.mutation.graphql

+mutation setSortPreference($input: UserPreferencesUpdateInput!) {
+  userPreferencesUpdate(input: $input) {
+    errors
+  }
+}

app/helpers/issues_helper.rb

@@ -199,6 +199,7 @@ module IssuesHelper
       calendar_path: url_for(safe_params.merge(calendar_url_options)),
       empty_state_svg_path: image_path('illustrations/issues.svg'),
       full_path: namespace.full_path,
+      initial_sort: current_user&.user_preference&.issues_sort,
       is_anonymous_search_disabled: Feature.enabled?(:disable_anonymous_search, type: :ops).to_s,
       is_issue_repositioning_disabled: issue_repositioning_disabled?.to_s,
       is_signed_in: current_user.present?.to_s,

spec/features/user_sorts_things_spec.rb

@@ -33,18 +33,6 @@ RSpec.describe "User sorts things" do
     expect(find(".issues-filters")).to have_content(sort_option)
   end
 
-  it "issues -> merge requests" do
-    sort_option = 'Updated date'
-
-    visit(project_issues_path(project))
-
-    sort_by(sort_option)
-
-    visit(project_merge_requests_path(project))
-
-    expect(find(".issues-filters")).to have_content(sort_option)
-  end
-
   it "merge requests -> dashboard merge requests" do
     sort_option = 'Updated date'
 

spec/frontend/issues/list/components/issues_list_app_spec.js

@@ -16,6 +16,8 @@ import {
   getIssuesQueryResponse,
   filteredTokens,
   locationSearch,
+  setSortPreferenceMutationResponse,
+  setSortPreferenceMutationResponseWithErrors,
   urlParams,
 } from 'jest/issues/list/mock_data';
 import createFlash, { FLASH_TYPES } from '~/flash';
@@ -43,16 +45,15 @@ import {
   urlSortParams,
 } from '~/issues/list/constants';
 import eventHub from '~/issues/list/eventhub';
-import { getSortOptions } from '~/issues/list/utils';
+import setSortPreferenceMutation from '~/issues/list/queries/set_sort_preference.mutation.graphql';
+import { getSortKey, getSortOptions } from '~/issues/list/utils';
 import axios from '~/lib/utils/axios_utils';
 import { scrollUp } from '~/lib/utils/scroll_utils';
 import { joinPaths } from '~/lib/utils/url_utility';
 
 jest.mock('@sentry/browser');
 jest.mock('~/flash');
-jest.mock('~/lib/utils/scroll_utils', () => ({
-  scrollUp: jest.fn().mockName('scrollUpMock'),
-}));
+jest.mock('~/lib/utils/scroll_utils', () => ({ scrollUp: jest.fn() }));
 
 describe('CE IssuesListApp component', () => {
   let axiosMock;
@@ -103,11 +104,13 @@ describe('CE IssuesListApp component', () => {
     provide = {},
     issuesQueryResponse = jest.fn().mockResolvedValue(defaultQueryResponse),
     issuesCountsQueryResponse = jest.fn().mockResolvedValue(getIssuesCountsQueryResponse),
+    sortPreferenceMutationResponse = jest.fn().mockResolvedValue(setSortPreferenceMutationResponse),
     mountFn = shallowMount,
   } = {}) => {
     const requestHandlers = [
       [getIssuesQuery, issuesQueryResponse],
       [getIssuesCountsQuery, issuesCountsQueryResponse],
+      [setSortPreferenceMutation, sortPreferenceMutationResponse],
     ];
     const apolloProvider = createMockApollo(requestHandlers);
 
@@ -192,12 +195,13 @@ describe('CE IssuesListApp component', () => {
 
     describe('csv import/export component', () => {
       describe('when user is signed in', () => {
-        const search = '?search=refactor&sort=created_date&state=opened';
-
         beforeEach(async () => {
-          setWindowLocation(search);
+          setWindowLocation('?search=refactor&state=opened');
 
-          wrapper = mountComponent({ provide: { isSignedIn: true }, mountFn: mount });
+          wrapper = mountComponent({
+            provide: { initialSortBy: CREATED_DESC, isSignedIn: true },
+            mountFn: mount,
+          });
 
           jest.runOnlyPendingTimers();
           await waitForPromises();
@@ -205,7 +209,7 @@ describe('CE IssuesListApp component', () => {
 
         it('renders', () => {
           expect(findCsvImportExportButtons().props()).toMatchObject({
-            exportCsvPath: `${defaultProvide.exportCsvPath}${search}`,
+            exportCsvPath: `${defaultProvide.exportCsvPath}?search=refactor&sort=created_date&state=opened`,
             issuableCount: 1,
           });
         });
@@ -306,31 +310,43 @@ describe('CE IssuesListApp component', () => {
     });
 
     describe('sort', () => {
-      it.each(Object.keys(urlSortParams))('is set as %s from the url params', (sortKey) => {
-        setWindowLocation(`?sort=${urlSortParams[sortKey]}`);
+      describe('when initial sort value uses old enum values', () => {
+        const oldEnumSortValues = Object.values(urlSortParams);
 
-        wrapper = mountComponent();
+        it.each(oldEnumSortValues)('initial sort is set with value %s', (sort) => {
+          wrapper = mountComponent({ provide: { initialSort: sort } });
 
-        expect(findIssuableList().props()).toMatchObject({
-          initialSortBy: sortKey,
-          urlParams: {
-            sort: urlSortParams[sortKey],
-          },
+          expect(findIssuableList().props()).toMatchObject({
+            initialSortBy: getSortKey(sort),
+            urlParams: { sort },
+          });
+        });
+      });
+
+      describe('when initial sort value uses new GraphQL enum values', () => {
+        const graphQLEnumSortValues = Object.keys(urlSortParams);
+
+        it.each(graphQLEnumSortValues)('initial sort is set with value %s', (sort) => {
+          wrapper = mountComponent({ provide: { initialSort: sort.toLowerCase() } });
+
+          expect(findIssuableList().props()).toMatchObject({
+            initialSortBy: sort,
+            urlParams: { sort: urlSortParams[sort] },
+          });
         });
       });
 
-      describe('when issue repositioning is disabled and the sort is manual', () => {
+      describe('when sort is manual and issue repositioning is disabled', () => {
         beforeEach(() => {
-          setWindowLocation(`?sort=${RELATIVE_POSITION}`);
-          wrapper = mountComponent({ provide: { isIssueRepositioningDisabled: true } });
+          wrapper = mountComponent({
+            provide: { initialSort: RELATIVE_POSITION, isIssueRepositioningDisabled: true },
+          });
         });
 
         it('changes the sort to the default of created descending', () => {
           expect(findIssuableList().props()).toMatchObject({
             initialSortBy: CREATED_DESC,
-            urlParams: {
-              sort: urlSortParams[CREATED_DESC],
-            },
+            urlParams: { sort: urlSortParams[CREATED_DESC] },
           });
         });
 
@@ -763,8 +779,9 @@ describe('CE IssuesListApp component', () => {
         const initialSort = CREATED_DESC;
 
         beforeEach(() => {
-          setWindowLocation(`?sort=${initialSort}`);
-          wrapper = mountComponent({ provide: { isIssueRepositioningDisabled: true } });
+          wrapper = mountComponent({
+            provide: { initialSort, isIssueRepositioningDisabled: true },
+          });
 
           findIssuableList().vm.$emit('sort', RELATIVE_POSITION_ASC);
         });
@@ -782,6 +799,43 @@ describe('CE IssuesListApp component', () => {
           });
         });
       });
+
+      describe('when user is signed in', () => {
+        it('calls mutation to save sort preference', () => {
+          const mutationMock = jest.fn().mockResolvedValue(setSortPreferenceMutationResponse);
+          wrapper = mountComponent({ sortPreferenceMutationResponse: mutationMock });
+
+          findIssuableList().vm.$emit('sort', CREATED_DESC);
+
+          expect(mutationMock).toHaveBeenCalledWith({ input: { issuesSort: CREATED_DESC } });
+        });
+
+        it('captures error when mutation response has errors', async () => {
+          const mutationMock = jest
+            .fn()
+            .mockResolvedValue(setSortPreferenceMutationResponseWithErrors);
+          wrapper = mountComponent({ sortPreferenceMutationResponse: mutationMock });
+
+          findIssuableList().vm.$emit('sort', CREATED_DESC);
+          await waitForPromises();
+
+          expect(Sentry.captureException).toHaveBeenCalledWith(new Error('oh no!'));
+        });
+      });
+
+      describe('when user is signed out', () => {
+        it('does not call mutation to save sort preference', () => {
+          const mutationMock = jest.fn().mockResolvedValue(setSortPreferenceMutationResponse);
+          wrapper = mountComponent({
+            provide: { isSignedIn: false },
+            sortPreferenceMutationResponse: mutationMock,
+          });
+
+          findIssuableList().vm.$emit('sort', CREATED_DESC);
+
+          expect(mutationMock).not.toHaveBeenCalled();
+        });
+      });
     });
 
     describe('when "update-legacy-bulk-edit" event is emitted by IssuableList', () => {

spec/frontend/issues/list/mock_data.js

@@ -90,6 +90,22 @@ export const getIssuesCountsQueryResponse = {
   },
 };
 
+export const setSortPreferenceMutationResponse = {
+  data: {
+    userPreferencesUpdate: {
+      errors: [],
+    },
+  },
+};
+
+export const setSortPreferenceMutationResponseWithErrors = {
+  data: {
+    userPreferencesUpdate: {
+      errors: ['oh no!'],
+    },
+  },
+};
+
 export const locationSearch = [
   '?search=find+issues',
   'author_username=homer',

spec/helpers/issues_helper_spec.rb

@@ -300,6 +300,7 @@ RSpec.describe IssuesHelper do
         has_any_issues: project_issues(project).exists?.to_s,
         import_csv_issues_path: '#',
         initial_email: project.new_issuable_address(current_user, 'issue'),
+        initial_sort: current_user&.user_preference&.issues_sort,
         is_anonymous_search_disabled: 'true',
         is_issue_repositioning_disabled: 'true',
         is_project: 'true',