refactor(dashboard): use getEnabledAppIds on metrics page#1394
refactor(dashboard): use getEnabledAppIds on metrics page#1394mantrakp04 merged 1 commit intodevfrom
Conversation
Replace typedEntries + filter with shared getEnabledAppIds from apps-utils for consistency with other call sites. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis change refactors the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR replaces a manual Confidence Score: 4/5Safe to merge; the refactor is consistent with the rest of the codebase, with only minor behavioral edge cases around sub-apps and alpha apps worth verifying. Only P2 findings present — subtle behavioral differences between the old inline logic and the new helper, but no definitive bugs in the changed code path. apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/metrics-page.tsx — verify sub-app and alpha-app behavior is intentional for the metrics page context. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[config.apps.installed] --> B{getEnabledAppIds}
B --> C[getAllAvailableAppIds\nObject.keys ALL_APPS\n⚠ filters alpha in prod]
C --> D{isAppEnabled per appId}
D -->|sub-app| E[check parent enabled\nin installedApps]
D -->|regular app| F[check appId.enabled\nin installedApps]
E --> G[installedApps array]
F --> G
G --> H[analyticsEnabled check\ninstalledApps.includes 'analytics']
Prompt To Fix All With AIThis is a comment left during a code review.
Path: apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/metrics-page.tsx
Line: 999-1001
Comment:
**Subtle behavioral change vs. original**
The old code iterated over `typedEntries(config.apps.installed)` — only the app IDs actually present in the installed map. `getEnabledAppIds` instead iterates over `getAllAvailableAppIds()` (i.e. all keys of `ALL_APPS`), which introduces two differences:
1. **Sub-apps are now included**: if a sub-app's parent is enabled, the sub-app ID will appear in `installedApps` even though sub-apps have no direct entry in `config.apps.installed`. Depending on how the metrics page consumes this list this may or may not matter.
2. **Alpha apps in production are silently excluded**: `getAllAvailableAppIds` filters out apps with `stage === "alpha"` in non-dev envs. Previously an alpha app that was explicitly marked `enabled: true` in the installed config would have been returned; now it won't be.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "refactor(dashboard): use getEnabledAppId..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
2 participants
Summary
Uses the shared
getEnabledAppIdshelper from@/lib/apps-utilsinstead of manually filtering installed apps withtypedEntrieson the project metrics page.Why
Keeps enabled-app logic consistent with other dashboard code paths and slightly reduces duplication.
Test plan
Made with Cursor
Summary by CodeRabbit