Skip to content

Commit

Permalink
modifyIndex-based pagination (#20350)
Browse files Browse the repository at this point in the history
* modifyIndex-based pagination

* modifyIndex gets its own column and pagination compacted with icons

* A generic withPagination handler for mirage

* Some live-PR changes

* Pagination and button disabled tests

* Job update handling tests for jobs index

* assertion timeout in case of long setTimeouts

* assert.timeouts down to 500ms

* de-to-do

* Clarifying comment and test descriptions
  • Loading branch information
philrenaud committed May 6, 2024
1 parent 6462ed1 commit ee1a19c
Show file tree
Hide file tree
Showing 7 changed files with 623 additions and 61 deletions.
6 changes: 0 additions & 6 deletions ui/app/adapters/job.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,6 @@ export default class JobAdapter extends WatchableNamespaceIDs {

const signal = get(options, 'adapterOptions.abortController.signal');

// when GETting our jobs list, we want to sort in reverse order, because
// the sort property is ModifyIndex and we want the most recent jobs first.
if (method === 'GET') {
query.reverse = true;
}

return this.ajax(url, method, {
signal,
data: query,
Expand Down
40 changes: 26 additions & 14 deletions ui/app/controllers/jobs/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export default class JobsIndexController extends Controller {
super(...arguments);
this.pageSize = this.userSettings.pageSize;
}
reverse = false;

queryParams = [
'cursorAt',
Expand Down Expand Up @@ -146,21 +145,28 @@ export default class JobsIndexController extends Controller {
// overwrite this controller's jobIDs, leverage its index, and
// restart a blocking watchJobIDs here.
let prevPageToken = await this.loadPreviousPageToken();
if (prevPageToken.length > 1) {
// if there's only one result, it'd be the job you passed into it as your nextToken (and the first shown on your current page)
const [id, namespace] = JSON.parse(prevPageToken.lastObject.id);
// If there's no nextToken, we're at the "start" of our list and can drop the cursorAt
if (!prevPageToken.meta.nextToken) {
this.cursorAt = null;
} else {
this.cursorAt = `${namespace}.${id}`;
}
// If there's no nextToken, we're at the "start" of our list and can drop the cursorAt
if (!prevPageToken.meta.nextToken) {
this.cursorAt = undefined;
} else {
// cursorAt should be the highest modifyIndex from the previous query.
// This will immediately fire the route model hook with the new cursorAt
this.cursorAt = prevPageToken
.sortBy('modifyIndex')
.get('lastObject').modifyIndex;
}
} else if (page === 'next') {
if (!this.nextToken) {
return;
}
this.cursorAt = this.nextToken;
} else if (page === 'first') {
this.cursorAt = undefined;
} else if (page === 'last') {
let prevPageToken = await this.loadPreviousPageToken({ last: true });
this.cursorAt = prevPageToken
.sortBy('modifyIndex')
.get('lastObject').modifyIndex;
}
}

Expand Down Expand Up @@ -235,13 +241,19 @@ export default class JobsIndexController extends Controller {
});
}

async loadPreviousPageToken() {
// Ask for the previous #page_size jobs, starting at the first job that's currently shown
// on our page, and the last one in our list should be the one we use for our
// subsequent nextToken.
async loadPreviousPageToken({ last = false } = {}) {
let next_token = +this.cursorAt + 1;
if (last) {
next_token = undefined;
}
let prevPageToken = await this.store.query(
'job',
{
prev_page_query: true, // TODO: debugging only!
next_token: this.cursorAt,
per_page: this.pageSize + 1,
next_token,
per_page: this.pageSize,
reverse: true,
},
{
Expand Down
58 changes: 45 additions & 13 deletions ui/app/templates/jobs/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@
@color="primary"
@icon="sync"
{{on "click" (perform this.updateJobList)}}
data-test-updates-pending-button
/>
{{/if}}
</Hds::ButtonSet>
Expand All @@ -288,6 +289,7 @@
{{on "click" (action this.gotoJob B.data)}}
class="job-row is-interactive {{if B.data.assumeGC "assume-gc"}}"
data-test-job-row={{B.data.plainId}}
data-test-modify-index={{B.data.modifyIndex}}
>
{{!-- {{#each this.tableColumns as |column|}}
<B.Td>{{get B.data (lowercase column.label)}}</B.Td>
Expand Down Expand Up @@ -360,15 +362,35 @@

<section id="jobs-list-pagination">
<div class="nav-buttons">
<span {{keyboard-shortcut
label="First Page"
pattern=(array "{" "{")
action=(action this.handlePageChange "first")}}
>
<Hds::Button
@text="First"
@color="tertiary"
@size="small"
@icon="chevrons-left"
@iconPosition="leading"
disabled={{not this.cursorAt}}
data-test-pager="first"
{{on "click" (action this.handlePageChange "first")}}
/>
</span>
<span {{keyboard-shortcut
label="Previous Page"
pattern=(array "[" "[")
action=(action this.handlePageChange "prev")}}
>
<Hds::Button
@text="Prev"
@color="secondary"
@text="Previous"
@color="tertiary"
@size="small"
@icon="chevron-left"
@iconPosition="leading"
disabled={{not this.cursorAt}}
data-test-pager="previous"
{{on "click" (action this.handlePageChange "prev")}}
/>
</span>
Expand All @@ -379,26 +401,36 @@
>
<Hds::Button
@text="Next"
@color="secondary"
@color="tertiary"
@size="small"
@icon="chevron-right"
@iconPosition="trailing"
disabled={{not this.nextToken}}
data-test-pager="next"
{{on "click" (action this.handlePageChange "next")}}
/>
</span>
<span {{keyboard-shortcut
label="Last Page"
pattern=(array "}" "}")
action=(action this.handlePageChange "last")}}
>
<Hds::Button
@text="Last"
@color="tertiary"
@icon="chevrons-right"
@iconPosition="trailing"
@size="small"
disabled={{not this.nextToken}}
data-test-pager="last"
{{on "click" (action this.handlePageChange "last")}}
/>
</span>
</div>
<div class="page-size">
<PageSizeSelect @onChange={{this.handlePageSizeChange}} />
</div>
</section>
{{!-- <Hds::Pagination::Compact
@onPageChange={{action this.handlePageChange}}
@isDisabledPrev={{not this.cursorAt}}
@isDisabledNext={{not this.nextToken}}
@sizeSelectorLabel="Per page"
@showSizeSelector={{true}}
@onPageSizeChange={{this.handlePageSizeChange}}
@pageSizes={{array 10 25 50}}
@currentPageSize={{this.pageSize}}
/> --}}
{{else}}
<Hds::ApplicationState data-test-empty-jobs-list as |A|>
{{!-- TODO: differentiate between "empty because there's nothing" and "empty because you have filtered/searched" --}}
Expand Down
87 changes: 68 additions & 19 deletions ui/mirage/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,20 @@ export default function () {

const nomadIndices = {}; // used for tracking blocking queries
const server = this;
const withBlockingSupport = function (fn) {
const withBlockingSupport = function (
fn,
{ pagination = false, tokenProperty = 'ModifyIndex' } = {}
) {
return function (schema, request) {
let handler = fn;
if (pagination) {
handler = withPagination(handler, tokenProperty);
}

// Get the original response
let { url } = request;
url = url.replace(/index=\d+[&;]?/, '');
const response = fn.apply(this, arguments);
let response = handler.apply(this, arguments);

// Get and increment the appropriate index
nomadIndices[url] || (nomadIndices[url] = 2);
Expand All @@ -58,6 +66,34 @@ export default function () {
};
};

const withPagination = function (fn, tokenProperty = 'ModifyIndex') {
return function (schema, request) {
let response = fn.apply(this, arguments);
let perPage = parseInt(request.queryParams.per_page || 25);
let page = parseInt(request.queryParams.page || 1);
let totalItems = response.length;
let totalPages = Math.ceil(totalItems / perPage);
let hasMore = page < totalPages;

let paginatedItems = response.slice((page - 1) * perPage, page * perPage);

let nextToken = null;
if (hasMore) {
nextToken = response[page * perPage][tokenProperty];
}

if (nextToken) {
return new Response(
200,
{ 'x-nomad-nexttoken': nextToken },
paginatedItems
);
} else {
return new Response(200, {}, paginatedItems);
}
};
};

this.get(
'/jobs',
withBlockingSupport(function ({ jobs }, { queryParams }) {
Expand All @@ -76,23 +112,36 @@ export default function () {

this.get(
'/jobs/statuses',
withBlockingSupport(function ({ jobs }, req) {
let per_page = req.queryParams.per_page || 20;
const namespace = req.queryParams.namespace || 'default';

const json = this.serialize(jobs.all());
return json
.sort((a, b) => b.ID.localeCompare(a.ID))
.sort((a, b) => b.ModifyIndex - a.ModifyIndex)
.filter((job) => {
if (namespace === '*') return true;
return namespace === 'default'
? !job.NamespaceID || job.NamespaceID === 'default'
: job.NamespaceID === namespace;
})
.map((job) => filterKeys(job, 'TaskGroups', 'NamespaceID'))
.slice(0, per_page);
})
withBlockingSupport(
function ({ jobs }, req) {
const namespace = req.queryParams.namespace || 'default';
let nextToken = req.queryParams.next_token || 0;
let reverse = req.queryParams.reverse === 'true';
const json = this.serialize(jobs.all());
let sortedJson = json
.sort((a, b) =>
reverse
? a.ModifyIndex - b.ModifyIndex
: b.ModifyIndex - a.ModifyIndex
)
.filter((job) => {
if (namespace === '*') return true;
return namespace === 'default'
? !job.NamespaceID || job.NamespaceID === 'default'
: job.NamespaceID === namespace;
})
.map((job) => filterKeys(job, 'TaskGroups', 'NamespaceID'));
if (nextToken) {
sortedJson = sortedJson.filter((job) =>
reverse
? job.ModifyIndex >= nextToken
: job.ModifyIndex <= nextToken
);
}
return sortedJson;
},
{ pagination: true, tokenProperty: 'ModifyIndex' }
)
);

this.post(
Expand Down
15 changes: 10 additions & 5 deletions ui/mirage/scenarios/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,16 @@ function jobsIndexTestCluster(server) {
faker.seed(1);
server.createList('agent', 1, 'withConsulLink', 'withVaultLink');
server.createList('node', 1);
server.createList('job', 1, {
namespaceId: 'default',
resourceSpec: Array(1).fill('M: 256, C: 500'),
groupAllocCount: 1,
});

const jobsToCreate = 55;
for (let i = 0; i < jobsToCreate; i++) {
server.create('job', {
namespaceId: 'default',
resourceSpec: Array(1).fill('M: 256, C: 500'),
groupAllocCount: 1,
modifyIndex: i + 1,
});
}
}

function smallCluster(server) {
Expand Down
Loading

0 comments on commit ee1a19c

Please sign in to comment.