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

modifyIndex-based pagination #20350

Merged

Conversation

philrenaud
Copy link
Contributor

@philrenaud philrenaud commented Apr 10, 2024

  • Adds support for the new modifyIndex-based pagination provided by the /statuses endpoint with a custom First/Prev/Next/Last pagination template on the jobs index
  • Adds a new withPagination hook for Mirage endpoints (that builds on top of withBlockingSupport) that takes a customizable property to paginate upon and provide as x-nomad-nexttoken

@philrenaud philrenaud added the hcc/bff Internal: server-side stuff in a client-side world label Apr 10, 2024
@philrenaud philrenaud self-assigned this Apr 10, 2024
Copy link

github-actions bot commented Apr 10, 2024

Ember Test Audit comparison

bff-job-summary-fe-redux-untangle 0160786 change
passes 1522 1529 +7
failures 0 0 0
flaky 0 0 0
duration 10m 42s 992ms 10m 53s 227ms +10s 235ms

Copy link
Member

@gulducat gulducat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a happy thought and one possible-duplicate call. bring on the tests!

next_token: this.cursorAt,
per_page: this.pageSize + 1,
next_token,
per_page: this.pageSize,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

being able to do arithmetic on nextToken/cursorAt instead of getting an extra buffer job feels really nice 👍

ui/mirage/config.js Outdated Show resolved Hide resolved
Copy link
Member

@gulducat gulducat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aside from a couple misleading messages, LGTM!

ui/tests/acceptance/jobs-list-test.js Outdated Show resolved Hide resolved
assert.deepEqual(
modifyIndexes,
[12, 11, 10, 9, 8, 7, 6, 4, 3, 2],
'Jobs are sorted by modify index, on-page job remains in-place, and off-page pending'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woop, same misleading message here too. I think the actual logic is right for the live-updates-enabled case we're in here, just not the comment and this assertion message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the catch, and yeah validated this is the behaviour from the actual endpoint too.

That said: I have eyes on doing some in-browser post-fetch magic at some point. Remembering the order the jobs were requested, and keeping them in that order regardless of the subsequent return. That way the user wouldn't see things dancing up to the top of the page if they had jostling disabled.

But that's for later if ever!

@philrenaud philrenaud changed the base branch from bff-job-summary-fe-redux to bff-job-summary-fe-redux-untangle April 18, 2024 18:16
@philrenaud philrenaud force-pushed the 19806-ui-handle-jobs-pagination branch from 2ebc575 to 0160786 Compare April 18, 2024 18:18
@philrenaud philrenaud merged commit 16b30df into bff-job-summary-fe-redux-untangle Apr 18, 2024
16 checks passed
@philrenaud philrenaud deleted the 19806-ui-handle-jobs-pagination branch April 18, 2024 19:07
philrenaud added a commit that referenced this pull request Apr 30, 2024
* 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
philrenaud added a commit that referenced this pull request May 2, 2024
* 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
gulducat pushed a commit that referenced this pull request May 3, 2024
* 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
philrenaud added a commit that referenced this pull request May 6, 2024
* 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
philrenaud added a commit that referenced this pull request May 6, 2024
* Hook and latch on the initial index

* Serialization and restart of controller and table

* de-log

* allocBlocks reimplemented at job model level

* totalAllocs doesnt mean on jobmodel what it did in steady.js

* Hamburgers to sausages

* Hacky way to bring new jobs back around and parent job handling in list view

* Getting closer to hook/latch

* Latch from update on hook from initialize, but fickle

* Note on multiple-watch problem

* Sensible monday morning comment removal

* use of abortController to handle transition and reset events

* Next token will now update when there's an on-page shift

* Very rough anti-jostle technique

* Demoable, now to move things out of route and into controller

* Into the controller, generally

* Smarter cancellations

* Reset abortController on index models run, and system/sysbatch jobs now have an improved groupCountSum computed property

* Prev Page reverse querying

* n+1th jobs existing will trigger nextToken/pagination display

* Start of a GET/POST statuses return

* Namespace fix

* Unblock tests

* Realizing to my small horror that this skipURLModification flag may be too heavy handed

* Lintfix

* Default liveupdates localStorage setting to true

* Pagination and index rethink

* Big uncoupling of watchable and url-append stuff

* Testfixes for region, search, and keyboard

* Job row class for test purposes

* Allocations in test now contain events

* Starting on the jobs list tests in earnest

* Forbidden state de-bubbling cleanup

* Job list page size fixes

* Facet/Search/Filter jobs list tests skipped

* Maybe it's the automatic mirage logging

* Unbreak task unit test

* Pre-sort sort

* styling for jobs list pagination and general PR cleanup

* moving from Job.ActiveDeploymentID to Job.LatestDeployment.ID

* modifyIndex-based pagination (#20350)

* 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

* Bugfix: resizing your browser on the new jobs index page would make the viz grow forever (#20458)

* [ui] Searching and filtering options (#20459)

* Beginnings of a search box for filter expressions

* jobSearchBox integration test

* jobs list updateFilter initial test

* Basic jobs list filtering tests

* First attempt at side-by-side facets and search with a computed filter

* Weirdly close to an iterative approach but checked isnt tracked properly

* Big rework to make filter composition and decomposition work nicely with the url

* Namespace facet dropdown added

* NodePool facet dropdown added

* hdsFacet for future testing and basic namespace filtering test

* Namespace filter existence test

* Status filtering

* Node pool/dynamic facet test

* Test patchups

* Attempt at optimize test fix

* Allocation re-load on optimize page explainer

* The Big Un-Skip

* Post-PR-review cleanup

* todo-squashing

* [ui] Handle parent/child jobs with the paginated Jobs Index route (#20493)

* First pass at a non-watchQuery version

* Parameterized jobs get child fetching and jobs index status style for parent jobs

* Completed allocs vs Running allocs in a child-job context, and fix an issue where moving from parent to parent would not reset index

* Testfix and better handling empty-child-statuses-list

* Parent/child test case

* Dont show empty allocation-status bars for parent jobs with no children

* Splits Settings into 2 sections, sign-in/profile and user settings (#20535)

* Changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hcc/bff Internal: server-side stuff in a client-side world
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants