Repository navigation
fix(wework): remove hidden Apps page reachable via /apps - #2546
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe legacy ChangesLegacy Apps route removal
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 1
🤖 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 `@wework/src/App.tsx`:
- Line 164: Remove the obsolete `/apps` fallback from the default branch of the
navigation resolver in `navigation.ts`, and instead return a supported
destination or fail explicitly while preserving `unknown` telemetry behavior.
Add a regression test covering unknown `DesktopAppKey` values and verifying the
new default behavior; do not retain or redirect through the `AppsPage` path.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a7517268-b1ca-4134-992b-a61156ba8b08
📒 Files selected for processing (13)
wework/src/App.apps.test.tsxwework/src/App.tsxwework/src/components/topnav/ChromeTitlebar.test.tsxwework/src/components/topnav/useChromeTabs.test.tswework/src/config/apps.tswework/src/extensions/apps.tsxwework/src/features/workspace-tabs/WorkspaceTabStrip.test.tsxwework/src/features/workspace-tabs/WorkspaceTabsContext.test.tsxwework/src/features/workspace-tabs/workspaceTabs.test.tswework/src/features/workspace-tabs/workspaceTabs.tswework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/pages/AppsPage.tsx
💤 Files with no reviewable changes (12)
- wework/src/i18n/locales/zh-CN/common.json
- wework/src/components/topnav/ChromeTitlebar.test.tsx
- wework/src/extensions/apps.tsx
- wework/src/features/workspace-tabs/WorkspaceTabsContext.test.tsx
- wework/src/pages/AppsPage.tsx
- wework/src/App.apps.test.tsx
- wework/src/i18n/locales/en/common.json
- wework/src/features/workspace-tabs/workspaceTabs.ts
- wework/src/features/workspace-tabs/WorkspaceTabStrip.test.tsx
- wework/src/components/topnav/useChromeTabs.test.ts
- wework/src/config/apps.ts
- wework/src/features/workspace-tabs/workspaceTabs.test.ts
| if (path === '/sites') return 'sites' as const | ||
| if (path === '/automations') return 'automations' as const | ||
| if (path === '/apps' || path.startsWith('/app/')) return 'apps' as const | ||
| if (path.startsWith('/app/')) return 'apps' as const |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Remove the remaining /apps navigation fallback.
wework/src/lib/navigation.ts:26-37 still returns /apps for unknown DesktopAppKey values. After this change, that path has no AppsPage handler, and telemetry records it as unknown. Update the resolver to use a supported destination or fail explicitly. Add a regression test for the default branch.
As per coding guidelines: fix the primary flow instead of retaining an obsolete fallback path.
Also applies to: 199-199
🤖 Prompt for 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.
In `@wework/src/App.tsx` at line 164, Remove the obsolete `/apps` fallback from
the default branch of the navigation resolver in `navigation.ts`, and instead
return a supported destination or fail explicitly while preserving `unknown`
telemetry behavior. Add a regression test covering unknown `DesktopAppKey`
values and verifying the new default behavior; do not retain or redirect through
the `AppsPage` path.
Source: Coding guidelines
The Apps page (AppsPage) was intended to be hidden but could still be opened through the /apps route, which surfaced a clickable "应用" workspace tab. Delete the page, its route, the hidden tab entry, the workspace-tab labels, extensions, and related tests so /apps no longer opens it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
6d968b4 to
78b131a
Compare
resolveDesktopAppRoute still returned the removed /apps route as its default. Fall back to the workbench instead and cover unknown app keys with a regression test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
AppsPageat/apps), which was meant to be hidden but could still be opened directly via the/appsroute, surfacing a clickable "应用" workspace tab in the top tab strip./appsroute and telemetry branch, the hiddenappstab inAPP_TABS, the workspace-tab auxiliary label, the@extensions/appsextension, and the related tests/i18n keys.Test plan
pnpm --filter wework testpasses (3229 tests)pnpm --filter wework exec prettier --checkpasses on changed filespnpm --filter wework exec eslintpasses on changed filestsc -btype check passes/appsno longer opens the "应用" page in a real Tauri session🤖 Generated with Claude Code
Summary by CodeRabbit
Changes
/appsroute from navigation./app/*routes remain available.Tests