🐛(frontend) redirect home page to login when homepage feature is disabled#2521
🐛(frontend) redirect home page to login when homepage feature is disabled#2521maartendra wants to merge 2 commits into
Conversation
…bled FRONTEND_HOMEPAGE_FEATURE_ENABLED=false makes the Auth guard send anonymous visitors on / straight to login instead of /home, but the /home page itself still rendered the full homepage when reached via a direct link. Redirect it to login too, so disabling the feature really disables the page. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PhqB4SHsHCuMfprwLEN3rR Signed-off-by: Maarten Draijer <maarten@madra.nl>
3415b0d to
cc66ce1
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe Home page reads Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/frontend/apps/e2e/__tests__/app-impress/home.spec.ts`:
- Around line 182-195: Add a positive Playwright test alongside the existing
disabled-feature case in the homepage navigation suite. Enable
FRONTEND_HOMEPAGE_FEATURE_ENABLED via overrideConfig, navigate directly to
/home/, and assert homepage content such as the first header is visible rather
than the login page.
In `@src/frontend/apps/impress/src/pages/home/index.tsx`:
- Around line 35-43: Update the home page’s useConfig-derived state and the
useEffect redirect guard so gotoLogin(false) runs only after configuration has
finished loading, using isFetched or the equivalent flag. Include the same
loading guard in the conditional rendering branch so the homepage remains
available while the feature flag is unresolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b86534ce-b4fd-4ae7-85d2-02a6f44139c1
📒 Files selected for processing (3)
CHANGELOG.mdsrc/frontend/apps/e2e/__tests__/app-impress/home.spec.tssrc/frontend/apps/impress/src/pages/home/index.tsx
Guard the login redirect on the config query being fetched, so a pending feature flag is not mistaken for a disabled one, and add a positive e2e test for a direct /home visit with the flag enabled. Addresses CodeRabbit review on suitenumerique#2521. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PhqB4SHsHCuMfprwLEN3rR Signed-off-by: Maarten Draijer <maarten@madra.nl>
Purpose
FRONTEND_HOMEPAGE_FEATURE_ENABLED=falseis supposed to disable the homepage: theAuthguard sends anonymous visitors on/straight to the login page instead of/home. But the/homepage itself doesn't check the flag, so anyone following a direct link to/home/still gets the full stock homepage. Since the auth guard only forces login on/and/docs(/homeis an "allowed" path), nothing stops it from rendering.We noticed this on our own instance (we run Docs with a custom skin as part of a sovereign EU business workspace): with the flag off, every entry point correctly leads to our SSO login, except a deep link to
/home/, which still shows the default homepage.Proposal
FRONTEND_HOMEPAGE_FEATURE_ENABLEDisfalse,/homenow redirects anonymous visitors to the login page (gotoLogin(false)— no point returning to/homeafter login), mirroring theAuthcomponent's behavior for/false, a direct visit to/home/lands on the login page (sibling of the existing flag test)External contributions
General requirements
Skip the checkbox below 👇 if you're fixing an issue or adding documentation
CI requirements
git commit --signoff(DCO compliance)git commit -S)<gitmoji>(type) title description## [Unreleased]section (if noticeable change)AI requirements
🤖 Generated with Claude Code
https://claude.ai/code/session_01PhqB4SHsHCuMfprwLEN3rR