Skip to content

G
gitlab-ce
Commit 268ea6fb authored by Peter Hegman's avatar Peter Hegman Committed by David O'Regan
Browse files
Options

Switch to using GlSort for group members view 

Switch from GlDropdown to GlSort for group members sorting
parent d88b97f7
Hide whitespace changes
Inline Side-by-side
Showing with 167 additions and 165 deletions
app/assets/javascripts/members/components/filter_sort/sort_dropdown.vue
View file @ 268ea6fb
<script>
import { mapState } from 'vuex';
import { GlDropdown, GlDropdownItem, GlFormGroup } from '@gitlab/ui';
import { parseSortParam, buildSortUrl } from '~/members/utils';
import { GlSorting, GlSortingItem } from '@gitlab/ui';
import { visitUrl } from '~/lib/utils/url_utility';
import { parseSortParam, buildSortHref } from '~/members/utils';
import { FIELDS } from '~/members/constants';
export default {
name: 'SortDropdown',
components: { GlDropdown, GlDropdownItem, GlFormGroup },
components: { GlSorting, GlSortingItem },
computed: {
...mapState(['tableSortableFields', 'filteredSearchBar']),
sort() {
return parseSortParam(this.tableSortableFields);
},
activeOption() {
return FIELDS.find(field => field.key === this.sort.sortByKey);
},
activeOptionLabel() {
return this.activeOption?.label;
},
isAscending() {
return !this.sort.sortDesc;
},
filteredOptions() {
const buildOption = (field, sortDesc) => ({
...(sortDesc ? field.sort.desc : field.sort.asc),
key: field.key,
sortDesc,
url: buildSortUrl({
sortBy: field.key,
sortDesc,
filteredSearchBarTokens: this.filteredSearchBar.tokens,
filteredSearchBarSearchParam: this.filteredSearchBar.searchParam,
return FIELDS.filter(field => this.tableSortableFields.includes(field.key) && field.sort).map(
field => ({
key: field.key,
label: field.label,
href: buildSortHref({
sortBy: field.key,
sortDesc: false,
filteredSearchBarTokens: this.filteredSearchBar.tokens,
filteredSearchBarSearchParam: this.filteredSearchBar.searchParam,
}),
}),
});
return FIELDS.filter(
field => this.tableSortableFields.includes(field.key) && field.sort,
).flatMap(field => [buildOption(field, false), buildOption(field, true)]);
);
},
},
methods: {
isChecked(key, sortDesc) {
return this.sort?.sortBy === key && this.sort?.sortDesc === sortDesc;
isActive(key) {
return this.activeOption.key === key;
},
handleSortDirectionChange() {
visitUrl(
buildSortHref({
sortBy: this.activeOption.key,
sortDesc: !this.sort.sortDesc,
filteredSearchBarTokens: this.filteredSearchBar.tokens,
filteredSearchBarSearchParam: this.filteredSearchBar.searchParam,
}),
);
},
},
};
</script>
<template>
<gl-form-group
:label="__('Sort by')"
class="gl-mb-0"
label-cols="auto"
label-class="gl-align-self-center gl-pb-0!"
<gl-sorting
class="gl-display-flex"
dropdown-class="gl-w-full"
data-testid="members-sort-dropdown"
:text="activeOptionLabel"
:is-ascending="isAscending"
:sort-direction-tool-tip="__('Sort direction')"
@sortDirectionChange="handleSortDirectionChange"
>
<gl-dropdown
:text="sort.sortByLabel"
block
toggle-class="gl-mb-0"
data-testid="members-sort-dropdown"
right
<gl-sorting-item
v-for="option in filteredOptions"
:key="option.key"
:href="option.href"
:active="isActive(option.key)"
>
<gl-dropdown-item
v-for="option in filteredOptions"
:key="option.param"
:href="option.url"
is-check-item
:is-checked="isChecked(option.key, option.sortDesc)"
>
{{ option.label }}
</gl-dropdown-item>
</gl-dropdown>
</gl-form-group>
{{ option.label }}
</gl-sorting-item>
</gl-sorting>
</template>
app/assets/javascripts/members/constants.js
View file @ 268ea6fb
import { __, s__ } from '~/locale';
const ACCOUNT_SORT_ASC_LABEL = s__('Members|Account, ascending');
import { __ } from '~/locale';
export const FIELDS = [
{
key: 'account',
label: __('Account'),
sort: {
asc: {
param: 'name_asc',
label: ACCOUNT_SORT_ASC_LABEL,
},
desc: {
param: 'name_desc',
label: s__('Members|Account, descending'),
},
asc: 'name_asc',
desc: 'name_desc',
},
},
{
......@@ -29,14 +21,8 @@ export const FIELDS = [
thClass: 'col-meta',
tdClass: 'col-meta',
sort: {
asc: {
param: 'last_joined',
label: s__('Members|Access granted, ascending'),
},
desc: {
param: 'oldest_joined',
label: s__('Members|Access granted, descending'),
},
asc: 'last_joined',
desc: 'oldest_joined',
},
},
{
......@@ -63,14 +49,8 @@ export const FIELDS = [
thClass: 'col-max-role',
tdClass: 'col-max-role',
sort: {
asc: {
param: 'access_level_asc',
label: s__('Members|Max role, ascending'),
},
desc: {
param: 'access_level_desc',
label: s__('Members|Max role, descending'),
},
asc: 'access_level_asc',
desc: 'access_level_desc',
},
},
{
......@@ -81,15 +61,10 @@ export const FIELDS = [
},
{
key: 'lastSignIn',
label: __('Last sign-in'),
sort: {
asc: {
param: 'recent_sign_in',
label: s__('Members|Last sign-in, ascending'),
},
desc: {
param: 'oldest_sign_in',
label: s__('Members|Last sign-in, descending'),
},
asc: 'recent_sign_in',
desc: 'oldest_sign_in',
},
},
{
......@@ -101,9 +76,8 @@ export const FIELDS = [
];
export const DEFAULT_SORT = {
sortBy: 'account',
sortByKey: 'account',
sortDesc: false,
sortByLabel: ACCOUNT_SORT_ASC_LABEL,
};
export const AVATAR_SIZE = 48;
......
app/assets/javascripts/members/utils.js
View file @ 268ea6fb
......@@ -51,23 +51,20 @@ export const parseSortParam = sortableFields => {
const sortParam = getParameterByName('sort');
const sortedField = FIELDS.filter(field => sortableFields.includes(field.key)).find(
field => field.sort?.asc?.param === sortParam || field.sort?.desc?.param === sortParam,
field => field.sort?.asc === sortParam || field.sort?.desc === sortParam,
);
if (!sortedField) {
return DEFAULT_SORT;
}
const isDesc = sortedField?.sort?.desc?.param === sortParam;
return {
sortBy: sortedField.key,
sortDesc: isDesc,
sortByLabel: isDesc ? sortedField?.sort?.desc?.label : sortedField?.sort?.asc?.label,
sortByKey: sortedField.key,
sortDesc: sortedField?.sort?.desc === sortParam,
};
};
export const buildSortUrl = ({
export const buildSortHref = ({
sortBy,
sortDesc,
filteredSearchBarTokens,
......@@ -79,7 +76,7 @@ export const buildSortUrl = ({
return '';
}
const sortParam = sortDesc ? sortDefinition.desc.param : sortDefinition.asc.param;
const sortParam = sortDesc ? sortDefinition.desc : sortDefinition.asc;
const filterParams =
filteredSearchBarTokens?.reduce((accumulator, token) => {
......
locale/gitlab.pot
View file @ 268ea6fb
......@@ -15995,6 +15995,9 @@ msgstr ""
msgid "Last seen"
msgstr ""
msgid "Last sign-in"
msgstr ""
msgid "Last successful sync"
msgstr ""
......@@ -16969,18 +16972,6 @@ msgstr ""
msgid "Members|2FA"
msgstr ""
msgid "Members|Access granted, ascending"
msgstr ""
msgid "Members|Access granted, descending"
msgstr ""
msgid "Members|Account, ascending"
msgstr ""
msgid "Members|Account, descending"
msgstr ""
msgid "Members|An error occurred while trying to enable LDAP override, please try again."
msgstr ""
......@@ -17044,21 +17035,9 @@ msgstr ""
msgid "Members|LDAP override enabled."
msgstr ""
msgid "Members|Last sign-in, ascending"
msgstr ""
msgid "Members|Last sign-in, descending"
msgstr ""
msgid "Members|Leave \"%{source}\""
msgstr ""
msgid "Members|Max role, ascending"
msgstr ""
msgid "Members|Max role, descending"
msgstr ""
msgid "Members|Membership"
msgstr ""
......
spec/features/groups/members/sort_members_spec.rb
View file @ 268ea6fb
......@@ -17,14 +17,20 @@ RSpec.describe 'Groups > Members > Sort members', :js do
end
context 'when `group_members_filtered_search` feature flag is enabled' do
dropdown_toggle_selector = '[data-testid="members-sort-dropdown"] > button'
def expect_sort_by(text, sort_direction)
within('[data-testid="members-sort-dropdown"]') do
expect(page).to have_css('button[aria-haspopup="true"]', text: text)
expect(page).to have_button("Sorting Direction: #{sort_direction == :asc ? 'Ascending' : 'Descending'}")
end
end
it 'sorts account by default' do
it 'sorts by account by default' do
visit_members_list(sort: nil)
expect(first_row.text).to include(owner.name)
expect(second_row.text).to include(developer.name)
expect(page).to have_css(dropdown_toggle_selector, text: 'Account, ascending')
expect_sort_by('Account', :asc)
end
it 'sorts by max role ascending' do
......@@ -32,7 +38,8 @@ RSpec.describe 'Groups > Members > Sort members', :js do
expect(first_row.text).to include(developer.name)
expect(second_row.text).to include(owner.name)
expect(page).to have_css(dropdown_toggle_selector, text: 'Max role, ascending')
expect_sort_by('Max role', :asc)
end
it 'sorts by max role descending' do
......@@ -40,7 +47,8 @@ RSpec.describe 'Groups > Members > Sort members', :js do
expect(first_row.text).to include(owner.name)
expect(second_row.text).to include(developer.name)
expect(page).to have_css(dropdown_toggle_selector, text: 'Max role, descending')
expect_sort_by('Max role', :desc)
end
it 'sorts by access granted ascending' do
......@@ -48,7 +56,8 @@ RSpec.describe 'Groups > Members > Sort members', :js do
expect(first_row.text).to include(developer.name)
expect(second_row.text).to include(owner.name)
expect(page).to have_css(dropdown_toggle_selector, text: 'Access granted, ascending')
expect_sort_by('Access granted', :asc)
end
it 'sorts by access granted descending' do
......@@ -56,7 +65,8 @@ RSpec.describe 'Groups > Members > Sort members', :js do
expect(first_row.text).to include(owner.name)
expect(second_row.text).to include(developer.name)
expect(page).to have_css(dropdown_toggle_selector, text: 'Access granted, descending')
expect_sort_by('Access granted', :desc)
end
it 'sorts by account ascending' do
......@@ -64,7 +74,8 @@ RSpec.describe 'Groups > Members > Sort members', :js do
expect(first_row.text).to include(owner.name)
expect(second_row.text).to include(developer.name)
expect(page).to have_css(dropdown_toggle_selector, text: 'Account, ascending')
expect_sort_by('Account', :asc)
end
it 'sorts by account descending' do
......@@ -72,7 +83,8 @@ RSpec.describe 'Groups > Members > Sort members', :js do
expect(first_row.text).to include(developer.name)
expect(second_row.text).to include(owner.name)
expect(page).to have_css(dropdown_toggle_selector, text: 'Account, descending')
expect_sort_by('Account', :desc)
end
it 'sorts by last sign-in ascending', :clean_gitlab_redis_shared_state do
......@@ -80,7 +92,8 @@ RSpec.describe 'Groups > Members > Sort members', :js do
expect(first_row.text).to include(owner.name)
expect(second_row.text).to include(developer.name)
expect(page).to have_css(dropdown_toggle_selector, text: 'Last sign-in, ascending')
expect_sort_by('Last sign-in', :asc)
end
it 'sorts by last sign-in descending', :clean_gitlab_redis_shared_state do
......@@ -88,7 +101,8 @@ RSpec.describe 'Groups > Members > Sort members', :js do
expect(first_row.text).to include(developer.name)
expect(second_row.text).to include(owner.name)
expect(page).to have_css(dropdown_toggle_selector, text: 'Last sign-in, descending')
expect_sort_by('Last sign-in', :desc)
end
end
......
spec/frontend/members/components/filter_sort/sort_dropdown_spec.js
View file @ 268ea6fb
import { mount, createLocalVue } from '@vue/test-utils';
import { within } from '@testing-library/dom';
import Vuex from 'vuex';
import { GlDropdownItem } from '@gitlab/ui';
import { GlSorting, GlSortingItem } from '@gitlab/ui';
import SortDropdown from '~/members/components/filter_sort/sort_dropdown.vue';
import * as urlUtilities from '~/lib/utils/url_utility';
const localVue = createLocalVue();
localVue.use(Vuex);
......@@ -34,10 +34,13 @@ describe('SortDropdown', () => {
});
};
const findSortingComponent = () => wrapper.find(GlSorting);
const findSortDirectionToggle = () =>
findSortingComponent().find('button[title="Sort direction"]');
const findDropdownToggle = () => wrapper.find('button[aria-haspopup="true"]');
const findDropdownItemByText = text =>
wrapper
.findAll(GlDropdownItem)
.findAll(GlSortingItem)
.wrappers.find(dropdownItemWrapper => dropdownItemWrapper.text() === text);
describe('dropdown options', () => {
......@@ -54,37 +57,21 @@ describe('SortDropdown', () => {
const expectedDropdownItems = [
{
label: 'Account, ascending',
label: 'Account',
url: `${EXPECTED_BASE_URL}name_asc`,
},
{
label: 'Account, descending',
url: `${EXPECTED_BASE_URL}name_desc`,
},
{
label: 'Access granted, ascending',
label: 'Access granted',
url: `${EXPECTED_BASE_URL}last_joined`,
},
{
label: 'Access granted, descending',
url: `${EXPECTED_BASE_URL}oldest_joined`,
},
{
label: 'Max role, ascending',
label: 'Max role',
url: `${EXPECTED_BASE_URL}access_level_asc`,
},
{
label: 'Max role, descending',
url: `${EXPECTED_BASE_URL}access_level_desc`,
},
{
label: 'Last sign-in, ascending',
label: 'Last sign-in',
url: `${EXPECTED_BASE_URL}recent_sign_in`,
},
{
label: 'Last sign-in, descending',
url: `${EXPECTED_BASE_URL}oldest_sign_in`,
},
];
createComponent();
......@@ -102,7 +89,7 @@ describe('SortDropdown', () => {
createComponent();
expect(findDropdownItemByText('Max role, ascending').props('isChecked')).toBe(true);
expect(findDropdownItemByText('Max role').vm.$attrs.active).toBe(true);
});
});
......@@ -112,10 +99,11 @@ describe('SortDropdown', () => {
window.location = new URL(URL_HOST);
});
it('defaults to sorting by "Account, ascending"', () => {
it('defaults to sorting by "Account" in ascending order', () => {
createComponent();
expect(findDropdownToggle().text()).toBe('Account, ascending');
expect(findSortingComponent().props('isAscending')).toBe(true);
expect(findDropdownToggle().text()).toBe('Account');
});
it('sets text as selected sort option', () => {
......@@ -123,13 +111,52 @@ describe('SortDropdown', () => {
createComponent();
expect(findDropdownToggle().text()).toBe('Max role, ascending');
expect(findDropdownToggle().text()).toBe('Max role');
});
});
it('renders dropdown label', () => {
createComponent();
describe('sort direction toggle', () => {
beforeEach(() => {
delete window.location;
window.location = new URL(URL_HOST);
jest.spyOn(urlUtilities, 'visitUrl');
});
describe('when current sort direction is ascending', () => {
beforeEach(() => {
window.location.search = '?sort=access_level_asc';
createComponent();
});
describe('when sort direction toggle is clicked', () => {
beforeEach(() => {
findSortDirectionToggle().trigger('click');
});
it('sorts in descending order', () => {
expect(urlUtilities.visitUrl).toHaveBeenCalledWith(`${URL_HOST}?sort=access_level_desc`);
});
});
});
expect(within(wrapper.element).queryByText('Sort by')).not.toBe(null);
describe('when current sort direction is descending', () => {
beforeEach(() => {
window.location.search = '?sort=access_level_desc';
createComponent();
});
describe('when sort direction toggle is clicked', () => {
beforeEach(() => {
findSortDirectionToggle().trigger('click');
});
it('sorts in ascending order', () => {
expect(urlUtilities.visitUrl).toHaveBeenCalledWith(`${URL_HOST}?sort=access_level_asc`);
});
});
});
});
});
spec/frontend/members/utils_spec.js
View file @ 268ea6fb
......@@ -8,7 +8,7 @@ import {
canUpdate,
canOverride,
parseSortParam,
buildSortUrl,
buildSortHref,
} from '~/members/utils';
import { DEFAULT_SORT } from '~/members/constants';
import { member as memberMock, group, invite } from './mock_data';
......@@ -148,14 +148,14 @@ describe('Members Utils', () => {
describe.each`
sortParam | expected
${'name_asc'} | ${{ sortBy: 'account', sortDesc: false, sortByLabel: 'Account, ascending' }}
${'name_desc'} | ${{ sortBy: 'account', sortDesc: true, sortByLabel: 'Account, descending' }}
${'last_joined'} | ${{ sortBy: 'granted', sortDesc: false, sortByLabel: 'Access granted, ascending' }}
${'oldest_joined'} | ${{ sortBy: 'granted', sortDesc: true, sortByLabel: 'Access granted, descending' }}
${'access_level_asc'} | ${{ sortBy: 'maxRole', sortDesc: false, sortByLabel: 'Max role, ascending' }}
${'access_level_desc'} | ${{ sortBy: 'maxRole', sortDesc: true, sortByLabel: 'Max role, descending' }}
${'recent_sign_in'} | ${{ sortBy: 'lastSignIn', sortDesc: false, sortByLabel: 'Last sign-in, ascending' }}
${'oldest_sign_in'} | ${{ sortBy: 'lastSignIn', sortDesc: true, sortByLabel: 'Last sign-in, descending' }}
${'name_asc'} | ${{ sortByKey: 'account', sortDesc: false }}
${'name_desc'} | ${{ sortByKey: 'account', sortDesc: true }}
${'last_joined'} | ${{ sortByKey: 'granted', sortDesc: false }}
${'oldest_joined'} | ${{ sortByKey: 'granted', sortDesc: true }}
${'access_level_asc'} | ${{ sortByKey: 'maxRole', sortDesc: false }}
${'access_level_desc'} | ${{ sortByKey: 'maxRole', sortDesc: true }}
${'recent_sign_in'} | ${{ sortByKey: 'lastSignIn', sortDesc: false }}
${'oldest_sign_in'} | ${{ sortByKey: 'lastSignIn', sortDesc: true }}
`('when `sort` query string param is `$sortParam`', ({ sortParam, expected }) => {
it(`returns ${JSON.stringify(expected)}`, async () => {
window.location.search = `?sort=${sortParam}`;
......@@ -167,7 +167,7 @@ describe('Members Utils', () => {
});
});
describe('buildSortUrl', () => {
describe('buildSortHref', () => {
beforeEach(() => {
delete window.location;
window.location = new URL(URL_HOST);
......@@ -176,7 +176,7 @@ describe('Members Utils', () => {
describe('when field passed in `sortBy` argument does not have `sort` key defined', () => {
it('returns an empty string', () => {
expect(
buildSortUrl({
buildSortHref({
sortBy: 'source',
sortDesc: false,
filteredSearchBarTokens: [],
......@@ -189,7 +189,7 @@ describe('Members Utils', () => {
describe('when there are no filter params set', () => {
it('sets `sort` param', () => {
expect(
buildSortUrl({
buildSortHref({
sortBy: 'account',
sortDesc: false,
filteredSearchBarTokens: [],
......@@ -204,7 +204,7 @@ describe('Members Utils', () => {
window.location.search = '?two_factor=enabled&with_inherited_permissions=exclude';
expect(
buildSortUrl({
buildSortHref({
sortBy: 'account',
sortDesc: false,
filteredSearchBarTokens: ['two_factor', 'with_inherited_permissions'],
......@@ -219,7 +219,7 @@ describe('Members Utils', () => {
window.location.search = '?search=foobar';
expect(
buildSortUrl({
buildSortHref({
sortBy: 'account',
sortDesc: false,
filteredSearchBarTokens: ['two_factor', 'with_inherited_permissions'],
......
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