Migrate versions page to paginated list #10659
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eth3lbert
force-pushed
the
load-more-versions
branch
2 times, most recently
from
f531696 to
d83cd81
Compare
eth3lbert
commented
Feb 22, 2025
| @@ -231,6 +236,7 @@ module('Model | Version', function (hooks) { | |||
| this.db.version.create({ crate, num: '0.4.3', yanked: true }); | |||
| crateRecord = await this.store.findRecord('crate', crate.name, { reload: true }); | |||
| versions = (await crateRecord.loadVersionsTask.perform({ reload: true })).slice(); | |||
| crateRecord.release_tracks = calculateReleaseTracks(versions); | |||
There was a problem hiding this comment.
This change was made to pass the tests, but it doesn't reflect what we do in practice (for versions page). Since the new release wouldn't be loaded without a page reload, as already mentioned in the description, I'm not sure what the best approach is in this case.
Comment on lines
+5
to
+6
| <strong>{{ this.sortedVersions.length }}</strong> of <strong>{{ this.crate.num_versions }}</strong> | ||
| <strong>{{ this.crate.name }}</strong> versions since |
There was a problem hiding this comment.
This currently seems a bit odd, since the bolded number and name are positioned directly beside each other. There might be better wording or display methods.
Turbo87
reviewed
Feb 26, 2025
eth3lbert
force-pushed
the
load-more-versions
branch
from
d83cd81 to
4afc56a
Compare
This decouple `versionIdsByDate`, `versionIdsBySemver` and `versionsObj` getters from `loadVersionsTask` and computes values from all loaded versions.
…ersions` endpoint
eth3lbert
force-pushed
the
load-more-versions
branch
3 times, most recently
from
ba8d502 to
cb627bf
Compare
eth3lbert
commented
Feb 26, 2025
Comment on lines
+134
to
+151
| test('navigating to the versions page', async function (assert) { | ||
| loadFixtures(this.db); | ||
|
|
||
| // default with a page size more than 13 | ||
| await visit('/crates/nanomsg'); | ||
| await click('[data-test-versions-tab] a'); | ||
|
|
||
| assert.dom('[data-test-page-description]').hasText(/All 13\s+versions of nanomsg since\s+December \d+th, 2014/); | ||
| assert.dom('[data-test-page-description]').hasText(/13 of 13\s+nanomsg versions since\s+December \d+th, 2014/); | ||
| }); | ||
|
|
||
| test('navigating to the versions page with custom per_page', async function (assert) { | ||
| loadFixtures(this.db); | ||
|
|
||
| await visit('/crates/nanomsg/versions?per_page=10'); | ||
|
|
||
| assert.dom('[data-test-page-description]').hasText(/10 of 13\s+nanomsg versions since\s+December \d+th, 2014/); | ||
|
|
||
| await click('[data-test-id="load-more"]'); | ||
| assert.dom('[data-test-page-description]').hasText(/13 of 13\s+nanomsg versions since\s+December \d+th, 2014/); |
There was a problem hiding this comment.
Since our per_page (100) is much bigger than the test records (13), I split it into two test cases. This can effectively test if 'load more' works.
This implement a paginated versions page. Instead of loading all versions at once, it loads more with a "Load More" button using a seek-based API.
This eliminates the computation of `release_tracks` in the app and fetches the computed meta from the backend once in the `versions` page.
These sorted version computations are no longer needed, we fetch the sorted data from the backend now.
eth3lbert
force-pushed
the
load-more-versions
branch
from
cb627bf to
e7a1f75
Compare
|
Rebased and feedback addressed! |
Turbo87
approved these changes
Feb 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR migrates the
versionspage to a paginated style, which loads more versions with a "Load More" button. Two sorted lists serve as different contexts, so they can display in different. Most of the computation for versions (sortingandrelease_tracks) on this page will no longer be needed on the app, they all done on the backend.Some major changes are highlighted as follows:
versionspage to a paginated list.releaseTrackSetwithrelease_tracksmeta.Regarding the first version hint, since we've moved to a paginated list and the API doesn't support reverse ordering, determining the first version would likely require fetching all versions. If we choose to support this rather than drop it, it would be nice to store this information in the
default_versionstable during publishing and expose it on the API, similar to #10519.For
release_tracks, since we now only fetch it once if it doesn't already exist, and paginated related data only resets when the controllerscratechanges (or page reload),release_trackscould become outdated if it changes after the initial fetch. Two operations can affect the result ofrelease_tracks:In the yanked scenario, a simple solution would be to refetch
release_tracksupon detection within the app. For new releases, the update currently only occurs upon page reload.Another possible and potentially better solution would be to, again, upsert
release_tracksduring publishing/yanking, and expose it on the API like #10519. This would be costly (offline) during backfill, as it would require calculation from scratch, but should be inexpensive during updates, as it would only need to compare to a single version. While this would complicate the publish endpoint, it would result in calculation only once, rather than request.Related:
release_tracksmeta to pagination versions #9624releaseTrackSetusingrelease_tracksmeta #10248preview