Enhance Request Feature #3142
Enhance Request Feature #3142
Conversation
WalkthroughThis update introduces a dynamic, feature-flag-driven system for requesting and approving access to workspace features. It generalizes the previous education-specific access request flow to support multiple features, adds new database schema elements, updates backend APIs, refactors UI components for flexibility, and implements internationalization throughout the feature request and approval workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WorkspaceSettingsPage
participant FeatureFlagsUtil
participant RequestFeatureAccessDialog
participant API
participant ApprovalsTable
User->>WorkspaceSettingsPage: Visits settings page
WorkspaceSettingsPage->>FeatureFlagsUtil: getFeatureFlags(wsId)
FeatureFlagsUtil-->>WorkspaceSettingsPage: Returns enabled features
WorkspaceSettingsPage->>RequestFeatureAccessDialog: Passes enabled features
User->>RequestFeatureAccessDialog: Opens dialog, selects feature, submits request
RequestFeatureAccessDialog->>API: POST /feature-request (with feature)
API-->>RequestFeatureAccessDialog: Responds with request status
User->>ApprovalsTable: Views approval requests
ApprovalsTable->>API: GET /feature-requests?feature=...
API-->>ApprovalsTable: Returns filtered requests
ApprovalsTable->>User: Displays requests and stats
User->>ApprovalsTable: Approves/rejects request
ApprovalsTable->>API: PATCH /feature-requests/[id] (approve/reject)
API-->>ApprovalsTable: Updates status
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/types/src/supabase.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs ✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @VNOsST, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances our feature request mechanism by generalizing the existing 'education access request' system. I've updated the database schema, extended the relevant API endpoint, and refined the UI to support a broader range of feature access requests (AI, Education, Challenges, Quizzes). This change provides a more flexible and user-friendly way for workspace owners to request access to new functionalities.
Highlights
- Database Schema Evolution: I've introduced a new
FEATURE_FLAGenum in the database, allowing theworkspace_education_access_requeststable to be generalized. This table now includes afeaturecolumn, enabling it to track access requests for various features like AI, Education, Challenges, and Quizzes, rather than being limited to just 'Education'. The unique index on this table was also updated to reflect this change. - API Generalization: The
/api/v1/workspaces/[wsId]/education-access-requestendpoint has been repurposed and enhanced. It now handles requests for any feature defined by the newFEATURE_FLAGenum, including feature-specific validation and error messages. Additionally, the GET endpoint for this route now retrieves all pending/approved feature requests for a workspace, not just the latest education-related one. - Dynamic UI for Feature Requests: The
RequestFeatureAccessDialogcomponent in the UI has been updated to dynamically display available features. It now fetches the workspace's currently enabled features and filters the options presented to the user, preventing requests for features that are already active or have a pending/approved request. This improves the user experience by providing more relevant choices.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request successfully enhances the feature request functionality by allowing users to request access to multiple features (AI, Challenges, Quizzes) through a single system. The changes involve updating the database schema, modifying the API route to handle generic feature requests, and updating the frontend dialog component to reflect available features and submit requests for specific ones.
The core logic for handling different features in the API and filtering available features in the UI based on existing requests and enabled flags appears correct. The database migration also correctly adds the feature column and updates the unique index.
The primary area for improvement is the naming consistency. The API route file and endpoint URL still refer to "education-access-request" despite handling requests for other features. Renaming these to something more generic like "feature-access-request" would improve code clarity and maintainability.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Honestly not happy with the code written, just gonna see what coderabbit has to say, might do like a full rework of this code, just really messy |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/upskii/src/components/request-feature-access-dialog.tsx (1)
210-211: Update endpoint URL to reflect generic feature requests.The endpoint path still contains "education-access-request" but now handles multiple feature types, which is misleading.
🧹 Nitpick comments (6)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx (1)
42-44: Remove debug logging before merging.Debug console.log statements should be removed from production code.
- // Debug logging - console.log('Server - Feature flags for wsId:', wsId, featureFlags); -apps/db/supabase/migrations/20250620133223_new_migration.sql (1)
1-8: Well-designed migration with proper backward compatibility.The migration correctly:
- Introduces the FEATURE_FLAG enum for type safety
- Adds the feature column with a sensible default
- Updates the unique constraint to prevent duplicate pending requests per feature
Consider renaming the table
workspace_education_access_requestsin a future migration to reflect its generic nature (e.g.,workspace_feature_access_requests).apps/upskii/src/components/request-feature-access-dialog.tsx (1)
143-150: Remove debug logging statements.Debug console.log statements should be removed before merging to production.
- // Debug logging - console.log('Debug - enabledFeatures:', enabledFeatures); - console.log('Debug - enabledFeaturesSet:', enabledFeaturesSet); - console.log( - 'Debug - requestedOrApprovedFeatures:', - requestedOrApprovedFeatures - ); - console.log('Debug - all feature keys:', Object.keys(featureConfig)); - const available = Object.keys(featureConfig).filter( (f) => !requestedOrApprovedFeatures.has(f) && !enabledFeaturesSet.has(f) ) as FeatureKey[]; - - console.log('Debug - availableFeatures:', available); - return available;Also applies to: 156-157
apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts (2)
1-220: Consider renaming the route file to match its generic functionality.The file is still named
education-access-request/route.tsbut now handles multiple feature types. Consider renaming tofeature-access-request/route.tsor similar to better reflect its purpose.Would you like me to help generate a migration plan to rename this endpoint across the codebase?
32-219: Add test coverage for the feature access request API.The static analysis indicates no test coverage for this critical API endpoint. Consider adding tests for:
- Valid feature requests for each feature type
- Invalid feature type validation
- Permission checks (non-owner attempts)
- Duplicate request prevention
- Already enabled feature handling
Would you like me to generate comprehensive test cases for this API endpoint?
packages/types/src/supabase.ts (1)
5555-5649: Consider table naming for future clarity.While the implementation is correct, the table name
workspace_education_access_requestsnow handles multiple feature types beyond just education. Consider whether this might cause confusion for future developers, though renaming could break existing functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/db/supabase/migrations/20250620133223_new_migration.sql(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx(3 hunks)apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts(6 hunks)apps/upskii/src/components/request-feature-access-dialog.tsx(5 hunks)packages/types/src/supabase.ts(21 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts (1)
packages/utils/src/feature-flags/data.ts (1)
FEATURE_FLAGS(1-16)
🪛 GitHub Check: codecov/patch
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx
[warning] 11-11: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx#L11
Added line #L11 was not covered by tests
[warning] 39-44: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx#L39-L44
Added lines #L39 - L44 were not covered by tests
[warning] 115-124: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx#L115-L124
Added lines #L115 - L124 were not covered by tests
apps/upskii/src/components/request-feature-access-dialog.tsx
[warning] 108-113: apps/upskii/src/components/request-feature-access-dialog.tsx#L108-L113
Added lines #L108 - L113 were not covered by tests
[warning] 136-141: apps/upskii/src/components/request-feature-access-dialog.tsx#L136-L141
Added lines #L136 - L141 were not covered by tests
[warning] 144-150: apps/upskii/src/components/request-feature-access-dialog.tsx#L144-L150
Added lines #L144 - L150 were not covered by tests
[warning] 152-153: apps/upskii/src/components/request-feature-access-dialog.tsx#L152-L153
Added lines #L152 - L153 were not covered by tests
[warning] 156-156: apps/upskii/src/components/request-feature-access-dialog.tsx#L156
Added line #L156 was not covered by tests
apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts
[warning] 2-2: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L2
Added line #L2 was not covered by tests
[warning] 11-31: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L11-L31
Added lines #L11 - L31 were not covered by tests
[warning] 49-49: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L49
Added line #L49 was not covered by tests
[warning] 52-52: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L52
Added line #L52 was not covered by tests
[warning] 54-54: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L54
Added line #L54 was not covered by tests
[warning] 59-68: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L59-L68
Added lines #L59 - L68 were not covered by tests
[warning] 86-88: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L86-L88
Added lines #L86 - L88 were not covered by tests
[warning] 93-94: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L93-L94
Added lines #L93 - L94 were not covered by tests
[warning] 98-98: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L98
Added line #L98 was not covered by tests
[warning] 101-101: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L101
Added line #L101 was not covered by tests
[warning] 103-105: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L103-L105
Added lines #L103 - L105 were not covered by tests
[warning] 110-110: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L110
Added line #L110 was not covered by tests
[warning] 115-115: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L115
Added line #L115 was not covered by tests
[warning] 122-122: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L122
Added line #L122 was not covered by tests
[warning] 128-128: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L128
Added line #L128 was not covered by tests
[warning] 136-136: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L136
Added line #L136 was not covered by tests
[warning] 145-145: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L145
Added line #L145 was not covered by tests
[warning] 152-152: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L152
Added line #L152 was not covered by tests
[warning] 196-197: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L196-L197
Added lines #L196 - L197 were not covered by tests
[warning] 201-201: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L201
Added line #L201 was not covered by tests
[warning] 206-206: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L206
Added line #L206 was not covered by tests
[warning] 211-211: apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts#L211
Added line #L211 was not covered by tests
🔇 Additional comments (7)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx (1)
115-124: Well-structured feature flag integration.The feature flags are properly fetched and passed to the dialog component with the correct structure.
apps/upskii/src/components/request-feature-access-dialog.tsx (1)
128-159: Well-implemented feature filtering logic.The logic correctly filters out:
- Features with pending or approved requests
- Features that are already enabled in the workspace
This prevents duplicate requests and provides a good user experience.
apps/upskii/src/app/api/v1/workspaces/[wsId]/education-access-request/route.ts (2)
11-31: Excellent feature mapping structure.The
FEATURE_MAPprovides:
- Type-safe feature validation
- Clear mapping between feature keys and their flags
- Human-readable display names for error messages
This is a well-designed approach for managing feature types.
93-127: Robust validation prevents duplicate feature requests.The implementation correctly:
- Checks if the requested feature is already enabled
- Prevents duplicate pending requests for the same feature
- Provides clear, feature-specific error messages
This ensures data integrity and good user experience.
packages/types/src/supabase.ts (3)
8270-8274: LGTM! Well-structured feature flag enum.The
FEATURE_FLAGenum is properly defined with descriptive values that clearly indicate the different features that can be enabled. The naming convention follows database enum standards.
5560-5560: Good integration of feature field into table schema.The
featurefield is properly integrated into theworkspace_education_access_requeststable:
- Required in
Rowtype (reflecting the database constraint)- Optional in
Inserttype (likely has a default value)- Optional in
Updatetype (standard for partial updates)The typing correctly references the
FEATURE_FLAGenum, ensuring type safety.Also applies to: 5574-5574, 5588-5588
8430-8435: Constants properly mirror the enum definition.The constants array correctly includes all four feature flag values and maintains consistency with the enum definition above.
|
damn... Yeah, gonna reanalyze this PR tmrw. |
- Change API routes name to be more generic - Change API behaviour to accept more features instead of default education - Enhance table to show feature requested - Refactor feature request dialog
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
apps/upskii/src/app/api/v1/admin/feature-requests/[requestId]/route.ts (1)
74-74: Update error message to be feature-agnostic.The error message still references "Education access request not found" but this route now handles all feature types. This could be confusing for users requesting non-education features.
- { error: 'Education access request not found' }, + { error: 'Feature access request not found' },
♻️ Duplicate comments (1)
apps/upskii/src/components/request-feature-access-dialog.tsx (1)
113-113: Consider pluralizing the endpoint for consistency.The endpoint
/feature-requestshould be/feature-requeststo follow REST conventions and maintain consistency with the admin endpoint/api/v1/admin/feature-requests.- `/api/v1/workspaces/${wsId}/feature-request` + `/api/v1/workspaces/${wsId}/feature-requests`Also applies to: 148-148
🧹 Nitpick comments (4)
apps/upskii/src/app/api/v1/admin/feature-requests/route.ts (1)
119-119: Update error message to reflect generalized feature requests.The error message still references "education access requests" but should be updated to reflect the generalized feature request system.
- { error: 'Failed to fetch education access requests' }, + { error: 'Failed to fetch feature access requests' },apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx (1)
44-54: Remove debug logging and verify feature filtering logic.The feature flag fetching and filtering logic looks correct, but the debug console.log statements should be removed from production code.
- // Debug logging - console.log('Server - Feature flags for wsId:', wsId, featureFlags); - const approvedFeatures = getRequestableFeatureKeys().filter( (key) => featureFlags[getRequestableFeature(key).flag] ); - console.log('approvedFeatures', approvedFeatures);Let me verify the feature filtering logic works as expected:
#!/bin/bash # Description: Verify the feature filtering logic and function signatures # Expected: Confirm getRequestableFeatureKeys and getRequestableFeature functions exist and work correctly # Check getRequestableFeatureKeys function ast-grep --pattern 'export function getRequestableFeatureKeys() { $$$ }' # Check getRequestableFeature function ast-grep --pattern 'export function getRequestableFeature($_) { $$$ }' # Check the requestable features structure rg -A 10 "REQUESTABLE_FEATURES.*=" packages/utils/src/feature-flags/requestable-features.tspackages/utils/src/feature-flags/requestable-features.ts (1)
8-8: Consider using a more explicit icon type definition.Using
typeof BookTextas the type base works but could be more explicit.Consider using the actual icon component type from the UI library:
- icon: typeof BookText; // Using one of the icons as the type base + icon: React.ComponentType<{ className?: string }>;Or if the UI library exports an icon type:
import type { IconComponent } from '@tuturuuu/ui/icons'; // ... icon: IconComponent;apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx (1)
122-136: Consider improving type safety in statistics calculation.The current implementation works but could benefit from better type safety and a more functional approach.
Apply this diff for improved type safety and readability:
- const getFeatureStats = () => { - const stats = { - pending: 0, - approved: 0, - rejected: 0, - total: 0, - }; - - approvals.data.forEach((request) => { - stats[request.status]++; - stats.total++; - }); - - return stats; - }; + const getFeatureStats = () => { + const stats = approvals.data.reduce( + (acc, request) => { + if (request.status in acc) { + acc[request.status]++; + acc.total++; + } + return acc; + }, + { + pending: 0, + approved: 0, + rejected: 0, + total: 0, + } + ); + + return stats; + };This ensures we only increment valid status keys and uses a more functional approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/db/supabase/migrations/20250620133223_new_migration.sql(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx(6 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx(2 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/feature-filter.tsx(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/page.tsx(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx(8 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx(3 hunks)apps/upskii/src/app/api/v1/admin/feature-requests/[requestId]/route.ts(6 hunks)apps/upskii/src/app/api/v1/admin/feature-requests/route.ts(5 hunks)apps/upskii/src/app/api/v1/workspaces/[wsId]/feature-request/route.ts(5 hunks)apps/upskii/src/components/feature-card.tsx(1 hunks)apps/upskii/src/components/request-feature-access-dialog.tsx(10 hunks)packages/types/src/supabase.ts(21 hunks)packages/utils/src/feature-flags/index.ts(1 hunks)packages/utils/src/feature-flags/requestable-features.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
apps/upskii/src/app/api/v1/admin/feature-requests/route.ts (1)
packages/utils/src/feature-flags/requestable-features.ts (2)
isRequestableFeature(37-41)REQUESTABLE_KEY_TO_FEATURE_FLAG(78-86)
apps/upskii/src/app/api/v1/workspaces/[wsId]/feature-request/route.ts (1)
packages/utils/src/feature-flags/requestable-features.ts (2)
isRequestableFeature(37-41)getRequestableFeature(44-48)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/feature-filter.tsx (2)
packages/utils/src/feature-flags/requestable-features.ts (2)
getRequestableFeatureKeys(51-53)getRequestableFeature(44-48)packages/ui/src/components/ui/select.tsx (5)
Select(170-170)SelectTrigger(178-178)SelectValue(179-179)SelectContent(171-171)SelectItem(173-173)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx (1)
packages/utils/src/feature-flags/requestable-features.ts (2)
REQUESTABLE_KEY_TO_FEATURE_FLAG(78-86)getRequestableFeature(44-48)
packages/utils/src/feature-flags/requestable-features.ts (2)
packages/utils/src/feature-flags/types.ts (1)
FeatureFlag(3-3)packages/utils/src/feature-flags/data.ts (1)
FEATURE_FLAGS(1-16)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx (2)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/status-filter.tsx (1)
StatusFilter(16-48)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/feature-filter.tsx (1)
FeatureFilter(20-59)
🪛 GitHub Check: codecov/patch
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/page.tsx
[warning] 34-35: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/page.tsx#L34-L35
Added lines #L34 - L35 were not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx
[warning] 61-61: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx#L61
Added line #L61 was not covered by tests
[warning] 78-78: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx#L78
Added line #L78 was not covered by tests
[warning] 103-103: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx#L103
Added line #L103 was not covered by tests
[warning] 120-120: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx#L120
Added line #L120 was not covered by tests
[warning] 159-159: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx#L159
Added line #L159 was not covered by tests
[warning] 179-180: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx#L179-L180
Added lines #L179 - L180 were not covered by tests
[warning] 280-280: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx#L280
Added line #L280 was not covered by tests
[warning] 283-285: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx#L283-L285
Added lines #L283 - L285 were not covered by tests
[warning] 343-343: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx#L343
Added line #L343 was not covered by tests
[warning] 346-347: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx#L346-L347
Added lines #L346 - L347 were not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx
[warning] 5-5: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx#L5
Added line #L5 was not covered by tests
[warning] 12-16: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx#L12-L16
Added lines #L12 - L16 were not covered by tests
[warning] 44-54: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx#L44-L54
Added lines #L44 - L54 were not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/feature-filter.tsx
[warning] 2-59: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/feature-filter.tsx#L2-L59
Added lines #L2 - L59 were not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx
[warning] 10-13: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L10-L13
Added lines #L10 - L13 were not covered by tests
[warning] 85-130: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L85-L130
Added lines #L85 - L130 were not covered by tests
[warning] 137-137: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L137
Added line #L137 was not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx
[warning] 4-4: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L4
Added line #L4 was not covered by tests
[warning] 55-55: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L55
Added line #L55 was not covered by tests
[warning] 60-60: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L60
Added line #L60 was not covered by tests
[warning] 76-76: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L76
Added line #L76 was not covered by tests
[warning] 79-79: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L79
Added line #L79 was not covered by tests
[warning] 98-98: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L98
Added line #L98 was not covered by tests
[warning] 112-112: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L112
Added line #L112 was not covered by tests
[warning] 119-139: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L119-L139
Added lines #L119 - L139 were not covered by tests
[warning] 141-141: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L141
Added line #L141 was not covered by tests
[warning] 171-205: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L171-L205
Added lines #L171 - L205 were not covered by tests
[warning] 208-208: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L208
Added line #L208 was not covered by tests
[warning] 210-222: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L210-L222
Added lines #L210 - L222 were not covered by tests
🔇 Additional comments (33)
packages/utils/src/feature-flags/index.ts (1)
1-8: LGTM! Clean barrel export pattern.The module organization with logical grouping and comments follows best practices for a centralized feature flags entry point.
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/page.tsx (1)
34-35: LGTM! Text updates align with multi-feature system.The updated pluralTitle and description appropriately reflect the expanded scope from education-only to multiple feature types (AI, Education, Quizzes, Challenges).
Note: Static analysis indicates these lines lack test coverage, which may be acceptable for UI text changes.
apps/upskii/src/app/api/v1/admin/feature-requests/route.ts (5)
6-9: LGTM! Proper imports for feature flag utilities.The imports provide the necessary validation and mapping functions for the enhanced feature filtering functionality.
54-54: LGTM! Feature parameter extraction follows established pattern.The feature query parameter extraction is consistent with other query parameters in the same function.
68-68: LGTM! Added feature field to database query.Including the feature field in the select query is necessary for the filtering and response transformation logic.
135-135: LGTM! Dynamic feature field mapping.The updated mapping uses the actual feature value from the database instead of a hardcoded string, which aligns with the multi-feature system.
94-101: ```bash
#!/bin/bashDisplay FEATURE_FLAGS definitions to confirm enum value shapes
sed -n '1,200p' packages/utils/src/feature-flags/data.ts
</details> <details> <summary>apps/upskii/src/components/feature-card.tsx (1)</summary> `1-17`: **LGTM! Well-designed presentational component.** The `FeatureCard` component follows React best practices with: - Clean TypeScript interface with proper `LucideIcon` typing - Appropriate use of `'use client'` directive - Semantic HTML structure with accessible styling - Nice hover effects for user interaction The component is focused, reusable, and integrates well with the feature flag system. </details> <details> <summary>apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx (4)</summary> `5-5`: **LGTM! Import for new FeatureCard component.** The import is properly added to support the new feature display functionality. --- `12-16`: **LGTM! Feature flag utility imports.** The imports provide the necessary functions for dynamic feature handling and filtering. --- `119-149`: **LGTM! Well-structured feature display section.** The new feature display section properly: - Uses conditional rendering for workspace owners only - Implements a responsive grid layout for feature cards - Integrates the updated `RequestFeatureAccessDialog` with feature flags - Maps through approved features to render `FeatureCard` components The implementation follows established UI patterns and provides a clean overview of enabled features. --- `125-129`: **LGTM! Updated dialog integration with feature flags.** The `RequestFeatureAccessDialog` correctly receives the `enabledFeatures` prop, allowing it to dynamically handle multiple feature types based on the workspace's current feature flags. </details> <details> <summary>apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx (2)</summary> `61-61`: **LGTM: API endpoint migration to generalized feature requests.** The API endpoints have been successfully updated from education-specific `/api/v1/admin/education-access-requests/` to the generalized `/api/v1/admin/feature-requests/` pattern, maintaining consistent functionality while supporting multiple feature types. Also applies to: 103-103 --- `78-78`: **LGTM: Dynamic feature reference implementation.** The UI text has been successfully refactored to use dynamic `approval.feature_requested` references instead of hardcoded "education access" strings. This enables the component to handle multiple feature types (AI, Education, Quizzes, Challenges) seamlessly while maintaining clear user communication. Also applies to: 120-120, 159-159, 280-280, 283-285, 343-343, 346-347 </details> <details> <summary>apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/feature-filter.tsx (2)</summary> `24-37`: **LGTM: Proper URL parameter management with pagination reset.** The component correctly handles URL parameter updates, including the important detail of resetting the page parameter when filtering changes. This ensures users start from the first page when applying a new filter, which is the expected UX behavior. --- `39-55`: **LGTM: Clean integration with feature flag utilities.** The component properly integrates with the requestable feature utilities, dynamically populating the dropdown with available features and their display names. The mapping from feature keys to user-friendly names enhances the user experience. </details> <details> <summary>apps/db/supabase/migrations/20250620133223_new_migration.sql (3)</summary> `1-1`: **LGTM: Well-structured feature flag enum.** The FEATURE_FLAG enum covers the expected feature types (AI, Education, Challenges, Quizzes) and provides a solid foundation for the generalized feature request system. --- `5-5`: **LGTM: Safe column addition with backward compatibility.** The new `feature` column is added with a NOT NULL constraint and a sensible default value of 'ENABLE_EDUCATION', ensuring existing data remains valid while supporting the new feature flag system. --- `7-7`: **LGTM: Updated unique constraint supports multiple feature requests.** The recreated unique index now properly includes the `feature` column, allowing workspaces to have one pending request per feature type rather than just one pending request total. This is the correct behavior for the generalized system. </details> <details> <summary>apps/upskii/src/app/api/v1/admin/feature-requests/[requestId]/route.ts (2)</summary> `99-143`: **LGTM: Proper dynamic feature flag handling for approvals.** The feature flag logic correctly uses the dynamic `currentRequest.feature` value instead of a hardcoded flag. The workspace secrets management properly creates, updates, or deletes the appropriate feature flag based on the approval status. --- `145-161`: **LGTM: Consistent feature flag handling for rejections.** The rejection logic properly uses the dynamic feature flag to remove the workspace secret when a previously approved request is rejected, maintaining consistency with the approval logic. </details> <details> <summary>apps/upskii/src/app/api/v1/workspaces/[wsId]/feature-request/route.ts (4)</summary> `31-47`: **LGTM: Robust feature validation and configuration.** The validation logic properly uses the utility functions to ensure only valid requestable features are accepted, and retrieves the appropriate configuration for dynamic message generation. This provides type safety and consistent feature handling. --- `68-90`: **LGTM: Dynamic feature-aware permission and state checking.** The error messages and database queries now properly reference the specific feature being requested rather than hardcoded "education" text. The feature flag checking logic correctly uses the dynamic `featureConfig.flag` value. --- `92-130`: **LGTM: Comprehensive database integration with feature flags.** The database operations properly include the new `feature` column and use the dynamic feature flag values. The error messages appropriately reference the specific feature being requested, providing clear feedback to users. --- `178-193`: **LGTM: Generalized GET handler for all feature requests.** The GET handler has been properly updated to return all feature access requests for the workspace rather than being limited to education requests. This supports the new multi-feature system while maintaining backward compatibility. </details> <details> <summary>apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx (1)</summary> `137-141`: **Good simplification of the DOM structure.** Removing the unnecessary wrapper div improves the cleanliness of the rendered HTML. </details> <details> <summary>apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx (1)</summary> `79-79`: **API endpoint correctly updated to reflect generic feature requests.** The change from `/education-access-requests` to `/feature-requests` aligns with the generalization of the feature request system. </details> <details> <summary>apps/upskii/src/components/request-feature-access-dialog.tsx (2)</summary> `67-94`: **Well-structured filtering logic for available features.** The implementation efficiently filters out already requested/approved and enabled features using Set operations. The logic correctly handles the mapping between feature flags and requestable keys. --- `96-107`: **Excellent UX enhancement for maintaining valid feature selection.** The effect properly handles edge cases when available features change, ensuring the user always has a valid selection or clear feedback when no features are available. </details> <details> <summary>packages/types/src/supabase.ts (4)</summary> `8270-8274`: **LGTM! Well-defined feature flag enum.** The `FEATURE_FLAG` enum is clearly defined with descriptive values that align with the generalized feature request system. The naming convention is consistent and the values clearly indicate the features they control. --- `5560-5560`: **Properly structured table field addition.** The `feature` field is correctly added to the `workspace_education_access_requests` table: - Required in `Row` type (line 5560) for data integrity - Optional in `Insert` type (line 5574) allowing defaults to be applied - Optional in `Update` type (line 5588) for partial updates This follows TypeScript and Supabase best practices for table schema evolution. Also applies to: 5574-5574, 5588-5588 --- `8430-8435`: **Correctly exported enum constants.** The `FEATURE_FLAG` enum is properly added to the `Constants.public.Enums` object, making it available for application code to use the enum values at runtime. This maintains consistency with other exported enums in the file. --- `7755-7760`: ```shell #!/bin/bash # Refined search for Supabase RPC usages and direct calls without relying on file-type heuristics echo "🔍 Searching for Supabase .rpc() invocations of the modified functions..." rg "\.rpc\(['\"](count_search_users|get_challenge_stats|get_daily_income_expense)['\"]" -n . echo -e "\n🔍 Searching for any direct calls to these function names (in case they’re abstracted or imported)..." rg "\\b(count_search_users|get_challenge_stats|get_daily_income_expense)\\s*\\(" -n . echo -e "\n⬇️ If no results are returned, please manually verify that no legacy calls to these signatures exist elsewhere in the codebase."
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx
Outdated
Show resolved
Hide resolved
|
It is ready for review, just gonna add some translations. |
- Revise education banner to the new dialog - Add translations - Remove legacy dialog
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/upskii/messages/en.json(1 hunks)apps/upskii/messages/vi.json(2 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx(8 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx(10 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/feature-filter.tsx(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/page.tsx(2 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx(21 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/status-filter.tsx(2 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx(3 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/layout.tsx(3 hunks)apps/upskii/src/components/request-access-button.tsx(0 hunks)apps/upskii/src/components/request-education-banner.tsx(2 hunks)packages/utils/src/feature-flags/core.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/upskii/src/components/request-access-button.tsx
✅ Files skipped from review due to trivial changes (2)
- packages/utils/src/feature-flags/core.ts
- apps/upskii/messages/en.json
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/page.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/settings/page.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/row-actions.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/feature-filter.tsx
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/layout.tsx (2)
packages/utils/src/feature-flags/core.ts (2)
isAnyEducationFeatureEnabled(314-323)getFeatureFlags(30-152)apps/upskii/src/components/request-education-banner.tsx (1)
EducationBanner(17-66)
apps/upskii/src/components/request-education-banner.tsx (2)
packages/utils/src/feature-flags/types.ts (1)
FeatureFlag(3-3)apps/upskii/src/components/request-feature-access-dialog.tsx (1)
RequestFeatureAccessDialog(50-368)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx (2)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/status-filter.tsx (1)
StatusFilter(17-50)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/feature-filter.tsx (1)
FeatureFilter(21-61)
🪛 GitHub Check: codecov/patch
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx
[warning] 4-4: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L4
Added line #L4 was not covered by tests
[warning] 9-9: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L9
Added line #L9 was not covered by tests
[warning] 54-55: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L54-L55
Added lines #L54 - L55 were not covered by tests
[warning] 58-58: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L58
Added line #L58 was not covered by tests
[warning] 63-63: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L63
Added line #L63 was not covered by tests
[warning] 79-79: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L79
Added line #L79 was not covered by tests
[warning] 82-82: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L82
Added line #L82 was not covered by tests
[warning] 101-101: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L101
Added line #L101 was not covered by tests
[warning] 115-115: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L115
Added line #L115 was not covered by tests
[warning] 122-142: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L122-L142
Added lines #L122 - L142 were not covered by tests
[warning] 144-144: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L144
Added line #L144 was not covered by tests
[warning] 149-149: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L149
Added line #L149 was not covered by tests
[warning] 165-165: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L165
Added line #L165 was not covered by tests
[warning] 174-210: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L174-L210
Added lines #L174 - L210 were not covered by tests
[warning] 213-213: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L213
Added line #L213 was not covered by tests
[warning] 215-227: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L215-L227
Added lines #L215 - L227 were not covered by tests
[warning] 237-237: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx#L237
Added line #L237 was not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx
[warning] 10-14: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L10-L14
Added lines #L10 - L14 were not covered by tests
[warning] 41-45: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L41-L45
Added lines #L41 - L45 were not covered by tests
[warning] 61-65: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L61-L65
Added lines #L61 - L65 were not covered by tests
[warning] 94-138: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L94-L138
Added lines #L94 - L138 were not covered by tests
[warning] 142-146: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L142-L146
Added lines #L142 - L146 were not covered by tests
[warning] 149-149: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L149
Added line #L149 was not covered by tests
[warning] 159-163: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L159-L163
Added lines #L159 - L163 were not covered by tests
[warning] 184-184: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L184
Added line #L184 was not covered by tests
[warning] 188-188: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L188
Added line #L188 was not covered by tests
[warning] 198-202: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L198-L202
Added lines #L198 - L202 were not covered by tests
[warning] 218-220: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L218-L220
Added lines #L218 - L220 were not covered by tests
[warning] 230-234: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L230-L234
Added lines #L230 - L234 were not covered by tests
[warning] 246-248: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx#L246-L248
Added lines #L246 - L248 were not covered by tests
🔇 Additional comments (22)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/status-filter.tsx (2)
10-10: LGTM! Proper internationalization setup.The import and initialization of the translation hook follows the correct pattern for next-intl.
Also applies to: 18-18
40-40: LGTM! Consistent internationalization implementation.All hardcoded English strings have been properly replaced with localized translations using descriptive keys from the 'approval-data-table' namespace.
Also applies to: 43-46
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/layout.tsx (4)
32-36: LGTM! Enhanced feature flag utilities import.The expanded imports properly include the new dynamic feature flag utilities needed for the multi-feature system.
251-251: LGTM! Comprehensive feature flag fetching.The addition of
getFeatureFlags(wsId)enables the component to have full visibility into the workspace's feature configuration.
289-293: LGTM! Enhanced component integration with feature flags.The EducationBanner component now receives the
enabledFeaturesprop, enabling it to make informed decisions about feature requests. The variable rename toshouldShowBannerimproves clarity.
242-245: Let’s inspect the underlyingisAnyFeatureFlagEnabledfor any caching or batching:#!/bin/bash # Show implementation of isAnyFeatureFlagEnabled to check for memoization/caching rg -n "export async function isAnyFeatureFlagEnabled" -C10 packages/utils/src/feature-flags/core.tsapps/upskii/src/components/request-education-banner.tsx (4)
3-8: LGTM! Proper imports for enhanced functionality.The new imports correctly bring in the dialog component, UI elements, types, and internationalization support needed for the feature request system.
14-14: LGTM! Well-typed feature flag integration.The
enabledFeaturesprop is properly typed using theFeatureFlagtype and correctly integrated into the component interface and parameters.Also applies to: 21-21
23-23: LGTM! Consistent internationalization implementation.The translation hook setup and usage for localizing banner text follows the established i18n patterns.
Also applies to: 45-45, 49-49
53-62: LGTM! Enhanced UX with dialog-based feature requests.The replacement of the simple button with
RequestFeatureAccessDialogprovides a much better user experience with feature selection, status tracking, and better visual feedback. The Plus icon is semantically appropriate for requesting features.apps/upskii/messages/vi.json (5)
4131-4153: Addws-settings.feature-requestVietnamese localization
This block introduces comprehensive translations for the feature‐request dialog—titles, toasts, labels, placeholders, and validation messages. Placeholders ("{ws}") and newline escapes (\n) follow JSON conventions.
4339-4340: Ensure JSON syntax aftercertificatesblock
A comma was added after the closing brace of thecertificatesobject to allow insertion of the newapproval-data-tablesection. Verify that your JSON parser or loader accepts this trailing comma and that there are no parse errors.
4343-4401: Add Vietnamese translations forapproval-data-table
All columns, statuses, actions, and feedback messages for the approval data table have been localized. Confirm key consistency with the component and correct rendering of placeholders like{workspace},{feature}, and{date}.
4402-4405: Localizeapprovalsoverview section
Introduced translations for the approvals workflow overview and description. Ensure the multi‐line description displays correctly in the UI.
4406-4408: Insert root‐level keys for feature UI prompts
Addedrequest-feature,unlock-education-features, andtransform-your-workspaceat the root. Verify that equivalent entries exist in other locale files (e.g.,en.json) and are loaded by the application.apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx (2)
10-14: Well-structured import organization for feature flag utilities.The imports are properly organized and align with the new dynamic feature flag system architecture.
41-45: Excellent localization implementation.The consistent use of the translation function
twith proper namespace structure across all column headers provides a solid foundation for internationalization.Also applies to: 61-65, 142-146, 159-163, 198-202, 230-234
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/approvals-table.tsx (5)
4-4: Solid feature filtering integration.The feature filtering implementation follows the same pattern as the existing status filtering, maintaining consistency in the codebase. The integration with URL parameters and useEffect dependencies is properly handled.
Also applies to: 58-58, 79-79, 115-115, 215-215
82-82: API endpoint migration aligns with system architecture.The change from
/api/v1/admin/education-access-requeststo/api/v1/admin/feature-requestscorrectly reflects the migration from education-specific to multi-feature support.
124-142: Clean and efficient statistics calculation.The
getFeatureStatsfunction provides a clear and efficient way to calculate approval statistics. The implementation correctly counts by status and maintains a total count.
182-211: Excellent statistics dashboard UI implementation.The color-coded statistics cards provide excellent visual feedback:
- Yellow for pending requests
- Green for approved requests
- Red for rejected requests
- Clean responsive grid layout
This significantly improves the user experience by providing at-a-glance insights into approval status distribution.
144-144: Improved loading states and error handling with localization.The loading state management has been enhanced with proper localization support and better visual feedback. The conditional rendering for
isLoading && !isRefreshingprovides a smoother user experience during data fetching.Also applies to: 149-149, 165-165, 174-179, 221-227
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/(workspace-settings)/approvals/columns.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/db/supabase/migrations/20250621133223_add_initial_feature_flags.sql (3)
1-1: Consider idempotence in enum creation
To prevent errors if this migration runs against an existingfeature_flagtype (e.g., in test or staging), useCREATE TYPE IF NOT EXISTS.
5-5: Evaluate table rewrite impact
Adding a non-nullable column with a default can trigger a full table rewrite on Postgres <11. For very large tables, consider a phased approach: add the column nullable, backfill values, then applyNOT NULL.
7-7: Consider concurrent index creation
Creating or recreating indexes non-concurrently may block writes on busy tables. For production,CREATE INDEX CONCURRENTLY ...can reduce locking (though it must run outside a transaction).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/db/supabase/migrations/20250621133223_add_initial_feature_flags.sql(1 hunks)packages/types/src/supabase.ts(21 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/types/src/supabase.ts
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Verify generated types
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
🔇 Additional comments (1)
apps/db/supabase/migrations/20250621133223_add_initial_feature_flags.sql (1)
3-3: Dropping index before recreation is correct
Removing the old partial index ensures we can rebuild it with the newfeaturecolumn.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores