Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: Reset page query param on search #4822

Merged
merged 4 commits into from
Nov 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion ui/app/components/search-box.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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);
}
10 changes: 10 additions & 0 deletions ui/app/mixins/searchable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions ui/app/templates/clients/client.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
<div>Allocations <span class="badge is-white">{{model.allocations.length}}</span></div>
{{search-box
searchTerm=(mut searchTerm)
onChange=(action resetPagination)
placeholder="Search allocations..."
class="is-inline pull-right"
inputClass="is-compact"}}
Expand Down
7 changes: 6 additions & 1 deletion ui/app/templates/clients/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@
{{else}}
{{#if nodes.length}}
<div class="content">
<div>{{search-box searchTerm=(mut searchTerm) placeholder="Search clients..."}}</div>
<div>
{{search-box
searchTerm=(mut searchTerm)
onChange=(action resetPagination)
placeholder="Search clients..."}}
</div>
</div>
{{/if}}
{{#list-pagination
Expand Down
8 changes: 4 additions & 4 deletions ui/app/templates/components/list-pagination.hbs
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion ui/app/templates/components/list-pagination/list-pager.hbs
Original file line number Diff line number Diff line change
@@ -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}}
6 changes: 5 additions & 1 deletion ui/app/templates/jobs/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
<div class="columns">
{{#if filteredJobs.length}}
<div class="column">
{{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..."}}
</div>
{{/if}}
<div class="column is-centered">
Expand Down
1 change: 1 addition & 0 deletions ui/app/templates/jobs/job/allocations.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
{{search-box
data-test-allocations-search
searchTerm=(mut searchTerm)
onChange=(action resetPagination)
placeholder="Search allocations..."}}
</div>
</div>
Expand Down
1 change: 1 addition & 0 deletions ui/app/templates/jobs/job/task-group.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
{{search-box
searchTerm=(mut searchTerm)
placeholder="Search allocations..."
onChange=(action resetPagination)
class="is-inline pull-right"
inputClass="is-compact"}}
</div>
Expand Down
18 changes: 18 additions & 0 deletions ui/tests/acceptance/jobs-list-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
3 changes: 3 additions & 0 deletions ui/tests/pages/jobs/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]'),
Expand Down
29 changes: 29 additions & 0 deletions ui/tests/unit/mixins/searchable-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});