Skip to content

G
gitlab-ce
Commit 55105bff authored by Coung Ngo's avatar Coung Ngo Committed by Andrew Fontaine
Browse files
Options

Fix sort params in issues page refactor

parent 6b58586e
Show whitespace changes
Inline Side-by-side
Showing with 141 additions and 92 deletions
app/assets/javascripts/issues_list/components/issues_list_app.vue
View file @ 55105bff
......@@ -16,12 +16,17 @@ import IssuableByEmail from '~/issuable/components/issuable_by_email.vue';
import IssuableList from '~/issuable_list/components/issuable_list_root.vue';
import { IssuableListTabs, IssuableStates } from '~/issuable_list/constants';
import {
apiSortParams,
CREATED_DESC,
i18n,
MAX_LIST_SIZE,
PAGE_SIZE,
RELATIVE_POSITION_ASC,
sortParams,
PARAM_PAGE,
PARAM_SORT,
PARAM_STATE,
RELATIVE_POSITION_DESC,
UPDATED_DESC,
urlSortParams,
} from '~/issues_list/constants';
import {
convertToApiParams,
......@@ -46,11 +51,8 @@ import eventHub from '../eventhub';
import IssueCardTimeInfo from './issue_card_time_info.vue';
export default {
CREATED_DESC,
i18n,
IssuableListTabs,
PAGE_SIZE,
sortParams,
components: {
CsvImportExportButtons,
GlButton,
......@@ -138,18 +140,18 @@ export default {
},
},
data() {
const orderBy = getParameterByName('order_by');
const sort = getParameterByName('sort');
const state = getParameterByName(PARAM_STATE);
const defaultSortKey = state === IssuableStates.Closed ? UPDATED_DESC : CREATED_DESC;
return {
exportCsvPathWithQuery: this.getExportCsvPathWithQuery(),
filterTokens: getFilterTokens(window.location.search),
isLoading: false,
issues: [],
page: toNumber(getParameterByName('page')) || 1,
page: toNumber(getParameterByName(PARAM_PAGE)) || 1,
showBulkEditSidebar: false,
sortKey: getSortKey(orderBy, sort) || CREATED_DESC,
state: getParameterByName('state') || IssuableStates.Opened,
sortKey: getSortKey(getParameterByName(PARAM_SORT)) || defaultSortKey,
state: state || IssuableStates.Opened,
totalIssues: 0,
};
},
......@@ -158,7 +160,7 @@ export default {
return this.showBulkEditSidebar || !this.issues.length;
},
isManualOrdering() {
return this.sortKey === RELATIVE_POSITION_ASC;
return this.sortKey === RELATIVE_POSITION_DESC;
},
isOpenTab() {
return this.state === IssuableStates.Opened;
......@@ -288,7 +290,7 @@ export default {
page: this.page,
search: this.searchQuery,
state: this.state,
...sortParams[this.sortKey],
...urlSortParams[this.sortKey],
...this.urlFilterParams,
};
},
......@@ -354,11 +356,11 @@ export default {
.get(this.endpoint, {
params: {
page: this.page,
per_page: this.$options.PAGE_SIZE,
per_page: PAGE_SIZE,
search: this.searchQuery,
state: this.state,
with_labels_details: true,
...sortParams[this.sortKey],
...apiSortParams[this.sortKey],
...this.apiFilterParams,
},
})
......
app/assets/javascripts/issues_list/constants.js
View file @ 55105bff
......@@ -88,21 +88,22 @@ export const i18n = {
export const JIRA_IMPORT_SUCCESS_ALERT_HIDE_MAP_KEY = 'jira-import-success-alert-hide-map';
export const BLOCKING_ISSUES_ASC = 'BLOCKING_ISSUES_ASC';
export const PARAM_PAGE = 'page';
export const PARAM_SORT = 'sort';
export const PARAM_STATE = 'state';
export const BLOCKING_ISSUES_DESC = 'BLOCKING_ISSUES_DESC';
export const CREATED_ASC = 'CREATED_ASC';
export const CREATED_DESC = 'CREATED_DESC';
export const DUE_DATE_ASC = 'DUE_DATE_ASC';
export const DUE_DATE_DESC = 'DUE_DATE_DESC';
export const LABEL_PRIORITY_ASC = 'LABEL_PRIORITY_ASC';
export const LABEL_PRIORITY_DESC = 'LABEL_PRIORITY_DESC';
export const MILESTONE_DUE_ASC = 'MILESTONE_DUE_ASC';
export const MILESTONE_DUE_DESC = 'MILESTONE_DUE_DESC';
export const POPULARITY_ASC = 'POPULARITY_ASC';
export const POPULARITY_DESC = 'POPULARITY_DESC';
export const PRIORITY_ASC = 'PRIORITY_ASC';
export const PRIORITY_DESC = 'PRIORITY_DESC';
export const RELATIVE_POSITION_ASC = 'RELATIVE_POSITION_ASC';
export const RELATIVE_POSITION_DESC = 'RELATIVE_POSITION_DESC';
export const UPDATED_ASC = 'UPDATED_ASC';
export const UPDATED_DESC = 'UPDATED_DESC';
export const WEIGHT_ASC = 'WEIGHT_ASC';
......@@ -111,13 +112,19 @@ export const WEIGHT_DESC = 'WEIGHT_DESC';
const SORT_ASC = 'asc';
const SORT_DESC = 'desc';
const CREATED_DATE_SORT = 'created_date';
const CREATED_ASC_SORT = 'created_asc';
const UPDATED_DESC_SORT = 'updated_desc';
const UPDATED_ASC_SORT = 'updated_asc';
const MILESTONE_SORT = 'milestone';
const MILESTONE_DUE_DESC_SORT = 'milestone_due_desc';
const DUE_DATE_DESC_SORT = 'due_date_desc';
const POPULARITY_ASC_SORT = 'popularity_asc';
const WEIGHT_DESC_SORT = 'weight_desc';
const BLOCKING_ISSUES_DESC_SORT = 'blocking_issues_desc';
const BLOCKING_ISSUES = 'blocking_issues';
export const sortParams = {
[PRIORITY_ASC]: {
order_by: PRIORITY,
sort: SORT_ASC,
},
export const apiSortParams = {
[PRIORITY_DESC]: {
order_by: PRIORITY,
sort: SORT_DESC,
......@@ -162,15 +169,11 @@ export const sortParams = {
order_by: POPULARITY,
sort: SORT_DESC,
},
[LABEL_PRIORITY_ASC]: {
order_by: LABEL_PRIORITY,
sort: SORT_ASC,
},
[LABEL_PRIORITY_DESC]: {
order_by: LABEL_PRIORITY,
sort: SORT_DESC,
},
[RELATIVE_POSITION_ASC]: {
[RELATIVE_POSITION_DESC]: {
order_by: RELATIVE_POSITION,
per_page: 100,
sort: SORT_ASC,
......@@ -183,16 +186,64 @@ export const sortParams = {
order_by: WEIGHT,
sort: SORT_DESC,
},
[BLOCKING_ISSUES_ASC]: {
order_by: BLOCKING_ISSUES,
sort: SORT_ASC,
},
[BLOCKING_ISSUES_DESC]: {
order_by: BLOCKING_ISSUES,
sort: SORT_DESC,
},
};
export const urlSortParams = {
[PRIORITY_DESC]: {
sort: PRIORITY,
},
[CREATED_ASC]: {
sort: CREATED_ASC_SORT,
},
[CREATED_DESC]: {
sort: CREATED_DATE_SORT,
},
[UPDATED_ASC]: {
sort: UPDATED_ASC_SORT,
},
[UPDATED_DESC]: {
sort: UPDATED_DESC_SORT,
},
[MILESTONE_DUE_ASC]: {
sort: MILESTONE_SORT,
},
[MILESTONE_DUE_DESC]: {
sort: MILESTONE_DUE_DESC_SORT,
},
[DUE_DATE_ASC]: {
sort: DUE_DATE,
},
[DUE_DATE_DESC]: {
sort: DUE_DATE_DESC_SORT,
},
[POPULARITY_ASC]: {
sort: POPULARITY_ASC_SORT,
},
[POPULARITY_DESC]: {
sort: POPULARITY,
},
[LABEL_PRIORITY_DESC]: {
sort: LABEL_PRIORITY,
},
[RELATIVE_POSITION_DESC]: {
sort: RELATIVE_POSITION,
per_page: 100,
},
[WEIGHT_ASC]: {
sort: WEIGHT,
},
[WEIGHT_DESC]: {
sort: WEIGHT_DESC_SORT,
},
[BLOCKING_ISSUES_DESC]: {
sort: BLOCKING_ISSUES_DESC_SORT,
},
};
export const MAX_LIST_SIZE = 10;
export const FILTERED_SEARCH_TERM = 'filtered-search-term';
......
app/assets/javascripts/issues_list/utils.js
View file @ 55105bff
import {
BLOCKING_ISSUES_ASC,
BLOCKING_ISSUES_DESC,
CREATED_ASC,
CREATED_DESC,
......@@ -7,30 +6,26 @@ import {
DUE_DATE_DESC,
FILTERED_SEARCH_TERM,
filters,
LABEL_PRIORITY_ASC,
LABEL_PRIORITY_DESC,
MILESTONE_DUE_ASC,
MILESTONE_DUE_DESC,
NORMAL_FILTER,
POPULARITY_ASC,
POPULARITY_DESC,
PRIORITY_ASC,
PRIORITY_DESC,
RELATIVE_POSITION_ASC,
sortParams,
RELATIVE_POSITION_DESC,
SPECIAL_FILTER,
SPECIAL_FILTER_VALUES,
UPDATED_ASC,
UPDATED_DESC,
urlSortParams,
WEIGHT_ASC,
WEIGHT_DESC,
} from '~/issues_list/constants';
import { __ } from '~/locale';
export const getSortKey = (orderBy, sort) =>
Object.keys(sortParams).find(
(key) => sortParams[key].order_by === orderBy && sortParams[key].sort === sort,
);
export const getSortKey = (sort) =>
Object.keys(urlSortParams).find((key) => urlSortParams[key].sort === sort);
export const getSortOptions = (hasIssueWeightsFeature, hasBlockedIssuesFeature) => {
const sortOptions = [
......@@ -38,7 +33,7 @@ export const getSortOptions = (hasIssueWeightsFeature, hasBlockedIssuesFeature)
id: 1,
title: __('Priority'),
sortDirection: {
ascending: PRIORITY_ASC,
ascending: PRIORITY_DESC,
descending: PRIORITY_DESC,
},
},
......@@ -86,7 +81,7 @@ export const getSortOptions = (hasIssueWeightsFeature, hasBlockedIssuesFeature)
id: 7,
title: __('Label priority'),
sortDirection: {
ascending: LABEL_PRIORITY_ASC,
ascending: LABEL_PRIORITY_DESC,
descending: LABEL_PRIORITY_DESC,
},
},
......@@ -94,8 +89,8 @@ export const getSortOptions = (hasIssueWeightsFeature, hasBlockedIssuesFeature)
id: 8,
title: __('Manual'),
sortDirection: {
ascending: RELATIVE_POSITION_ASC,
descending: RELATIVE_POSITION_ASC,
ascending: RELATIVE_POSITION_DESC,
descending: RELATIVE_POSITION_DESC,
},
},
];
......@@ -116,7 +111,7 @@ export const getSortOptions = (hasIssueWeightsFeature, hasBlockedIssuesFeature)
id: 10,
title: __('Blocking'),
sortDirection: {
ascending: BLOCKING_ISSUES_ASC,
ascending: BLOCKING_ISSUES_DESC,
descending: BLOCKING_ISSUES_DESC,
},
});
......
app/assets/javascripts/vue_shared/components/filtered_search_bar/filtered_search_bar_root.vue
View file @ 55105bff
......@@ -93,9 +93,9 @@ export default {
sortBy.sortDirection.descending === this.initialSortBy,
)
.pop();
selectedSortDirection = this.initialSortBy.endsWith('_desc')
? SortDirection.descending
: SortDirection.ascending;
selectedSortDirection = Object.keys(selectedSortOption.sortDirection).find(
(key) => selectedSortOption.sortDirection[key] === this.initialSortBy,
);
}
return {
......
spec/features/issuables/sorting_list_spec.rb
View file @ 55105bff
......@@ -57,7 +57,7 @@ RSpec.describe 'Sort Issuable List' do
it 'is "last updated"' do
visit_merge_requests_with_state(project, 'merged')
expect(find('.filter-dropdown-container')).to have_content('Last updated')
expect(page).to have_button 'Last updated'
expect(first_merge_request).to include(last_updated_issuable.title)
expect(last_merge_request).to include(first_updated_issuable.title)
end
......@@ -69,7 +69,7 @@ RSpec.describe 'Sort Issuable List' do
it 'is "last updated"' do
visit_merge_requests_with_state(project, 'closed')
expect(find('.filter-dropdown-container')).to have_content('Last updated')
expect(page).to have_button 'Last updated'
expect(first_merge_request).to include(last_updated_issuable.title)
expect(last_merge_request).to include(first_updated_issuable.title)
end
......@@ -81,7 +81,7 @@ RSpec.describe 'Sort Issuable List' do
it 'is "created date"' do
visit_merge_requests_with_state(project, 'all')
expect(find('.filter-dropdown-container')).to have_content('Created date')
expect(page).to have_button 'Created date'
expect(first_merge_request).to include(last_created_issuable.title)
expect(last_merge_request).to include(first_created_issuable.title)
end
......@@ -94,15 +94,13 @@ RSpec.describe 'Sort Issuable List' do
it 'supports sorting in asc and desc order' do
visit_merge_requests_with_state(project, 'open')
page.within('.filter-dropdown-container') do
click_button('Created date')
click_link('Last updated')
end
expect(first_merge_request).to include(last_updated_issuable.title)
expect(last_merge_request).to include(first_updated_issuable.title)
find('.filter-dropdown-container .rspec-reverse-sort').click
click_on 'Sort direction'
expect(first_merge_request).to include(first_updated_issuable.title)
expect(last_merge_request).to include(last_updated_issuable.title)
......@@ -133,7 +131,7 @@ RSpec.describe 'Sort Issuable List' do
it 'is "created date"' do
visit_issues project
expect(find('.filter-dropdown-container')).to have_content('Created date')
expect(page).to have_button 'Created date'
expect(first_issue).to include(last_created_issuable.title)
expect(last_issue).to include(first_created_issuable.title)
end
......@@ -145,7 +143,7 @@ RSpec.describe 'Sort Issuable List' do
it 'is "created date"' do
visit_issues_with_state(project, 'opened')
expect(find('.filter-dropdown-container')).to have_content('Created date')
expect(page).to have_button 'Created date'
expect(first_issue).to include(last_created_issuable.title)
expect(last_issue).to include(first_created_issuable.title)
end
......@@ -157,7 +155,7 @@ RSpec.describe 'Sort Issuable List' do
it 'is "last updated"' do
visit_issues_with_state(project, 'closed')
expect(find('.filter-dropdown-container')).to have_content('Last updated')
expect(page).to have_button 'Last updated'
expect(first_issue).to include(last_updated_issuable.title)
expect(last_issue).to include(first_updated_issuable.title)
end
......@@ -169,7 +167,7 @@ RSpec.describe 'Sort Issuable List' do
it 'is "created date"' do
visit_issues_with_state(project, 'all')
expect(find('.filter-dropdown-container')).to have_content('Created date')
expect(page).to have_button 'Created date'
expect(first_issue).to include(last_created_issuable.title)
expect(last_issue).to include(first_created_issuable.title)
end
......@@ -183,7 +181,7 @@ RSpec.describe 'Sort Issuable List' do
end
it 'shows the sort order as created date' do
expect(find('.filter-dropdown-container')).to have_content('Created date')
expect(page).to have_button 'Created date'
expect(first_issue).to include(last_created_issuable.title)
expect(last_issue).to include(first_created_issuable.title)
end
......@@ -196,15 +194,17 @@ RSpec.describe 'Sort Issuable List' do
it 'supports sorting in asc and desc order' do
visit_issues_with_state(project, 'opened')
page.within('.filter-dropdown-container') do
click_button('Created date')
click_link('Last updated')
end
click_on('Last updated')
wait_for_requests
expect(first_issue).to include(last_updated_issuable.title)
expect(last_issue).to include(first_updated_issuable.title)
find('.filter-dropdown-container .rspec-reverse-sort').click
click_on 'Sort direction'
wait_for_requests
expect(first_issue).to include(first_updated_issuable.title)
expect(last_issue).to include(last_updated_issuable.title)
......
spec/features/projects/labels/issues_sorted_by_priority_spec.rb
View file @ 55105bff
......@@ -33,16 +33,16 @@ RSpec.describe 'Issue prioritization' do
sign_in user
visit project_issues_path(project, sort: 'label_priority')
wait_for_requests
# Ensure we are indicating that issues are sorted by priority
expect(page).to have_selector('.dropdown', text: 'Label priority')
expect(page).to have_button 'Label priority'
page.within('.issues-holder') do
issue_titles = all('.issues-list .issue-title-text').map(&:text)
expect(issue_titles).to eq(%w(issue_4 issue_3 issue_5 issue_2 issue_1))
end
end
end
context 'when issues have multiple labels', :js do
it 'are sorted properly' do
......@@ -72,9 +72,10 @@ RSpec.describe 'Issue prioritization' do
sign_in user
visit project_issues_path(project, sort: 'label_priority')
expect(page).to have_selector('.dropdown', text: 'Label priority')
wait_for_requests
expect(page).to have_button 'Label priority'
page.within('.issues-holder') do
issue_titles = all('.issues-list .issue-title-text').map(&:text)
expect(issue_titles[0..1]).to contain_exactly('issue_5', 'issue_8')
......@@ -82,5 +83,4 @@ RSpec.describe 'Issue prioritization' do
expect(issue_titles[5..-1]).to eq(%w(issue_2 issue_4 issue_6))
end
end
end
end
spec/frontend/issues_list/components/issues_list_app_spec.js
View file @ 55105bff
......@@ -11,11 +11,12 @@ import IssuableList from '~/issuable_list/components/issuable_list_root.vue';
import { IssuableListTabs, IssuableStates } from '~/issuable_list/constants';
import IssuesListApp from '~/issues_list/components/issues_list_app.vue';
import {
apiSortParams,
CREATED_DESC,
PAGE_SIZE,
PAGE_SIZE_MANUAL,
RELATIVE_POSITION_ASC,
sortParams,
RELATIVE_POSITION_DESC,
urlSortParams,
} from '~/issues_list/constants';
import eventHub from '~/issues_list/eventhub';
import { getSortOptions } from '~/issues_list/utils';
......@@ -146,7 +147,7 @@ describe('IssuesListApp component', () => {
describe('csv import/export component', () => {
describe('when user is signed in', () => {
it('renders', async () => {
const search = '?page=1&search=refactor&state=opened&order_by=created_at&sort=desc';
const search = '?page=1&search=refactor&state=opened&sort=created_date';
global.jsdom.reconfigure({ url: `${TEST_HOST}${search}` });
......@@ -239,14 +240,14 @@ describe('IssuesListApp component', () => {
});
describe('sort', () => {
it.each(Object.keys(sortParams))('is set as %s from the url params', (sortKey) => {
global.jsdom.reconfigure({ url: setUrlParams(sortParams[sortKey], TEST_HOST) });
it.each(Object.keys(urlSortParams))('is set as %s from the url params', (sortKey) => {
global.jsdom.reconfigure({ url: setUrlParams(urlSortParams[sortKey], TEST_HOST) });
wrapper = mountComponent();
expect(findIssuableList().props()).toMatchObject({
initialSortBy: sortKey,
urlParams: sortParams[sortKey],
urlParams: urlSortParams[sortKey],
});
});
});
......@@ -528,7 +529,7 @@ describe('IssuesListApp component', () => {
});
describe('when "sort" event is emitted by IssuableList', () => {
it.each(Object.keys(sortParams))(
it.each(Object.keys(apiSortParams))(
'fetches issues with correct params with payload `%s`',
async (sortKey) => {
wrapper = mountComponent();
......@@ -539,10 +540,10 @@ describe('IssuesListApp component', () => {
expect(axiosMock.history.get[1].params).toEqual({
page: xPage,
per_page: sortKey === RELATIVE_POSITION_ASC ? PAGE_SIZE_MANUAL : PAGE_SIZE,
per_page: sortKey === RELATIVE_POSITION_DESC ? PAGE_SIZE_MANUAL : PAGE_SIZE,
state,
with_labels_details: true,
...sortParams[sortKey],
...apiSortParams[sortKey],
});
},
);
......
spec/frontend/issues_list/utils_spec.js
View file @ 55105bff
......@@ -8,7 +8,7 @@ import {
urlParams,
urlParamsWithSpecialValues,
} from 'jest/issues_list/mock_data';
import { sortParams } from '~/issues_list/constants';
import { urlSortParams } from '~/issues_list/constants';
import {
convertToApiParams,
convertToSearchQuery,
......@@ -19,9 +19,9 @@ import {
} from '~/issues_list/utils';
describe('getSortKey', () => {
it.each(Object.keys(sortParams))('returns %s given the correct inputs', (sortKey) => {
const { order_by, sort } = sortParams[sortKey];
expect(getSortKey(order_by, sort)).toBe(sortKey);
it.each(Object.keys(urlSortParams))('returns %s given the correct inputs', (sortKey) => {
const { sort } = urlSortParams[sortKey];
expect(getSortKey(sort)).toBe(sortKey);
});
});
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7