Commit 3f22e424 authored by Tom Quirk's avatar Tom Quirk Committed by Brandon Labuschagne

Only fetch namespaces if search value changes

Addressing reviewer feedback in
the Jira Connect app search functionality.

Changelog: changed
parent a35c1e1e
...@@ -30,7 +30,8 @@ export default { ...@@ -30,7 +30,8 @@ export default {
page: 1, page: 1,
totalItems: 0, totalItems: 0,
errorMessage: null, errorMessage: null,
searchTerm: '', userSearchTerm: '',
searchValue: '',
}; };
}, },
computed: { computed: {
...@@ -45,16 +46,11 @@ export default { ...@@ -45,16 +46,11 @@ export default {
}, },
methods: { methods: {
loadGroups() { loadGroups() {
// fetchGroups returns no results for search terms 0 < {length} < 3.
// The desired UX is to return the unfiltered results for searches {length} < 3.
// Here, we set the search to an empty string if {length} < 3
const search = this.searchTerm?.length < MINIMUM_SEARCH_TERM_LENGTH ? '' : this.searchTerm;
this.isLoadingMore = true; this.isLoadingMore = true;
return fetchGroups(this.groupsPath, { return fetchGroups(this.groupsPath, {
page: this.page, page: this.page,
perPage: this.$options.DEFAULT_GROUPS_PER_PAGE, perPage: this.$options.DEFAULT_GROUPS_PER_PAGE,
search, search: this.searchValue,
}) })
.then((response) => { .then((response) => {
const { page, total } = parseIntPagination(normalizeHeaders(response.headers)); const { page, total } = parseIntPagination(normalizeHeaders(response.headers));
...@@ -69,12 +65,24 @@ export default { ...@@ -69,12 +65,24 @@ export default {
this.isLoadingMore = false; this.isLoadingMore = false;
}); });
}, },
onGroupSearch(searchTerm) { onGroupSearch(userSearchTerm = '') {
// keep a copy of the search term for pagination this.userSearchTerm = userSearchTerm;
this.searchTerm = searchTerm;
// reset the current page // fetchGroups returns no results for search terms 0 < {length} < 3.
// The desired UX is to return the unfiltered results for searches {length} < 3.
// Here, we set the search to an empty string '' if {length} < 3
const newSearchValue =
this.userSearchTerm.length < MINIMUM_SEARCH_TERM_LENGTH ? '' : this.userSearchTerm;
// don't fetch new results if the search value didn't change.
if (newSearchValue === this.searchValue) {
return;
}
// reset the page.
this.page = 1; this.page = 1;
return this.loadGroups(); this.searchValue = newSearchValue;
this.loadGroups();
}, },
}, },
DEFAULT_GROUPS_PER_PAGE, DEFAULT_GROUPS_PER_PAGE,
...@@ -92,7 +100,7 @@ export default { ...@@ -92,7 +100,7 @@ export default {
debounce="500" debounce="500"
:placeholder="__('Search by name')" :placeholder="__('Search by name')"
:is-loading="isLoadingMore" :is-loading="isLoadingMore"
:value="searchTerm" :value="userSearchTerm"
@input="onGroupSearch" @input="onGroupSearch"
/> />
......
...@@ -100,6 +100,8 @@ describe('GroupsList', () => { ...@@ -100,6 +100,8 @@ describe('GroupsList', () => {
}); });
createComponent(); createComponent();
// wait for the initial loadGroups
// to finish.
await waitForPromises(); await waitForPromises();
}); });
...@@ -136,6 +138,8 @@ describe('GroupsList', () => { ...@@ -136,6 +138,8 @@ describe('GroupsList', () => {
describe('while groups are loading', () => { describe('while groups are loading', () => {
beforeEach(async () => { beforeEach(async () => {
fetchGroups.mockClear(); fetchGroups.mockClear();
// return a never-ending promise to make test
// deterministic.
fetchGroups.mockReturnValue(new Promise(() => {})); fetchGroups.mockReturnValue(new Promise(() => {}));
findSearchBox().vm.$emit('input', mockSearchTeam); findSearchBox().vm.$emit('input', mockSearchTeam);
...@@ -172,7 +176,7 @@ describe('GroupsList', () => { ...@@ -172,7 +176,7 @@ describe('GroupsList', () => {
describe('when group search finishes loading', () => { describe('when group search finishes loading', () => {
beforeEach(async () => { beforeEach(async () => {
fetchGroups.mockResolvedValue({ data: [mockGroup1] }); fetchGroups.mockResolvedValue({ data: [mockGroup1] });
findSearchBox().vm.$emit('input'); findSearchBox().vm.$emit('input', mockSearchTeam);
await waitForPromises(); await waitForPromises();
}); });
...@@ -183,32 +187,48 @@ describe('GroupsList', () => { ...@@ -183,32 +187,48 @@ describe('GroupsList', () => {
}); });
}); });
it.each` describe.each`
userSearchTerm | finalSearchTerm previousSearch | newSearch | shouldSearch | expectedSearchValue
${'gitl'} | ${'gitl'} ${''} | ${'git'} | ${true} | ${'git'}
${'git'} | ${'git'} ${'g'} | ${'git'} | ${true} | ${'git'}
${'gi'} | ${''} ${'git'} | ${'gitl'} | ${true} | ${'gitl'}
${'g'} | ${''} ${'git'} | ${'gi'} | ${true} | ${''}
${''} | ${''} ${'gi'} | ${'g'} | ${false} | ${undefined}
${undefined} | ${undefined} ${'g'} | ${''} | ${false} | ${undefined}
${''} | ${'g'} | ${false} | ${undefined}
`( `(
'searches for "$finalSearchTerm" when user enters "$userSearchTerm"', 'when previous search was "$previousSearch" and user enters "$newSearch"',
async ({ userSearchTerm, finalSearchTerm }) => { ({ previousSearch, newSearch, shouldSearch, expectedSearchValue }) => {
beforeEach(async () => {
fetchGroups.mockResolvedValue({ fetchGroups.mockResolvedValue({
data: [mockGroup1], data: [mockGroup1],
headers: { 'X-PAGE': 1, 'X-TOTAL': 1 }, headers: { 'X-PAGE': 1, 'X-TOTAL': 1 },
}); });
// wait for initial load
createComponent(); createComponent();
await waitForPromises(); await waitForPromises();
const searchBox = findSearchBox(); // set up the "previous search"
searchBox.vm.$emit('input', userSearchTerm); findSearchBox().vm.$emit('input', previousSearch);
await waitForPromises();
expect(fetchGroups).toHaveBeenLastCalledWith(mockGroupsPath, { fetchGroups.mockClear();
});
it(`${shouldSearch ? 'should' : 'should not'} execute fetch new results`, () => {
// enter the new search
findSearchBox().vm.$emit('input', newSearch);
if (shouldSearch) {
expect(fetchGroups).toHaveBeenCalledWith(mockGroupsPath, {
page: 1, page: 1,
perPage: DEFAULT_GROUPS_PER_PAGE, perPage: DEFAULT_GROUPS_PER_PAGE,
search: finalSearchTerm, search: expectedSearchValue,
});
} else {
expect(fetchGroups).not.toHaveBeenCalled();
}
}); });
}, },
); );
...@@ -226,7 +246,13 @@ describe('GroupsList', () => { ...@@ -226,7 +246,13 @@ describe('GroupsList', () => {
await waitForPromises(); await waitForPromises();
const paginationEl = findPagination(); const paginationEl = findPagination();
paginationEl.vm.$emit('input', 2);
// mock the response from page 2
fetchGroups.mockResolvedValue({
headers: { 'X-TOTAL': totalItems, 'X-PAGE': 2 },
data: mockGroups,
});
await paginationEl.vm.$emit('input', 2);
}); });
it('should load results for page 2', () => { it('should load results for page 2', () => {
...@@ -237,18 +263,23 @@ describe('GroupsList', () => { ...@@ -237,18 +263,23 @@ describe('GroupsList', () => {
}); });
}); });
it('resets page to 1 on search `input` event', () => { it.each`
const mockSearchTerm = 'gitlab'; scenario | searchTerm | expectedPage | expectedSearchTerm
${'preserves current page'} | ${'gi'} | ${2} | ${''}
${'resets page to 1'} | ${'gitlab'} | ${1} | ${'gitlab'}
`(
'$scenario when search term is $searchTerm',
({ searchTerm, expectedPage, expectedSearchTerm }) => {
const searchBox = findSearchBox(); const searchBox = findSearchBox();
searchBox.vm.$emit('input', searchTerm);
searchBox.vm.$emit('input', mockSearchTerm);
expect(fetchGroups).toHaveBeenLastCalledWith(mockGroupsPath, { expect(fetchGroups).toHaveBeenLastCalledWith(mockGroupsPath, {
page: 1, page: expectedPage,
perPage: DEFAULT_GROUPS_PER_PAGE, perPage: DEFAULT_GROUPS_PER_PAGE,
search: mockSearchTerm, search: expectedSearchTerm,
});
}); });
},
);
}); });
}); });
......
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