Skip to content

fix: hide the History link on the results and converters pages when HIDE_HISTORY is set - #620

Open
thejdubb02 wants to merge 2 commits into
C4illin:mainfrom
thejdubb02:fix/hide-history-556
Open

fix: hide the History link on the results and converters pages when HIDE_HISTORY is set#620
thejdubb02 wants to merge 2 commits into
C4illin:mainfrom
thejdubb02:fix/hide-history-556

Conversation

@thejdubb02

Copy link
Copy Markdown

Summary

When HIDE_HISTORY=true, the History nav link is hidden on most pages but still showed on two: the conversion results page and the converters list page. Both rendered <Header> without the hideHistory prop, so it defaulted to visible. The results page is the one seen in #556, right after a conversion completes.

Every other page that renders the logged-in header (root.tsx, history.tsx, user.tsx) already passes hideHistory={HIDE_HISTORY}. This does the same on results.tsx and listConverters.tsx, so the setting is respected everywhere.

Resolves #556

Changes

  • src/pages/results.tsx: pass hideHistory={HIDE_HISTORY} to <Header> (and import it).
  • src/pages/listConverters.tsx: same.
  • tests/pages/listConverters.test.ts: regression test that renders the converters page with HIDE_HISTORY=true and asserts the History link is absent. It fails without the fix.

Testing

  • bun test passes, including the new test. With the listConverters change reverted, the new test fails on the href="/history" assertion, so it genuinely guards the fix.
  • bun run lint is clean (tsc, eslint, prettier, knip, xss-scan).
The results and converters pages rendered <Header> without the hideHistory
prop, so the History nav link stayed visible on them even when HIDE_HISTORY
was set. Every other page that renders the logged-in header already passes
hideHistory={HIDE_HISTORY}; do the same here.

Adds a regression test that renders the converters page with HIDE_HISTORY set
and asserts the History link is absent (it fails without this change).

Resolves C4illin#556

Signed-off-by: Justin Willhite <5132924+thejdubb02@users.noreply.github.com>
@github-actions github-actions Bot added the Fix label Aug 26, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/pages/listConverters.test.ts">

<violation number="1" location="tests/pages/listConverters.test.ts:26">
P2: The PR's primary reported scenario is the results page (#556, the header shown right after a conversion), yet this regression test only covers the converters page. `tests/pages/results.test.ts` only exercises `buildDownloadUrl` and never renders the results page, so the `hideHistory={HIDE_HISTORY}` fix on `results.tsx` ships with no test guarding it. Add an equivalent page-rendering test for the results route (inserting a job row into the in-memory DB and asserting the History link is absent), or the results regression can silently return.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

}

// Regression test for #556.
test("converters page hides the History link when HIDE_HISTORY is set", async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The PR's primary reported scenario is the results page (#556, the header shown right after a conversion), yet this regression test only covers the converters page. tests/pages/results.test.ts only exercises buildDownloadUrl and never renders the results page, so the hideHistory={HIDE_HISTORY} fix on results.tsx ships with no test guarding it. Add an equivalent page-rendering test for the results route (inserting a job row into the in-memory DB and asserting the History link is absent), or the results regression can silently return.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/pages/listConverters.test.ts, line 26:

<comment>The PR's primary reported scenario is the results page (#556, the header shown right after a conversion), yet this regression test only covers the converters page. `tests/pages/results.test.ts` only exercises `buildDownloadUrl` and never renders the results page, so the `hideHistory={HIDE_HISTORY}` fix on `results.tsx` ships with no test guarding it. Add an equivalent page-rendering test for the results route (inserting a job row into the in-memory DB and asserting the History link is absent), or the results regression can silently return.</comment>

<file context>
@@ -0,0 +1,37 @@
+}
+
+// Regression test for #556.
+test("converters page hides the History link when HIDE_HISTORY is set", async () => {
+  const res = await listConverters.handle(
+    new Request("http://localhost/converters", { headers: { Cookie: sessionCookie() } }),
</file context>
Comment thread tests/pages/listConverters.test.ts
Addresses review feedback on the header tests:
- Pin WEBROOT="" so the History-link assertions stay deterministic in
  environments where WEBROOT is set.
- Add a results-page render test (the scenario in C4illin#556), seeding a job for the
  authenticated user and asserting the History link is absent. It fails without
  the hideHistory fix on results.tsx.

Signed-off-by: Justin Willhite <5132924+thejdubb02@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 participant