OpenConceptLab/ocl_issues#2869 | TBv2's apps menu links to the OCL Mapper

[paynejd]

closes OpenConceptLab/ocl_issues#2869

TBv2's apps menu (the grid icon in the header) gets an OCL Mapper tile next to TermBrowser and Bulk Importer.

• Same icon as the Mapper entry in the left-nav Tools menu (MotionPhotosAutoOutlined).
• Links via toMapperURL(), so each environment opens its own Mapper (host routing is unchanged). It opens in a new tab, like the Tools entry.
• The popper's min width goes from 330px to min(390px, 100vw - 16px) so the three tiles come out equal (at 330px the TermBrowser label made its tile wider than the others) and the menu still fits a phone screen.
• toMapperURL() drops the current page's query string from the referrer. The Mapper reads auth=true from the segment after the first ? in the referrer's hash, so from TBv2 pages with a query (search, concept lists) signed-in users weren't auto-signed-in on the Mapper. This also fixes the existing Tools link.
• Sites that hide both TermBrowser and Importer still get no apps menu (the hideAppsMenu logic is unchanged).

Checked: CI's ESLint command passes. Referrer and auth parsing was simulated against the Mapper's parser. The layout was checked at desktop and 375px widths in a static copy of the menu (same SCSS rules, popper width and type), not in a running TBv2: its dev server needs Node 14 + node-sass 4. Codex passes 1–6 are below; pass 6 is clean.

🤖 Generated with Claude Code

[paynejd]

Codex adversarial review, pass 1 (codex-cli 0.159.3, commit 68fe6dc)

Found three issues, all P2 (medium):

1. Demo links to the wrong Mapper environment. AppsMenu.jsx:64 calls toMapperURL(), whose generic hostname branch overwrites its explicit demo → QA mapping. Executing the helper confirmed app.demo.openconceptlab.org produces map.demo.openconceptlab.org, despite the preceding branch specifying map.qa.openconceptlab.org. Fix: make those branches mutually exclusive. This helper defect already affects the Tools entry; the new tile inherits it.

2. The wider popup cannot fit common mobile viewports. AppsMenu.jsx:39 imposes a 390px minimum, plus the Paper border. It cannot fit a 375px viewport; Popper repositioning cannot shrink it. This also applies when only two tiles are visible. Fix: use a viewport-constrained width and responsive minimum width; give anchors flex: 1 and min-width: 0 so visible tiles share available space.

3. Hiding both existing apps also hides the new Mapper tile. The unconditional tile at AppsMenu.jsx:64 is unreachable when both hide flags are true, because Header.jsx:184 still suppresses the entire menu. Fix: determine menu visibility from all enabled apps, including an explicit Mapper availability flag. That flag can also suppress Mapper on FHIR/custom deployments without a suitable destination.

Render-time URL evaluation avoids the Tools menu’s module-time staleness. The new anchor supports keyboard activation and safely opens a new tab. No additional concrete accessibility regression found.

Review was read-only; URL behavior was checked with Node. Layout findings are from source inspection, without browser rendering.

[paynejd]

Responses to Codex pass 1 (fixed in 4bf01e1):

1. Demo → wrong Mapper host: fixed. toMapperURL()'s branches are now else if, so app.demo and app.qa go to map.qa.openconceptlab.org. map.demo.openconceptlab.org doesn't resolve in DNS, so the left-nav Tools link on demo was broken before this PR too. Checked: prod → map, qa/demo → map.qa, staging → map.staging, localhost → :4004, any other host → map.
2. Too wide on phones: fixed. The min width is now min(390px, 100vw - 16px). At a 375px viewport the menu is 361px wide, the three tiles are 120px each, and nothing scrolls sideways (checked in a static copy of the menu).
3. Hiding both apps also hides the Mapper: kept as designed. The ticket's acceptance criteria say that sites hiding both TermBrowser and Importer still get no apps menu, so those deployments don't suddenly get an OCL Mapper link. The left-nav Tools menu already links the OCL Mapper on every deployment that shows the left nav. A hideMapperApp flag can follow if a custom deployment asks for one.

[paynejd]

Codex adversarial review, pass 2 (codex-cli 0.159.3, commit 4bf01e1)

One concrete finding:

• P2 — Render-time Mapper URLs embed an unescaped referrer. AppsMenu.jsx:64 calls the helper with the current route, exposing its existing query-encoding bug at utils.js:1191. From /search?q=diabetes&page=2, it produces:

  …/#/?referrer=https://…/search?q=diabetes&page=2?auth=true
  

  Query parsing truncates referrer before &page and treats page as a Mapper parameter. Suggested fix: encode the complete referrer with URLSearchParams, preserving the Mapper’s expected authentication convention.

No other concrete regressions established. The demo/QA ordering is corrected; I did not re-raise the intentional both-hidden behavior. Review was read-only, with the URL issue reproduced in Node; layout and accessibility were inspected statically.

