diff --git a/RELEASE-TESTING.md b/RELEASE-TESTING.md index 69fbe960..e5b6c4f7 100644 --- a/RELEASE-TESTING.md +++ b/RELEASE-TESTING.md @@ -46,13 +46,13 @@ rows, the rows are right: this line has drifted twice from being edited by hand. ## Front page -| Item | Status | -| ------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| Version number and release date at the bottom of the homepage | **auto** — `e2e/release/content-currency.spec.ts` compares it against `/data/database/version` | -| Every button links where it says, including the participating-institute links | **auto** — `homepage.spec.ts` covers the shortcut cards, and the institute logos: each has an absolute destination that answers, and each logo really draws | -| News is current, and the links inside the latest news item work | **auto** — `e2e/release/content-currency.spec.ts` checks the newest announcement against the release being served. _Announcements are imported verbatim by `npm run import:news`; nothing generates their prose_ | -| Search `p53` returns >1700 results, confined to Homo sapiens or species-less entities | **auto** — `e2e/release/release-checklist.spec.ts` | -| A newly added pathway, reaction and complex render; and an old one | **gap** — the checklist means "something added in _this_ release", which a test cannot hardcode. It does not need to: the release announcement we import names the new and updated pathways as links, so the release suite could read that list and load each one. Until then a person picks from the announcement | +| Item | Status | +| ------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Version number and release date at the bottom of the homepage | **auto** — `e2e/release/content-currency.spec.ts` compares it against `/data/database/version` | +| Every button links where it says, including the participating-institute links | **auto** — `homepage.spec.ts` covers the shortcut cards, and the institute logos: each has an absolute destination that answers, and each logo really draws | +| News is current, and the links inside the latest news item work | **auto** — `e2e/release/content-currency.spec.ts` checks the newest announcement against the release being served. _Announcements are imported verbatim by `npm run import:news`; nothing generates their prose_ | +| Search `p53` returns >1700 results, confined to Homo sapiens or species-less entities | **auto** — `e2e/release/release-checklist.spec.ts` | +| A newly added pathway, reaction and complex render; and an old one | **auto** — `e2e/release/new-in-this-release.spec.ts`. The announcement we publish names the new and updated pathways, so the suite reads that list from the newest release note in the repo and opens each one exactly as written — 21 of them for v97. It also asserts each lands on a stable id: the announcements link by dbId, which is not stable across releases. | ## Navigation bar diff --git a/e2e/release/new-in-this-release.spec.ts b/e2e/release/new-in-this-release.spec.ts new file mode 100644 index 00000000..abe00df6 --- /dev/null +++ b/e2e/release/new-in-this-release.spec.ts @@ -0,0 +1,89 @@ +import { test, expect } from '@playwright/test'; +import { readFileSync, readdirSync } from 'node:fs'; + +/** + * Everything the release announcement says is new actually opens. + * + * The release document asks a person to check that "a newly added pathway, + * reaction and complex render" -- which a test cannot hardcode, because what is + * new changes every release. It does not need to: the announcement we publish + * names them, and that list is in the repo. + * + * It writes them as bare dbIds -- `/PathwayBrowser/#1280218` -- which is how the + * old browser addressed a pathway. Until #182 none of those links worked at all: + * 278 of them across the news, 24 in the current release's announcement, every + * one opening the browser with no pathway in it. So this checks both halves at + * once, the links and the pathways they point at. + */ + +const NEWS = 'projects/website-angular/content/about/news'; + +/** The newest release announcement in the repo, and the pathways it links. */ +function announced(): { file: string; ids: string[] } { + const files = readdirSync(NEWS).filter( + (name) => /released|news/i.test(name) && name.endsWith('.mdx') + ); + // Named with a leading sequence number, so the highest is the newest. + const newest = files + .map((name) => ({ name, order: Number(/^(\d+)/.exec(name)?.[1] ?? 0) })) + .sort((a, b) => b.order - a.order)[0]; + if (!newest) return { file: '', ids: [] }; + + const body = readFileSync(`${NEWS}/${newest.name}`, 'utf8'); + const ids = [ + ...new Set( + [...body.matchAll(/PathwayBrowser\/#(\d{4,}|R-[A-Z]{3}-\d+)/g)].map((match) => match[1]) + ), + ]; + return { file: newest.name, ids }; +} + +test.describe('What the release announcement says is new', () => { + test('every pathway it links to opens', async ({ context }) => { + const { file, ids } = announced(); + test.skip(!ids.length, `no pathway links found in ${file || 'any announcement'}`); + + // Each one is a real diagram or illustration load. + test.setTimeout(12 * 60 * 1000); + console.log(`Checking ${ids.length} pathways linked from ${file}`); + + const failures: string[] = []; + + // Three at a time, as the top-level coverage test does: sequentially this is + // twenty-odd diagram loads and the suite has a budget. + for (let start = 0; start < ids.length; start += 3) { + await Promise.all( + ids.slice(start, start + 3).map(async (id) => { + const page = await context.newPage(); + try { + // Exactly as the announcement writes it, fragment and all. + await page.goto(`/PathwayBrowser/#${id}`, { waitUntil: 'domcontentloaded' }); + await page.waitForFunction( + () => { + const container = document.querySelector('#cytoscape') as + (HTMLElement & { _cyreg?: { cy?: { elements(): { length: number } } } }) | null; + const drawn = container?._cyreg?.cy?.elements().length ?? 0; + return drawn > 0 || Boolean(document.querySelector('cr-ehld svg')); + }, + { timeout: 90_000 } + ); + // And it left the reader on a stable id. A dbId is not stable + // across releases, so a URL carrying one is a URL not worth + // keeping -- an old link should hand over a good one. + const landed = new URL(page.url()).pathname; + if (!/R-[A-Z]{3}-\d+/.test(landed)) { + failures.push(`${id} opened but left a dbId in the URL: ${landed}`); + } + } catch { + const landed = page.url(); + failures.push(`${id} (landed on ${landed})`); + } finally { + await page.close(); + } + }) + ); + } + + expect(failures, `announced pathways that did not open, from ${file}`).toEqual([]); + }); +}); diff --git a/projects/pathway-browser/src/app/services/url-state.service.spec.ts b/projects/pathway-browser/src/app/services/url-state.service.spec.ts index 5e8cf2ae..3678c9fd 100644 --- a/projects/pathway-browser/src/app/services/url-state.service.spec.ts +++ b/projects/pathway-browser/src/app/services/url-state.service.spec.ts @@ -51,6 +51,27 @@ describe('a legacy pathway link in the fragment', () => { expect(params).toEqual({ PATH: 'R-HSA-1643685,R-HSA-5663205' }); }); + it('opens the pathway a release announcement links to', () => { + // Every announcement writes its list of what is new as bare dbIds: 278 in + // this site's own news, 24 in the current release's. The browser already + // resolves a dbId given in the path; only the fragment form was missing. + for (const [fragment, id] of [ + ['1280218', '1280218'], + ['/9932451', '9932451'], + ['73864', '73864'], + ]) { + expect(route(fragment).id, fragment).toBe(id); + } + }); + + it('does not mistake a page anchor for a dbId', () => { + // A section anchor is short or not a number at all. The shortest dbId in + // the content is five digits, so four is the floor. + for (const fragment of ['12', '999', 'top', 'section-2', '2024-news']) { + expect(route(fragment).id, fragment).toBeUndefined(); + } + }); + it('leaves a fragment that is not a pathway alone', () => { // A section to scroll to, and the old analysis-tool fragment. Matching // these would turn them into a route to nowhere and a junk query diff --git a/projects/pathway-browser/src/app/services/url-state.service.ts b/projects/pathway-browser/src/app/services/url-state.service.ts index 00abb600..18f5955b 100644 --- a/projects/pathway-browser/src/app/services/url-state.service.ts +++ b/projects/pathway-browser/src/app/services/url-state.service.ts @@ -22,11 +22,19 @@ import { toSignal } from '@angular/core/rxjs-interop'; * pathway reference -- `#introduction`, naming a section to scroll to -- has to * fall through untouched rather than be read as a stale route. * + * A bare number is a **dbId**, which is how every release announcement writes + * its list of what is new: `#1280218`, not `#R-HSA-1280218`. There are 278 of + * those in this site's own news, 24 in the current release's announcement, and + * every one opened the browser with no pathway in it. The browser already + * resolves a dbId given in the path, so only the fragment form was missing. + * Four digits at least, so an ordinary page anchor cannot be mistaken for one -- + * the shortest dbId in the content is five. + * * A trailing `.4` is a stIdVersion. The old links carry it, the content service * does not want it, and it was previously parsed as a query parameter called * ".4"; it is consumed and dropped here. */ -export const FRAGMENT_PATTERN = /^\/?(?R-[A-Z]{3}-\d+)(?:\.\d+)?(?:&(?.*))?$/; +export const FRAGMENT_PATTERN = /^\/?(?R-[A-Z]{3}-\d+|\d{4,})(?:\.\d+)?(?:&(?.*))?$/; export type UrlParam = WritableSignal & { otherTokens?: string[]; @@ -63,6 +71,20 @@ export class UrlStateService implements State { private router: Router = inject(Router); private http: HttpClient = inject(HttpClient); + /** + * The stable id for a dbId. + * + * Falls back to the dbId when the lookup fails, and says so: a page that + * loads on a less-good URL beats a link that goes nowhere, and the alternative + * -- refusing to navigate -- would turn a working legacy link into a dead one. + */ + private stableIdFor(dbId: string) { + return this.http.get<{ stId?: string }>(`${CONTENT_SERVICE}/data/query/${dbId}`).pipe( + map((object) => object?.stId), + catchError(() => of(undefined)) + ); + } + private readonly tabsCompatibility: [string | null, string][] = [ ['ST', 'details'], [null, 'details'], @@ -186,12 +208,25 @@ export class UrlStateService implements State { } } - void this.navigateTo(id ?? null, { - queryParamsHandling: 'merge', - fragment: fragment.replace(FRAGMENT_PATTERN, ''), - preserveFragment: false, - queryParams: params, - }); + const go = (resolved: string | undefined) => + void this.navigateTo(resolved ?? null, { + queryParamsHandling: 'merge', + fragment: fragment.replace(FRAGMENT_PATTERN, ''), + preserveFragment: false, + queryParams: params, + }); + + // A legacy link may name a pathway by dbId. The browser can load one, + // but the reader would then be left on a dbId URL to copy and share, + // and a dbId is not stable across releases. Resolve it and navigate to + // the stable id instead, so an old link hands over a good one. + if (id && /^\d+$/.test(id)) { + this.stableIdFor(id) + .pipe(untilDestroyed(this)) + .subscribe((stId) => go(stId ?? id)); + } else { + go(id); + } } });