🐛 Fixed Portal crash when navigating via hash links on a page

[kevinansfield]

Summary

• Portal's updateStateForPreviewLinks() (the hashchange event handler) called fetchLinkData() without arguments, leaving site and member as undefined
• This caused a crash when clicking portal hash links (e.g. <a href="#/portal/account/profile">) on an already-loaded page
• Loading the full URL directly (e.g. http://localhost:2368/#/portal/account/profile) worked fine because the initial fetchData() code path passed the arguments correctly
• Fixed by passing this.state.site and this.state.member to fetchLinkData() in updateStateForPreviewLinks()
• Added tests for both logged-in and logged-out hashchange navigation

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

Walkthrough

App.updateStateForPreviewLinks was changed to call fetchLinkData with this.state.site and this.state.member as arguments. New hashchange-based tests were added covering authenticated and unauthenticated navigation to account routes (including plans, profile, newsletters) and verifying sign-in redirection and magic-link request behavior. The portal package version in apps/portal/package.json was bumped from 2.64.2 to 2.64.3.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing a Portal crash when navigating via hash links, which is the core issue addressed in this PR.
Description check	✅ Passed	The description is directly related to the changeset, providing clear context about the bug, root cause, fix, and tests added.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-portal-hashchange-crash

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

apps/portal/test/portal-links.test.js (1)

388-413: Authenticated tests are nested inside the "unauthenticated account page access" describe block.

These tests verify logged-in behavior but live under the describe('unauthenticated account page access', ...) block (line 347). Consider moving them to their own describe or renaming the parent block (e.g., "hashchange account page access").

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/portal/test/portal-links.test.js` around lines 388 - 413, Tests using
test.each(...) that assert logged-in behavior are currently placed inside the
describe block named 'unauthenticated account page access'; move those tests out
by either creating a new describe('hashchange account page access', ...) or
renaming the parent describe so the context matches (e.g., change the parent
describe name or cut the test.each(...) block and paste it into a new describe
that sets up an authenticated context), ensuring the setup(...) call still uses
FixtureMember.free and the assertions (expect(triggerButtonFrame)...,
window.location.hash changes, and
within(popupFrame.contentDocument).queryByText(expectedText)) remain unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/portal/test/portal-links.test.js`:
- Around line 388-413: Tests using test.each(...) that assert logged-in behavior
are currently placed inside the describe block named 'unauthenticated account
page access'; move those tests out by either creating a new describe('hashchange
account page access', ...) or renaming the parent describe so the context
matches (e.g., change the parent describe name or cut the test.each(...) block
and paste it into a new describe that sets up an authenticated context),
ensuring the setup(...) call still uses FixtureMember.free and the assertions
(expect(triggerButtonFrame)..., window.location.hash changes, and
within(popupFrame.contentDocument).queryByText(expectedText)) remain unchanged.