diff --git a/e2e/content-page-urls.spec.ts b/e2e/content-page-urls.spec.ts new file mode 100644 index 0000000..a542054 --- /dev/null +++ b/e2e/content-page-urls.spec.ts @@ -0,0 +1,65 @@ +/** + * The content pages own their own addresses. + * + * They render some of the same panels as the pathway browser, from the same + * UrlStateService, and that service has two effects that write pathway browser + * state into the URL. On a content page those have to stand down, or opening a + * detail page rewrites its address into a pathway browser one. + * + * The guard that does this used to be a substring test over the whole URL, so a + * query parameter carrying the word "content" or "query" decided it too -- and + * `sample` is a column name taken verbatim from the reader's own expression file. + * Both halves are covered here: the content pages still stand down, and the + * pathway browser still writes its URL when a parameter happens to say "content". + */ +import { expect, test } from '@playwright/test'; + +test.describe.configure({ timeout: 4 * 60 * 1000 }); + +const contentPages = [ + '/content/detail/R-HSA-1430728', + '/content/query?q=kinase', + '/content/schema/Pathway', +]; + +for (const path of contentPages) { + test(`${path} keeps its own address`, async ({ page }) => { + await page.goto(path, { waitUntil: 'domcontentloaded' }); + await page.waitForTimeout(4000); + + const landed = new URL(page.url()); + expect(landed.pathname, 'the page rewrote its own address').toContain(path.split('?')[0]); + }); +} + +test('the pathway browser still writes its URL when a parameter says "content"', async ({ + page, +}) => { + // `?sample=GC__content` is what an expression file with a column called + // "GC content" produces. Under the old guard this URL silenced every write + // that followed it: no selection, no flag, no tab, nothing to share. + await page.goto('/PathwayBrowser/R-HSA-109606?sample=GC__content', { + waitUntil: 'domcontentloaded', + }); + // The tabs are not there until the diagram is, and the default tab settles a + // moment after that. + await page.waitForFunction( + () => { + const container = document.querySelector('#cytoscape') as + (HTMLElement & { _cyreg?: { cy?: { elements(): { length: number } } } }) | null; + return (container?._cyreg?.cy?.elements().length ?? 0) > 0; + }, + { timeout: 90_000 } + ); + await page.waitForTimeout(2500); + + const molecule = page + .locator('[role="tab"]') + .filter({ hasText: /Molecule/i }) + .first(); + test.skip((await molecule.count()) === 0, 'this pathway offers no Molecule tab'); + await molecule.click(); + await page.waitForTimeout(2500); + + expect(new URL(page.url()).search, 'choosing a tab reached the URL').toContain('tab=molecule'); +}); 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 3678c9f..0946cbc 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 @@ -9,7 +9,7 @@ * 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'; +import { FRAGMENT_PATTERN, isContentRoute } from './url-state.service'; /** What the subscriber does with a fragment, reduced to its decisions. */ function route(fragment: string) { @@ -88,3 +88,41 @@ describe('a legacy pathway link in the fragment', () => { } }); }); + +/** + * Standing down on the content pages, without standing down anywhere else. + * + * The guard used to be a substring test over the whole URL, so a query parameter + * carrying either word decided it. `sample` is a column name out of the reader's + * own expression file, set automatically to the first column, and "GC content" is + * an ordinary thing for a column to be called. + */ +describe('recognising a content page', () => { + it('stands down on the content pages', () => { + expect(isContentRoute('/content/detail/R-HSA-1430728')).toBe(true); + expect(isContentRoute('/content/schema/Pathway')).toBe(true); + }); + + it('still stands down on the search page, which is a content page', () => { + expect(isContentRoute('/content/query')).toBe(true); + expect(isContentRoute('/content/query?q=kinase')).toBe(true); + }); + + it('does not stand down in the pathway browser', () => { + expect(isContentRoute('/PathwayBrowser/R-HSA-1430728')).toBe(false); + expect(isContentRoute('/PathwayBrowser/R-HSA-1430728?tab=details&sel=R-HSA-9')).toBe(false); + }); + + it('does not stand down because the reader named a column "GC content"', () => { + expect(isContentRoute('/PathwayBrowser/R-HSA-1430728?sample=GC__content')).toBe(false); + }); + + it('does not stand down for a value that merely contains the word', () => { + expect(isContentRoute('/PathwayBrowser/R-HSA-1?overlay=contents')).toBe(false); + expect(isContentRoute('/PathwayBrowser/R-HSA-1#content')).toBe(false); + }); + + it('does not stand down for the word "query" anywhere but a content path', () => { + expect(isContentRoute('/PathwayBrowser/R-HSA-1?sample=query__1')).toBe(false); + }); +}); 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 27e1b82..b98d46f 100644 --- a/projects/pathway-browser/src/app/services/url-state.service.ts +++ b/projects/pathway-browser/src/app/services/url-state.service.ts @@ -36,6 +36,34 @@ import { toSignal } from '@angular/core/rxjs-interop'; */ export const FRAGMENT_PATTERN = /^\/?(?R-[A-Z]{3}-\d+|\d{4,})(?:\.\d+)?(?:&(?.*))?$/; +/** + * Whether this URL is one of the content pages rather than the pathway browser. + * + * The content pages render some of the same panels from this same service, but + * they own their own addresses -- so the two effects that write pathway browser + * state into the URL have to stand down there, or opening a detail page rewrites + * its address into a pathway browser one. + * + * The path, and whole segments of it. This used to ask whether the whole URL + * *contained* "content" or "query", and the query string answers that just as + * readily as the path does: `sample` holds a column name out of the reader's own + * expression file -- set automatically to the first column of it -- so a file + * with a column called "GC content" puts `?sample=GC__content` in the URL, and + * from that moment nothing the reader did was written to the URL again. Not + * selecting a node, not flagging, not changing tab. None of it survived a + * reload, and none of it was in a link they shared. + * + * "query" is gone rather than fixed: the search page is `content/query`, which + * the first test already covers, and of the two words it is the likelier to turn + * up in somebody's data. + */ +export function isContentRoute(url: string): boolean { + return url + .split(/[?#]/, 1)[0] + .split('/') + .some((segment) => segment === 'content'); +} + export type UrlParam = WritableSignal & { otherTokens?: string[]; initialValue: T; @@ -161,11 +189,7 @@ export class UrlStateService implements State { effect(() => { // console.log('Updating patwhayId to ', this.pathwayId()) - //If in content/search do not navigate - if (this.router.url.includes('content') || this.router.url.includes('query')) { - // console.log('In content or search route, not navigating on pathwayId change'); - return; - } + if (isContentRoute(this.router.url)) return; void this.navigateTo(this.pathwayId() ?? null, { queryParamsHandling: 'preserve', @@ -274,7 +298,7 @@ export class UrlStateService implements State { }); effect(() => { const queryParams = this.currentQueryParams(); - if (this.router.url.includes('content') || this.router.url.includes('query')) return; + if (isContentRoute(this.router.url)) return; // Settling is not a step the reader took, so it replaces rather than adds: // opening a pathway wrote ?tab=info and then ?tab=details, two entries for a