From 99ba5ef860b0d51f1528c29f59564d6811ae2302 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Wed, 16 Sep 2026 15:17:16 +0000 Subject: [PATCH 1/2] fix(interactors): make the shared link work, and let a custom resource be put away Two faults in the custom-resource work, both found by reviewing it rather than by any failure, and the first is a promise the app was not keeping. **"Share by link" produced a link that drew nothing.** Ticking the box uploads the data precisely so the overlay can be opened by someone else. The address carries `?overlay=`, and on a fresh load that string was cast straight to an InteractorToken -- so `summary` was undefined and `getCustomResourceInteractors` returned at its first line, silently. Measured: the link opened with 0 badges where the session that made it had 1. A string is now resolved to a token object, and listed in the panel, so whoever follows the link can see and clear what it turned on rather than facing an overlay with no control for it. The service has no endpoint that names a token -- `token//summary` is a 404, measured -- so it is labelled by its first characters: "Shared (d4f9f8da)". Honest about being someone else's resource, and identifiable against the address. **Clicking an active custom resource never put it away.** The toggle read `resource.summary?.token` while `currentResource()` holds the resource's *name*, so the two could never match -- the one gesture that branch exists for, and the one asked for when it was built. It compares names now, for all three kinds of resource. Shown red against both: clicking again left the badge drawn, expected 0. Co-Authored-By: Claude Opus 5 --- e2e/custom-interactor-dialog.spec.ts | 73 +++++++++++++++++++ .../app/interactors/interactors.component.ts | 47 +++++++++++- 2 files changed, 117 insertions(+), 3 deletions(-) diff --git a/e2e/custom-interactor-dialog.spec.ts b/e2e/custom-interactor-dialog.spec.ts index aa14129d..f12d05ef 100644 --- a/e2e/custom-interactor-dialog.spec.ts +++ b/e2e/custom-interactor-dialog.spec.ts @@ -329,3 +329,76 @@ test.describe('The dialog layout', () => { }); } }); + +/** + * What the "share by link" choice actually promises. + * + * Ticking it uploads the data so the overlay can be opened by someone else. It + * could not: the address carries `?overlay=`, and on a fresh load that + * string was cast straight to an InteractorToken, so `summary` was undefined and + * the fetch returned at its first line. Silently. Measured before the fix: the + * link opened with 0 badges where the session that made it had 1. + * + * Also here because the same block identifies a custom resource by token while + * `currentResource()` holds its name, so clicking an active one never put it + * away -- the one gesture that branch exists for. + */ +test.describe('An overlay shared by link', () => { + test.describe.configure({ timeout: 5 * 60 * 1000 }); + + const badges = (page: Page) => + page.evaluate(() => { + const cy = (document.querySelector('#cytoscape') as CytoscapeHost | null)?._cyreg?.cy; + return cy?.nodes('.InteractorOccurrences').length ?? 0; + }); + + test('opens for whoever follows it, and can be put away again', async ({ page }) => { + await page.goto(`/PathwayBrowser/${PATHWAY}`, { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#cytoscape canvas', { timeout: BOOT_TIMEOUT }); + await page.waitForTimeout(4000); + + const accessions = await page.evaluate(() => { + const cy = (document.querySelector('#cytoscape') as CytoscapeHost | null)?._cyreg?.cy; + if (!cy) throw new Error('no cytoscape instance on #cytoscape'); + return [...new Set(cy.nodes('[acc]').map((node) => node.data('acc') as string))].filter( + Boolean + ); + }); + + await page.locator('.species-interactor-container .interactor').click(); + await page.waitForTimeout(1000); + await page.getByRole('button', { name: 'Add overlay resource' }).click(); + await expect(dialog(page)).toHaveCount(1); + await page.getByLabel('Name').fill('SharedByLink'); + await page.getByRole('radio', { name: /copy & paste/i }).click(); + await page.locator('textarea').fill(`#ID_A\t${'ID_B'}\n${accessions[0]}\tQ99741\n`); + // The opt-in, which is what makes a token at all. + await page.locator('.share-choice input').check(); + await page.getByRole('button', { name: /submit/i }).click(); + await expect(dialog(page)).toHaveCount(0, { timeout: 60_000 }); + + await expect.poll(() => badges(page), { timeout: 30_000 }).toBeGreaterThan(0); + const drawn = await badges(page); + + const shared = page.url(); + expect(shared, 'the address carries a token').toMatch(/overlay=[0-9a-f]{8,}/); + + // Clicking the active custom resource puts it away. + await page.locator('cr-interactors mat-list-option').first().click(); + await expect + .poll(() => badges(page), { message: 'clicking it again puts it away', timeout: 30_000 }) + .toBe(0); + + // A fresh load of the link someone was given. + await page.goto(shared, { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#cytoscape canvas', { timeout: BOOT_TIMEOUT }); + await expect + .poll(() => badges(page), { message: 'the shared link draws it', timeout: BOOT_TIMEOUT }) + .toBe(drawn); + + // And it is listed, so whoever followed the link can clear it rather than + // facing an overlay with no control for it. + await page.locator('.species-interactor-container .interactor').click(); + await expect(page.locator('cr-interactors mat-list-option')).toContainText(/Shared \(/); + }); +}); diff --git a/projects/pathway-browser/src/app/interactors/interactors.component.ts b/projects/pathway-browser/src/app/interactors/interactors.component.ts index 680d8e74..1b5e5996 100644 --- a/projects/pathway-browser/src/app/interactors/interactors.component.ts +++ b/projects/pathway-browser/src/app/interactors/interactors.component.ts @@ -197,8 +197,11 @@ export class InteractorsComponent implements AfterViewInit { // the highlighted thing to un-highlight it -- and it did nothing at all, so // the only way out was the separate "Clear overlays" button, which is easy // to miss when the button you just pressed looks like it should work. - // A custom resource is identified by its token, a named one by its name. - const name = typeof resource === 'string' ? resource : resource.summary?.token; + // By name, for all three kinds. This read `resource.summary?.token` for a + // custom resource while `currentResource()` holds its *name*, so the two + // could never match and clicking an active custom resource never put it + // away -- the one gesture this branch exists for. + const name = typeof resource === 'string' ? resource : resource.summary?.name; if (chosenByReader && name && this.currentResource().name === name) { this.clearInteractors(); return; @@ -219,7 +222,13 @@ export class InteractorsComponent implements AfterViewInit { this.getPsicquicResourceInteractors(resource as string); break; case ResourceType.CUSTOM: - this.getCustomResourceInteractors(resource as InteractorToken); + // A string here is a token out of the address, not a token object. + // It used to be cast straight to InteractorToken, so `summary` was + // undefined and `getCustomResourceInteractors` returned at its first + // line -- silently. That is why a link carrying a shared overlay + // opened with nothing drawn, which is the whole of what the "share + // by link" choice promises. + this.getCustomResourceInteractors(this.asToken(resource)); break; default: throw new Error('Unknown resource type encountered: ' + resourceType); @@ -233,6 +242,38 @@ export class InteractorsComponent implements AfterViewInit { }); } + /** + * A token object for something that may only be a token string. + * + * A shared address carries `?overlay=` and nothing else, so on a fresh + * load there is no entry in `resourceTokens` to find -- those live only for as + * long as the page does. One is made, and listed, so the reader can see and + * clear what the link turned on rather than facing an overlay with no control + * for it. + * + * The service has no endpoint that names a token -- `token//summary` is a + * 404, measured -- so it is labelled by its first characters. Honest about + * being someone else's resource, and identifiable against the address. + */ + private asToken(resource: string | InteractorToken): InteractorToken { + if (typeof resource !== 'string') return resource; + + const known = this.resourceTokens?.find((token) => token.summary?.token === resource); + if (known) return known; + + const made: InteractorToken = { + summary: { + token: resource, + name: `Shared (${resource.slice(0, 8)})`, + fileName: resource, + interactors: 0, + interactions: 0, + }, + }; + this.resourceTokens?.push(made); + return made; + } + getStaticInteractors(resource: string | null) { if (resource) { this.clear = false; From 45651c2c0b84d5193d4b0fadd7f85a6e8d3d6cb0 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Wed, 16 Sep 2026 15:41:13 +0000 Subject: [PATCH 2/2] fix(interactors): delete a shared resource from the diagram too Found reviewing this PR before merging it, which is the point of doing so. #206 fixed `deleteCustomResource` to remove elements by the resource's name rather than by an object stringified to "[object Object]". That covered a resource read in this page, which is drawn under its name, and missed one fetched from the service, which is drawn under the token the response names. So deleting a shared resource still left its badges on the diagram -- the same fault, still live on the other half of the path. Measured on beta: one badge before deleting, one after. It removes by both identifiers now, and the shared-link case asserts the diagram is clear afterwards rather than only that the list is empty -- which is the assertion that let this through the first time. Co-Authored-By: Claude Opus 5 --- e2e/custom-interactor-dialog.spec.ts | 12 ++++++++++++ .../src/app/interactors/interactors.component.ts | 12 +++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/e2e/custom-interactor-dialog.spec.ts b/e2e/custom-interactor-dialog.spec.ts index f12d05ef..ba5c9649 100644 --- a/e2e/custom-interactor-dialog.spec.ts +++ b/e2e/custom-interactor-dialog.spec.ts @@ -400,5 +400,17 @@ test.describe('An overlay shared by link', () => { // facing an overlay with no control for it. await page.locator('.species-interactor-container .interactor').click(); await expect(page.locator('cr-interactors mat-list-option')).toContainText(/Shared \(/); + + // Deleting it takes it off the diagram. A shared resource is drawn under the + // token the service names, not under the resource's name, so removing by + // name alone left the badges behind -- the same fault #206 fixed for a + // locally read resource, still live on this half of the path. + await page.locator('cr-interactors mat-list-option button').first().click(); + await expect + .poll(() => badges(page), { + message: 'deleting a shared resource clears it', + timeout: 30_000, + }) + .toBe(0); }); }); diff --git a/projects/pathway-browser/src/app/interactors/interactors.component.ts b/projects/pathway-browser/src/app/interactors/interactors.component.ts index 1b5e5996..88e09a79 100644 --- a/projects/pathway-browser/src/app/interactors/interactors.component.ts +++ b/projects/pathway-browser/src/app/interactors/interactors.component.ts @@ -383,9 +383,19 @@ export class InteractorsComponent implements AfterViewInit { const name = resource.summary?.name; if (!name) return; + // By name *and* by token. A resource read in this page is drawn under its + // name, and one fetched from the service under the token the response names + // -- so removing by name alone left a shared resource's badges on the + // diagram, which is the same fault this method was just fixed for, still + // live on the other half of the path. Measured: one badge before deleting, + // one after. + const identifiers = [name, resource.summary?.token].filter( + (identifier): identifier is string => !!identifier + ); + const drawn = this.currentResource().name === name; this.cys()?.forEach((cy) => { - cy.elements(`[resource = '${name}']`).remove(); + identifiers.forEach((identifier) => cy.elements(`[resource = '${identifier}']`).remove()); }); // Held in memory only for resources parsed in the page; harmless otherwise.