[paynejd]

Response to Codex pass 2 (fixed in c052580):

Unescaped referrer: fixed, though differently from the suggestion. The Mapper's forceLoginUser() (oclmap src/components/app/App.jsx) reads auth from the segment after the first ? in the referrer's hash. From a TBv2 page with its own query string, e.g. #/search/?q=diabetes&page=2, that segment is the page's query, so a signed-in TBv2 user wasn't auto-signed-in on the Mapper. URL-encoding the referrer wouldn't fix this without a matching Mapper change, so toMapperURL() now drops the page's query from the referrer. The Mapper only uses the referrer to recognise OCL clients and read auth. Simulated with history v4's parsePath plus the Mapper's check: #/, #/search/?q=diabetes&page=2 and #/orgs/CIEL/sources/CIEL/concepts/?q=malaria all yield auth=true now (the last two yielded null before). This also fixes the existing Tools link.

[paynejd]

Codex adversarial review, pass 3 (codex-cli 0.159.3, commit c052580)

No concrete new findings in git diff origin/master...HEAD.

Checked URL selection, render-time evaluation, Mapper auth parsing, accessibility, hidden-app layouts, and FHIR/custom-site behavior. All 16 focused URL/auth cases passed; git diff --check passed.

Layout review was static; I did not run a browser. No files changed.

[paynejd]

Codex adversarial review, pass 4 (codex-cli 0.159.3, commit 4d6dc68)

One concrete finding:

• [P2] OCL hosts with explicit ports fall back to production Mapper — src/common/utils.js:1188. window.location.host includes the port, so the anchored regex rejects app.staging.openconceptlab.org:8443, sending users to https://map.openconceptlab.org instead of staging. Production, QA, and demo with ports also hit this fallback; production loses its port.

  Suggested fix: Use window.location.hostname for both the QA/demo comparison and the regex. Keep the origin replacement to preserve ports for production/staging; QA/demo then consistently route to https://map.qa.openconceptlab.org.

Executed the actual before/after function across the hostname cases. Portless production, staging, QA, demo, localhost with a port, and unrelated hosts retain their mappings. Lookalike domains now correctly fall back to production. No other findings in the incremental diff or its full PR context.

[paynejd]

Codex passes 3 and 4, plus a CodeQL fix in between:

• CodeQL, "Incomplete regular expression for hostnames" (4d6dc68): the old 'app.*.openconceptlab.org' pattern counted as new because this PR touched its line. It's now anchored and escaped: /^app\.([a-z0-9-]+\.)*openconceptlab\.org$/. It still matches prod, staging, qa and demo, and no longer matches lookalikes such as app.x.openconceptlab.org.evil.com. CodeQL passes.
• Pass 4, hosts with a port (669a647): fixed. Both host checks now use location.hostname, and the origin replacement keeps the port. Checked: app.staging…:8443 → map.staging…:8443; app.demo…:8443 → map.qa; prod, qa, localhost and other hosts are unchanged.

[paynejd]

Codex adversarial review, pass 5 (codex-cli 0.159.3, commit 669a647)

One finding, pre-existing in the localhost check:

• src/common/utils.js:1184 — host.includes('localhost') misroutes lookalikes such as localhost.evil.test and notlocalhost.example.org to http://localhost:4004. It also overrides the OCL mapping for app.localhost.openconceptlab.org. Suggested fix: use window.location.hostname === 'localhost'.

No findings in the two-line pass-5 change itself. Executed 24 host/port cases: prod and staging preserve ports; QA and demo map to https://map.qa.openconceptlab.org with or without source ports; localhost maps to port 4004; other tested non-OCL hosts and lookalikes fall back to production Mapper.

Confirmed git diff 4d6dc68d..HEAD changes only the two checks from host to hostname.

Response: the localhost finding predates this PR, and the PR doesn't touch that line. A lookalike host only misroutes its own users to their own localhost:4004, so it stays out of scope here. The PR's own changes are clean as of this pass.

[paynejd]

Codex adversarial review, pass 6 (codex-cli 0.159.3, commit fbb3d3b)

No concrete findings in the PR’s changes.

Reviewed the full git diff origin/master...HEAD, surrounding menu styles, and Mapper’s forceLoginUser parser. Query stripping preserves auth detection in the checked URL cases. Host routing is unchanged and intentional. git diff --check passes.

Review was read-only; no browser rendering or build was run.

[paynejd]

Host routing restored to master (fbb3d3b), per Jon. Each environment's TBv2 links to its own Mapper: demo → demo, QA → QA. The demo environment is inactive for now, and when it comes back, demo instances point to other demo instances. That reverts the else if change from pass 1 and the regex and hostname follow-ups (4bf01e1, 4d6dc68, 669a647). Those lines are now identical to master, so CodeQL's hostname finding no longer applies to this PR. The referrer query fix (c052580) and the apps-menu tile stay.