From badb6c42d01028455e58cf4743a01a9976101021 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 30 Oct 2018 10:50:24 -0700 Subject: [PATCH 1/4] Add onChange hook to the search-box component --- ui/app/components/search-box.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ui/app/components/search-box.js b/ui/app/components/search-box.js index 8066f3e0b..4f58b38d8 100644 --- a/ui/app/components/search-box.js +++ b/ui/app/components/search-box.js @@ -12,6 +12,9 @@ export default Component.extend({ // Used to throttle sets to searchTerm debounce: 150, + // A hook that's called when the search value changes + onChange() {}, + classNames: ['search-box', 'field', 'has-addons'], actions: { @@ -23,5 +26,7 @@ export default Component.extend({ }); function updateSearch() { - this.set('searchTerm', this.get('_searchTerm')); + const newTerm = this.get('_searchTerm'); + this.onChange(newTerm); + this.set('searchTerm', newTerm); } From f7ad2ce8bbb0684515591edf2dc44071c656bbf4 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 30 Oct 2018 10:50:50 -0700 Subject: [PATCH 2/4] Add a resetPagination method to the searchable mixin Searchable can be used without pagination, but reseting pagination is more a function of search than pagination insofar as if you add search to a page, you are also going to want automatic pagination resetting. --- ui/app/mixins/searchable.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ui/app/mixins/searchable.js b/ui/app/mixins/searchable.js index 180e4e1b3..0dfa9ba3e 100644 --- a/ui/app/mixins/searchable.js +++ b/ui/app/mixins/searchable.js @@ -36,6 +36,16 @@ export default Mixin.create({ fuzzySearchEnabled: false, regexEnabled: true, + // Search should reset pagination. Not every instance of + // search will be paired with pagination, but it's still + // preferable to generalize this rather than risking it being + // forgotten on a single page. + resetPagination() { + if (this.get('currentPage') != null) { + this.set('currentPage', 1); + } + }, + fuse: computed('listToSearch.[]', 'fuzzySearchProps.[]', function() { return new Fuse(this.get('listToSearch'), { shouldSort: true, From 44e33d01f0f9694566b3ef552a969f50a640e9db Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 30 Oct 2018 10:52:54 -0700 Subject: [PATCH 3/4] Add the resetPagination hook to search boxes across the app --- ui/app/templates/clients/client.hbs | 1 + ui/app/templates/clients/index.hbs | 7 ++++++- ui/app/templates/jobs/index.hbs | 6 +++++- ui/app/templates/jobs/job/allocations.hbs | 1 + ui/app/templates/jobs/job/task-group.hbs | 1 + 5 files changed, 14 insertions(+), 2 deletions(-) diff --git a/ui/app/templates/clients/client.hbs b/ui/app/templates/clients/client.hbs index 2c286e4ba..cb63f84cc 100644 --- a/ui/app/templates/clients/client.hbs +++ b/ui/app/templates/clients/client.hbs @@ -98,6 +98,7 @@
Allocations {{model.allocations.length}}
{{search-box searchTerm=(mut searchTerm) + onChange=(action resetPagination) placeholder="Search allocations..." class="is-inline pull-right" inputClass="is-compact"}} diff --git a/ui/app/templates/clients/index.hbs b/ui/app/templates/clients/index.hbs index 528dc6e91..4ed9e7712 100644 --- a/ui/app/templates/clients/index.hbs +++ b/ui/app/templates/clients/index.hbs @@ -4,7 +4,12 @@ {{else}} {{#if nodes.length}}
-
{{search-box searchTerm=(mut searchTerm) placeholder="Search clients..."}}
+
+ {{search-box + searchTerm=(mut searchTerm) + onChange=(action resetPagination) + placeholder="Search clients..."}} +
{{/if}} {{#list-pagination diff --git a/ui/app/templates/jobs/index.hbs b/ui/app/templates/jobs/index.hbs index 10a04f422..97e5d1a76 100644 --- a/ui/app/templates/jobs/index.hbs +++ b/ui/app/templates/jobs/index.hbs @@ -5,7 +5,11 @@
{{#if filteredJobs.length}}
- {{search-box data-test-jobs-search searchTerm=(mut searchTerm) placeholder="Search jobs..."}} + {{search-box + data-test-jobs-search + searchTerm=(mut searchTerm) + onChange=(action resetPagination) + placeholder="Search jobs..."}}
{{/if}}
diff --git a/ui/app/templates/jobs/job/allocations.hbs b/ui/app/templates/jobs/job/allocations.hbs index e8f4c07d3..acd57bc9a 100644 --- a/ui/app/templates/jobs/job/allocations.hbs +++ b/ui/app/templates/jobs/job/allocations.hbs @@ -6,6 +6,7 @@ {{search-box data-test-allocations-search searchTerm=(mut searchTerm) + onChange=(action resetPagination) placeholder="Search allocations..."}}
diff --git a/ui/app/templates/jobs/job/task-group.hbs b/ui/app/templates/jobs/job/task-group.hbs index b9ead72d8..1b511adb8 100644 --- a/ui/app/templates/jobs/job/task-group.hbs +++ b/ui/app/templates/jobs/job/task-group.hbs @@ -46,6 +46,7 @@ {{search-box searchTerm=(mut searchTerm) placeholder="Search allocations..." + onChange=(action resetPagination) class="is-inline pull-right" inputClass="is-compact"}} From b00916ac177a11c4f2fbd36ea374f06750839cbc Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 30 Oct 2018 14:17:23 -0700 Subject: [PATCH 4/4] Test coverage for resetPagination --- .../templates/components/list-pagination.hbs | 8 ++--- .../components/list-pagination/list-pager.hbs | 2 +- ui/tests/acceptance/jobs-list-test.js | 18 ++++++++++++ ui/tests/pages/jobs/list.js | 3 ++ ui/tests/unit/mixins/searchable-test.js | 29 +++++++++++++++++++ 5 files changed, 55 insertions(+), 5 deletions(-) diff --git a/ui/app/templates/components/list-pagination.hbs b/ui/app/templates/components/list-pagination.hbs index bb86e606a..e42c7fc62 100644 --- a/ui/app/templates/components/list-pagination.hbs +++ b/ui/app/templates/components/list-pagination.hbs @@ -1,9 +1,9 @@ {{#if source.length}} {{yield (hash - first=(component "list-pagination/list-pager" page=1 visible=(not (eq page 1))) - prev=(component "list-pagination/list-pager" page=(dec page) visible=(not (eq page 1))) - next=(component "list-pagination/list-pager" page=(inc page) visible=(not (eq page lastPage))) - last=(component "list-pagination/list-pager" page=lastPage visible=(not (eq page lastPage))) + first=(component "list-pagination/list-pager" test="first" page=1 visible=(not (eq page 1))) + prev=(component "list-pagination/list-pager" test="prev" page=(dec page) visible=(not (eq page 1))) + next=(component "list-pagination/list-pager" test="next" page=(inc page) visible=(not (eq page lastPage))) + last=(component "list-pagination/list-pager" test="last" page=lastPage visible=(not (eq page lastPage))) pageLinks=pageLinks currentPage=page totalPages=lastPage diff --git a/ui/app/templates/components/list-pagination/list-pager.hbs b/ui/app/templates/components/list-pagination/list-pager.hbs index cc3437927..4cf46f528 100644 --- a/ui/app/templates/components/list-pagination/list-pager.hbs +++ b/ui/app/templates/components/list-pagination/list-pager.hbs @@ -1,5 +1,5 @@ {{#if visible}} - {{#link-to (query-params currentPage=page) class=attrs.class}} + {{#link-to (query-params currentPage=page) class=attrs.class data-test-pager=test}} {{yield}} {{/link-to}} {{/if}} diff --git a/ui/tests/acceptance/jobs-list-test.js b/ui/tests/acceptance/jobs-list-test.js index 69c9eed71..f8b8bc92a 100644 --- a/ui/tests/acceptance/jobs-list-test.js +++ b/ui/tests/acceptance/jobs-list-test.js @@ -106,6 +106,24 @@ test('when there are jobs, but no matches for a search result, there is an empty }); }); +test('searching resets the current page', function(assert) { + server.createList('job', JobsList.pageSize + 1, { createAllocations: false }); + JobsList.visit(); + + andThen(() => { + JobsList.nextPage(); + }); + + andThen(() => { + assert.equal(currentURL(), '/jobs?page=2', 'Page query param captures page=2'); + JobsList.search('foobar'); + }); + + andThen(() => { + assert.equal(currentURL(), '/jobs?search=foobar', 'No page query param'); + }); +}); + test('when the namespace query param is set, only matching jobs are shown and the namespace value is forwarded to app state', function(assert) { server.createList('namespace', 2); const job1 = server.create('job', { namespaceId: server.db.namespaces[0].id }); diff --git a/ui/tests/pages/jobs/list.js b/ui/tests/pages/jobs/list.js index 252b92efc..37ebd6746 100644 --- a/ui/tests/pages/jobs/list.js +++ b/ui/tests/pages/jobs/list.js @@ -31,6 +31,9 @@ export default create({ clickName: clickable('[data-test-job-name] a'), }), + nextPage: clickable('[data-test-pager="next"]'), + prevPage: clickable('[data-test-pager="prev"]'), + isEmpty: isPresent('[data-test-empty-jobs-list]'), emptyState: { headline: text('[data-test-empty-jobs-list-headline]'), diff --git a/ui/tests/unit/mixins/searchable-test.js b/ui/tests/unit/mixins/searchable-test.js index 10396e749..00b749d94 100644 --- a/ui/tests/unit/mixins/searchable-test.js +++ b/ui/tests/unit/mixins/searchable-test.js @@ -176,3 +176,32 @@ test('each search mode has independent search props', function(assert) { 'Canada is not matched by the regex because only id is looked at for regex search' ); }); + +test('the resetPagination method is a no-op', function(assert) { + const subject = this.subject(); + assert.strictEqual(subject.get('currentPage'), undefined, 'No currentPage value set'); + subject.resetPagination(); + assert.strictEqual(subject.get('currentPage'), undefined, 'Still no currentPage value set'); +}); + +moduleFor('mixin:searchable', 'Unit | Mixin | Searchable (with pagination)', { + subject() { + const SearchablePaginatedObject = EmberObject.extend(Searchable, { + source: null, + searchProps: computed(() => ['id', 'name']), + listToSearch: alias('source'), + currentPage: 1, + }); + + this.register('test-container:searchable-paginated-object', SearchablePaginatedObject); + return getOwner(this).lookup('test-container:searchable-paginated-object'); + }, +}); + +test('the resetPagination method sets the currentPage to 1', function(assert) { + const subject = this.subject(); + subject.set('currentPage', 5); + assert.equal(subject.get('currentPage'), 5, 'Current page is something other than 1'); + subject.resetPagination(); + assert.equal(subject.get('currentPage'), 1, 'Current page gets reset to 1'); +});