From fdcd9ea5866504b05d6e3596a93b167d6adc89da Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Fri, 4 Sep 2026 23:20:29 +0000 Subject: [PATCH] fix(routing): open the pathway a legacy news link points at 86 links in this site's own news archive open the pathway browser with nothing in it. They are the old browser's fragment links, and they come in two spellings because the old site produced both: #/R-HSA-1430728 770 links, work #R-HSA-202733 41 links, blank page #R-HSA-8853659.1 45 links, blank page FRAGMENT_PATTERN required the leading slash. Without one nothing matched, so no id was extracted and the browser opened on no pathway -- from a link in a release announcement, which is where someone arrives from a citation. The slash is now optional and the id is required, which is what keeps a fragment that is not a pathway -- `#introduction`, naming a section to scroll to -- falling through untouched instead of being read as a stale route. A trailing stIdVersion is consumed and dropped: the content service does not want it, and it used to arrive as a query parameter called ".1". Verified on beta: all three previously blank forms now open their pathway, and the two that already worked are unchanged, parameters included. The test cases are taken from what is actually in the content rather than invented, and fail against the old pattern -- bare id and version go red, the four that already worked stay green. Not covered: 37 links spelling `#TOOL=AT`, the old analysis tool fragment. They fall through as before rather than opening anything, and where they should land is a product decision rather than a regex. Co-Authored-By: Claude Opus 5 --- .../app/services/url-state.service.spec.ts | 69 +++++++++++++++++++ .../src/app/services/url-state.service.ts | 19 ++++- 2 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 projects/pathway-browser/src/app/services/url-state.service.spec.ts 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 new file mode 100644 index 00000000..5e8cf2ae --- /dev/null +++ b/projects/pathway-browser/src/app/services/url-state.service.spec.ts @@ -0,0 +1,69 @@ +/** + * Legacy pathway links in the URL fragment. + * + * The old browser addressed a pathway in the fragment, and this site's own news + * archive is full of those links: 770 spelled `#/R-HSA-1430728` and 86 spelled + * `#R-HSA-202733`, with no slash. The pattern used to require the slash, so the + * second kind matched nothing and opened the browser with no pathway in it. + * + * The cases below are taken from what is actually in the content, not invented. + */ +import { describe, expect, it } from 'vitest'; +import { FRAGMENT_PATTERN } from './url-state.service'; + +/** What the subscriber does with a fragment, reduced to its decisions. */ +function route(fragment: string) { + const match = fragment.match(FRAGMENT_PATTERN); + if (!match?.groups) return { id: undefined, params: {} as Record }; + const params: Record = {}; + if (match.groups['params']) { + for (const [key, value] of match.groups['params'].split('&').map((p) => p.split('='))) { + params[key] = value || true; + } + } + return { id: match.groups['id'], params }; +} + +describe('a legacy pathway link in the fragment', () => { + it('opens the pathway when the link carries a slash', () => { + expect(route('/R-HSA-1430728').id).toBe('R-HSA-1430728'); + }); + + it('opens the pathway when the link carries no slash', () => { + // 41 links in the news archive look like this, and every one of them used + // to land on an empty browser. + expect(route('R-HSA-202733').id).toBe('R-HSA-202733'); + expect(route('R-HSA-913531').id).toBe('R-HSA-913531'); + }); + + it('drops a stIdVersion rather than passing it on as a parameter', () => { + // 45 links carry one. It used to arrive as a query parameter named ".1". + for (const fragment of ['R-HSA-8853659.1', '/R-HSA-69231.4', 'R-HSA-3371497.12']) { + const { id, params } = route(fragment); + expect(id, fragment).toMatch(/^R-[A-Z]{3}-\d+$/); + expect(Object.keys(params), fragment).toEqual([]); + } + }); + + it('keeps the parameters an old link carries', () => { + const { id, params } = route('/R-HSA-8876384&PATH=R-HSA-1643685,R-HSA-5663205'); + expect(id).toBe('R-HSA-8876384'); + expect(params).toEqual({ PATH: 'R-HSA-1643685,R-HSA-5663205' }); + }); + + 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 + // parameter; they have to fall through untouched. + for (const fragment of ['introduction', 'summation', 'TOOL=AT', 'literature']) { + expect(route(fragment).id, fragment).toBeUndefined(); + expect(Object.keys(route(fragment).params), fragment).toEqual([]); + } + }); + + it('does not take an identifier out of the middle of something else', () => { + for (const fragment of ['see-R-HSA-202733-here', 'XR-HSA-202733']) { + expect(route(fragment).id, fragment).toBeUndefined(); + } + }); +}); 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 2242f656..00abb600 100644 --- a/projects/pathway-browser/src/app/services/url-state.service.ts +++ b/projects/pathway-browser/src/app/services/url-state.service.ts @@ -9,7 +9,24 @@ import type { Analysis } from '../model/analysis.model'; import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy'; import { toSignal } from '@angular/core/rxjs-interop'; -const FRAGMENT_PATTERN = /\/(?R-[A-Z]{3}-\d+)?&?(?.*)/; +/** + * A legacy pathway link, as the old browser addressed one in the URL fragment. + * + * Both spellings are in the wild, because the old site produced both: 770 links + * in this site's own news archive say `#/R-HSA-1430728` and 86 say + * `#R-HSA-202733` with no slash. Requiring the slash meant the second kind + * matched nothing, so every one of them opened the browser with no pathway in + * it -- a blank page from a link in a release announcement. + * + * The id must be there for this to match at all. A fragment that is not a + * pathway reference -- `#introduction`, naming a section to scroll to -- has to + * fall through untouched rather than be read as a stale route. + * + * 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 type UrlParam = WritableSignal & { otherTokens?: string[];