diff --git a/app/adapters/version.js b/app/adapters/version.js index bf2c4739868..8c499bfddbf 100644 --- a/app/adapters/version.js +++ b/app/adapters/version.js @@ -7,6 +7,15 @@ export default class VersionAdapter extends ApplicationAdapter { return `/${this.namespace}/crates/${crateName}/${num}`; } + urlForQuery(query) { + let { name } = query ?? {}; + let baseUrl = this.buildURL('crate', name); + let url = `${baseUrl}/versions`; + // The following used to remove them from URL's query string. + delete query.name; + return url; + } + urlForQueryRecord(query) { let { name, num } = query ?? {}; let baseUrl = this.buildURL('crate', name); diff --git a/app/components/version-list/row.hbs b/app/components/version-list/row.hbs index 945bed13a69..726193181db 100644 --- a/app/components/version-list/row.hbs +++ b/app/components/version-list/row.hbs @@ -14,8 +14,6 @@ {{svg-jar "trash"}} {{else if @version.invalidSemver}} ? - {{else if @version.isFirst}} - {{svg-jar "star"}} {{else}} {{@version.releaseTrack}} {{/if}} diff --git a/app/components/version-list/row.js b/app/components/version-list/row.js index 6eccca9108b..8c7242fdd6c 100644 --- a/app/components/version-list/row.js +++ b/app/components/version-list/row.js @@ -19,9 +19,6 @@ export default class VersionRow extends Component { if (version.invalidSemver) { return `Failed to parse version ${version.num}`; } - if (version.isFirst) { - return 'This is the first version that was released'; - } let { releaseTrack } = version; diff --git a/app/controllers/crate/versions.js b/app/controllers/crate/versions.js index 259fd4aee61..0b88a1bd5f1 100644 --- a/app/controllers/crate/versions.js +++ b/app/controllers/crate/versions.js @@ -1,18 +1,115 @@ import Controller from '@ember/controller'; +import { inject as service } from '@ember/service'; import { tracked } from '@glimmer/tracking'; +import { didCancel, dropTask } from 'ember-concurrency'; +import { alias } from 'macro-decorators'; + +import { AjaxError } from '../../utils/ajax'; + +function defaultVersionsContext() { + return { data: [], next_page: undefined }; +} + export default class SearchController extends Controller { - queryParams = ['sort']; + @service sentry; + @service store; + queryParams = ['per_page', 'sort']; @tracked sort; + @tracked per_page = 100; + + @tracked byDate; + @tracked bySemver; + /** @type {import("../../models/crate").default} */ + @tracked crate; + + @alias('versionsContext.data') data; + @alias('versionsContext.next_page') next_page; + + constructor() { + super(...arguments); + this.reset(); + } get currentSortBy() { return this.sort === 'semver' ? 'SemVer' : 'Date'; } + get versionsContext() { + return this.sort === 'semver' ? this.bySemver : this.byDate; + } + get sortedVersions() { - let { versionIdsBySemver, versionIdsByDate, versionsObj: versions } = this.model; + let { loadedVersionsById: versions } = this.crate; + return this.data.map(id => versions.get(id)); + } + + loadMoreTask = dropTask(async () => { + let { crate, data, next_page, per_page, sort, versionsContext } = this; + let query; + + if (next_page) { + let params = new URLSearchParams(next_page); + params.set('name', crate.name); + params.delete('include'); + query = Object.fromEntries(params.entries()); + } else { + if (sort !== 'semver') { + sort = 'date'; + } + query = { + name: crate.name, + sort, + per_page, + }; + } + if (crate.release_tracks == null) { + query.include = 'release_tracks'; + } + + try { + let versions = await this.store.query('version', query); + let meta = versions.meta; + + let ids = versions.map(it => it.id); + if (sort === 'semver') { + this.bySemver = { + ...versionsContext, + data: data.concat(ids), + next_page: meta.next_page, + }; + } else { + this.byDate = { + ...versionsContext, + data: data.concat(ids), + next_page: meta.next_page, + }; + } + + // set release_tracks to crate + if (meta.release_tracks) { + let payload = { + crate: { + id: crate.id, + release_tracks: meta.release_tracks, + }, + }; + this.store.pushPayload(payload); + } + + return versions; + } catch (error) { + // report unexpected errors to Sentry and ignore `ajax()` errors + if (!didCancel(error) && !(error instanceof AjaxError)) { + this.sentry.captureException(error); + } + } + }); - return (this.sort === 'semver' ? versionIdsBySemver : versionIdsByDate).map(id => versions[id]); + reset() { + this.crate = undefined; + this.byDate = defaultVersionsContext(); + this.bySemver = defaultVersionsContext(); } } diff --git a/app/models/crate.js b/app/models/crate.js index fdac65649d3..f7112d80ea5 100644 --- a/app/models/crate.js +++ b/app/models/crate.js @@ -29,6 +29,14 @@ export default class Crate extends Model { @attr documentation; @attr repository; + /** + * This isn't an attribute in the crate response. + * It's actually the `meta` attribute that belongs to `versions` + * and needs to be assigned to `crate` manually. + * @type {Object.?} + **/ + @attr release_tracks; + @hasMany('version', { async: true, inverse: 'crate' }) versions; @hasMany('team', { async: true, inverse: null }) owner_team; @hasMany('user', { async: true, inverse: null }) owner_user; @@ -37,35 +45,12 @@ export default class Crate extends Model { @hasMany('category', { async: true, inverse: null }) categories; @hasMany('dependency', { async: true, inverse: null }) reverse_dependencies; - @cached get versionIdsBySemver() { - let { last } = this.loadVersionsTask; - assert('`loadVersionsTask.perform()` must be called before calling `versionIdsBySemver`', last != null); - let versions = last?.value ?? []; - return versions - .slice() - .sort(compareVersionBySemver) - .map(v => v.id); - } - - @cached get versionIdsByDate() { - let { last } = this.loadVersionsTask; - assert('`loadVersionsTask.perform()` must be called before calling `versionIdsByDate`', last != null); - let versions = last?.value ?? []; - return versions - .slice() - .sort(compareVersionByDate) - .map(v => v.id); - } - - @cached get firstVersionId() { - return this.versionIdsByDate.at(-1); - } - - @cached get versionsObj() { - let { last } = this.loadVersionsTask; - assert('`loadVersionsTask.perform()` must be called before calling `versionsObj`', last != null); - let versions = last?.value ?? []; - return Object.fromEntries(versions.slice().map(v => [v.id, v])); + /** @return {Map} */ + @cached + get loadedVersionsById() { + let versionsRef = this.hasMany('versions'); + let values = versionsRef.value(); + return new Map(values?.map(ref => [ref.id, ref])); } /** @return {Map} */ @@ -77,15 +62,9 @@ export default class Crate extends Model { } @cached get releaseTrackSet() { - let map = new Map(); - let { versionsObj: versions, versionIdsBySemver } = this; - for (let id of versionIdsBySemver) { - let { releaseTrack, isPrerelease, yanked } = versions[id]; - if (releaseTrack && !isPrerelease && !yanked && !map.has(releaseTrack)) { - map.set(releaseTrack, id); - } - } - return new Set(map.values()); + let { release_tracks } = this; + let nums = release_tracks ? Object.values(this.release_tracks).map(it => it.highest) : []; + return new Set(nums); } hasOwnerUser(userId) { @@ -145,25 +124,3 @@ export default class Crate extends Model { return (await fut) ?? []; }); } - -function compareVersionBySemver(a, b) { - let aSemver = a.semver; - let bSemver = b.semver; - - if (aSemver === bSemver) { - return b.created_at - a.created_at; - } else if (aSemver === null) { - return 1; - } else if (bSemver === null) { - return -1; - } else { - return bSemver.compare(aSemver); - } -} - -function compareVersionByDate(a, b) { - let bDate = b.created_at.getTime(); - let aDate = a.created_at.getTime(); - - return bDate === aDate ? parseInt(b.id) - parseInt(a.id) : bDate - aDate; -} diff --git a/app/models/version.js b/app/models/version.js index 16171ed918f..05385c4cefb 100644 --- a/app/models/version.js +++ b/app/models/version.js @@ -72,10 +72,6 @@ export default class Version extends Model { return Date.now() - this.created_at.getTime() < EIGHT_DAYS; } - @cached get isFirst() { - return this.id === this.crate?.firstVersionId; - } - get semver() { return semverParse(this.num, { loose: true }); } @@ -106,7 +102,7 @@ export default class Version extends Model { return false; } - return this.crate?.releaseTrackSet.has(this.id); + return this.crate?.releaseTrackSet.has(this.num); } get featureList() { diff --git a/app/routes/crate/versions.js b/app/routes/crate/versions.js index 1144952055b..1b5bd05681b 100644 --- a/app/routes/crate/versions.js +++ b/app/routes/crate/versions.js @@ -2,11 +2,27 @@ import Route from '@ember/routing/route'; import { waitForPromise } from '@ember/test-waiters'; export default class VersionsRoute extends Route { + queryParams = { + sort: { refreshModel: true }, + }; + + model(params) { + // we need a model() implementation that changes, otherwise the setupController() hook + // is not called and we won't reload the results if a new query string is used + return params; + } + setupController(controller) { super.setupController(...arguments); let crate = this.modelFor('crate'); + // reset when crate changes + if (crate && crate !== controller.crate) { + controller.reset(); + } controller.set('crate', crate); - // TODO: Add error handling - waitForPromise(crate.loadVersionsTask.perform()); + // Fetch initial data only if empty + if (controller.data.length === 0) { + waitForPromise(controller.loadMoreTask.perform()); + } } } diff --git a/app/styles/crate/versions.module.css b/app/styles/crate/versions.module.css index 6ea10305341..acde094f036 100644 --- a/app/styles/crate/versions.module.css +++ b/app/styles/crate/versions.module.css @@ -26,3 +26,29 @@ margin-top: var(--space-2xs); } } + +.load-more { + --shadow: 0 1px 3px light-dark(hsla(51, 90%, 42%, .35), #232321); + + /* TODO: move to shared */ + composes: load-more from '../../styles/dashboard.module.css'; + + border: 0; + padding: 0 var(--space-m); + + button { + border-radius: var(--space-3xs); + box-shadow: var(--shadow); + cursor: pointer; + position: relative; + } + + .loading-spinner { + display: inline-flex; + align-items: center; + position: absolute; + height: 100%; + top: 0; + margin-left: var(--space-2xs); + } +} diff --git a/app/templates/crate/versions.hbs b/app/templates/crate/versions.hbs index f6909fee5c7..1582c8fdfc0 100644 --- a/app/templates/crate/versions.hbs +++ b/app/templates/crate/versions.hbs @@ -1,10 +1,10 @@ - +
- All {{ this.model.num_versions }} - versions of {{ this.model.name }} since - {{date-format this.model.created_at 'PPP'}} + {{ this.sortedVersions.length }} of {{ this.crate.num_versions }} + {{ this.crate.name }} versions since + {{date-format this.crate.created_at 'PPP'}}
@@ -23,3 +23,15 @@ {{/each}} +{{#if this.next_page}} +
+ +
+{{/if}} diff --git a/e2e/acceptance/crate.spec.ts b/e2e/acceptance/crate.spec.ts index 39077256f79..c943cbbb39f 100644 --- a/e2e/acceptance/crate.spec.ts +++ b/e2e/acceptance/crate.spec.ts @@ -123,14 +123,29 @@ test.describe('Acceptance | crate page', { tag: '@acceptance' }, () => { await expect(page.locator('[data-test-heading] [data-test-crate-name]')).toHaveText('foo-bar'); }); - test('navigating to the all versions page', async ({ page, msw }) => { + test('navigating to the versions page', async ({ page, msw }) => { loadFixtures(msw.db); + // default with a page size more than 13 await page.goto('/crates/nanomsg'); await page.click('[data-test-versions-tab] a'); await expect(page.locator('[data-test-page-description]')).toHaveText( - /All 13\s+versions of nanomsg since\s+December \d+th, 2014/, + /13 of 13\s+nanomsg versions since\s+December \d+th, 2014/, + ); + }); + + test('navigating to the versions page with custom per_page', async ({ page, msw }) => { + loadFixtures(msw.db); + + await page.goto('/crates/nanomsg/versions?per_page=10'); + await expect(page.locator('[data-test-page-description]')).toHaveText( + /10 of 13\s+nanomsg versions since\s+December \d+th, 2014/, + ); + + await page.getByTestId('load-more').click(); + await expect(page.locator('[data-test-page-description]')).toHaveText( + /13 of 13\s+nanomsg versions since\s+December \d+th, 2014/, ); }); diff --git a/tests/acceptance/crate-test.js b/tests/acceptance/crate-test.js index 35d2022ceb9..30ca2756f83 100644 --- a/tests/acceptance/crate-test.js +++ b/tests/acceptance/crate-test.js @@ -130,13 +130,25 @@ module('Acceptance | crate page', function (hooks) { assert.dom('[data-test-heading] [data-test-crate-name]').hasText('foo-bar'); }); - test('navigating to the all versions page', async function (assert) { + 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/); }); test('navigating to the reverse dependencies page', async function (assert) { diff --git a/tests/components/version-list-row-test.js b/tests/components/version-list-row-test.js index 384a9a43f3c..ece4dd5aeea 100644 --- a/tests/components/version-list-row-test.js +++ b/tests/components/version-list-row-test.js @@ -23,7 +23,7 @@ module('Component | VersionList::Row', function (hooks) { this.secondVersion = versions.find(it => it.num === '0.3.0-alpha.01'); await render(hbs``); - assert.dom('[data-test-release-track] svg').exists(); + assert.dom('[data-test-release-track]').hasText('0.4'); assert.dom('[data-test-release-track-link]').hasText('0.4.0-alpha.01'); await render(hbs``); diff --git a/tests/models/version-test.js b/tests/models/version-test.js index 56fac053c94..a24b7873f17 100644 --- a/tests/models/version-test.js +++ b/tests/models/version-test.js @@ -1,5 +1,7 @@ import { module, test } from 'qunit'; +import { calculateReleaseTracks } from '@crates-io/msw/utils/release-tracks'; + import { setupTest } from 'crates-io/tests/helpers'; import setupMsw from 'crates-io/tests/helpers/setup-msw'; @@ -169,6 +171,7 @@ module('Model | Version', function (hooks) { let crateRecord = await this.store.findRecord('crate', crate.name); let versions = (await crateRecord.loadVersionsTask.perform()).slice(); + crateRecord.release_tracks = calculateReleaseTracks(versions); assert.deepEqual( versions.map(it => ({ num: it.num, isHighestOfReleaseTrack: it.isHighestOfReleaseTrack })), @@ -200,6 +203,7 @@ module('Model | Version', function (hooks) { let crateRecord = await this.store.findRecord('crate', crate.name); let versions = (await crateRecord.loadVersionsTask.perform()).slice(); + crateRecord.release_tracks = calculateReleaseTracks(versions); assert.deepEqual( versions.map(it => ({ num: it.num, isHighestOfReleaseTrack: it.isHighestOfReleaseTrack })), @@ -218,6 +222,7 @@ module('Model | Version', function (hooks) { let crateRecord = await this.store.findRecord('crate', crate.name); let versions = (await crateRecord.loadVersionsTask.perform()).slice(); + crateRecord.release_tracks = calculateReleaseTracks(versions); assert.deepEqual( versions.map(it => ({ num: it.num, isHighestOfReleaseTrack: it.isHighestOfReleaseTrack })), @@ -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); assert.deepEqual( versions.map(it => ({ num: it.num, isHighestOfReleaseTrack: it.isHighestOfReleaseTrack })),