Space Creation - Remove Plans Table + remove the query on init load #7624
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis pull request adds new localization keys to the translation file, including a support email and a space welcome dialog. The welcome dialog is integrated within the space dashboard and managed via cache utilities. The create space dialog now features updated state logic with a loading button and invokes caching upon successful creation. New GraphQL queries and custom hooks for determining plan availability and managing space plans have been introduced, while deprecated hooks and unused imports are removed. Additionally, the virtual contributor wizard’s logic is simplified by removing plan-related checks. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant SD as SpaceDashboardView
participant WS as SpaceWelcomeDialog
participant LS as LocalStorage (Cache Utils)
U->>SD: Navigate to space dashboard
SD->>LS: getSpaceWelcomeCache()
LS-->>SD: Return cached space id (if any)
SD->>SD: Validate spaceId and cache\nset openWelcome = true
SD->>WS: Render welcome dialog
U->>WS: Click "Get Started"
WS->>SD: Trigger onCloseWelcome()
SD->>LS: removeSpaceWelcomeCache()
SD->>SD: Hide welcome dialog
sequenceDiagram
participant U as User
participant CSD as CreateSpaceDialog
participant CA as Cache Utils
U->>CSD: Fill in details & submit form
CSD->>CSD: Validate form & set creatingLoading
CSD->>CA: addSpaceWelcomeCache(spaceId)
CSD-->>U: Confirm space creation and close dialog
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
src/domain/journey/space/createSpace/utils.ts (2)
1-1: Consider using a more specific cache key.The current key
'SP_WELCOME'could be more descriptive. Consider using a more specific key like'SPACE_WELCOME_DIALOG'to better indicate its purpose.-const SPACE_CREATION_CACHE_KEY = 'SP_WELCOME'; +const SPACE_CREATION_CACHE_KEY = 'SPACE_WELCOME_DIALOG';
3-9: Add error handling for localStorage operations.localStorage operations can fail in various scenarios (e.g., private browsing mode, storage quota exceeded). Consider adding try-catch blocks to handle these cases gracefully.
export const addSpaceWelcomeCache = (id: string) => { - localStorage.setItem(SPACE_CREATION_CACHE_KEY, id); + try { + localStorage.setItem(SPACE_CREATION_CACHE_KEY, id); + } catch (error) { + console.warn('Failed to store space welcome cache:', error); + } }; export const getSpaceWelcomeCache = () => { - return localStorage.getItem(SPACE_CREATION_CACHE_KEY); + try { + return localStorage.getItem(SPACE_CREATION_CACHE_KEY); + } catch (error) { + console.warn('Failed to retrieve space welcome cache:', error); + return null; + } };src/domain/journey/space/SpaceDashboard/SpaceWelcomeDialog.tsx (2)
7-9: Consider adding more descriptive props interface.The
onCloseprop could benefit from a more descriptive name and JSDoc comment to better indicate its purpose.interface SpaceWelcomeDialogProps { + /** Callback fired when the dialog is closed by the user */ - onClose: () => void; + onDialogClose: () => void; }
15-34: Enhance dialog accessibility.The dialog could benefit from improved accessibility attributes.
- <DialogWithGrid open onClose={onClose} columns={6}> + <DialogWithGrid + open + onClose={onClose} + columns={6} + aria-labelledby="space-welcome-dialog-title" + aria-describedby="space-welcome-dialog-description" + > - <DialogHeader title={t('components.spaceWelcomeDialog.title')} onClose={onClose} /> + <DialogHeader + title={t('components.spaceWelcomeDialog.title')} + onClose={onClose} + id="space-welcome-dialog-title" + /> <DialogContent> - <Caption alignSelf="center"> + <Caption alignSelf="center" id="space-welcome-dialog-description">src/domain/journey/space/createSpace/useSpacePlans.ts (2)
24-27: Add error handling for GraphQL queries.The hook should handle potential GraphQL errors gracefully.
- const { data, loading: plansLoading } = usePlansTableQuery({ skip }); + const { data, loading: plansLoading, error: plansError } = usePlansTableQuery({ skip }); - const { isPlanAvailable, loading } = usePlanAvailability({ skip, accountId }); + const { isPlanAvailable, loading, error: availabilityError } = usePlanAvailability({ skip, accountId }); + if (plansError || availabilityError) { + console.error('Failed to load plans:', plansError || availabilityError); + }
28-41: Optimize performance with useMemo.The filtering and sorting logic is complex and could benefit from memoization to prevent unnecessary recalculations.
+ const filteredPlans = useMemo( + () => + data?.platform.licensingFramework.plans + .filter(plan => plan.type === LicensingCredentialBasedPlanType.SpacePlan) + .filter(plan => plan.enabled) ?? [], + [data] + ); const availablePlans = useMemo( () => - ( - data?.platform.licensingFramework.plans - .filter(plan => plan.type === LicensingCredentialBasedPlanType.SpacePlan) - .filter(plan => plan.enabled) + (filteredPlans .filter(plan => isPlanAvailable(plan)) .sort((a, b) => a.sortOrder - b.sortOrder) ?? [] ).map(plan => ({ id: plan.id, name: plan.name, })), - [data, isPlanAvailable] + [filteredPlans, isPlanAvailable] );src/domain/journey/space/createSpace/usePlanAvailability.ts (2)
26-26: Implement accountId feature.The TODO comment indicates missing functionality for account-level privileges.
Would you like me to help implement the account privileges feature? I can generate the necessary GraphQL query and hook implementation.
30-54: Simplify plan availability checks using a lookup map.The current implementation uses multiple if-else statements. Consider using a lookup map for better maintainability.
+ const PLAN_REQUIREMENTS = { + [LicensingCredentialBasedCredentialType.SpaceLicenseFree]: LicenseEntitlementType.AccountSpaceFree, + [LicensingCredentialBasedCredentialType.SpaceLicensePlus]: LicenseEntitlementType.AccountSpacePlus, + [LicensingCredentialBasedCredentialType.SpaceLicensePremium]: LicenseEntitlementType.AccountSpacePremium, + }; const isPlanAvailable = (plan: { name: string }) => { if (loadingUser || loadingMeData || !currentUser?.user?.id) { return false; } - //TODO: Add or remove explicit checks if needed - if (plan.name === LicensingCredentialBasedCredentialType.SpaceLicenseFree) { - return ( - myPrivileges.includes(AuthorizationPrivilege.CreateSpace) && - entitlements.includes(LicenseEntitlementType.AccountSpaceFree) - ); - } else if (plan.name === LicensingCredentialBasedCredentialType.SpaceLicensePlus) { - return ( - myPrivileges.includes(AuthorizationPrivilege.CreateSpace) && - entitlements.includes(LicenseEntitlementType.AccountSpacePlus) - ); - } else if (plan.name === LicensingCredentialBasedCredentialType.SpaceLicensePremium) { - return ( - myPrivileges.includes(AuthorizationPrivilege.CreateSpace) && - entitlements.includes(LicenseEntitlementType.AccountSpacePremium) - ); - } else { + const requiredEntitlement = PLAN_REQUIREMENTS[plan.name]; + if (!requiredEntitlement) { return false; // switch from available to unavailable when we stopped using PlansTable } + return ( + myPrivileges.includes(AuthorizationPrivilege.CreateSpace) && + entitlements.includes(requiredEntitlement) + ); };src/domain/journey/space/createSpace/plansTable/PlansTableDialog.tsx (1)
33-35: Consider removing this deprecated component.Since this component is marked as deprecated and not used, consider removing it to reduce technical debt. If you need to reference this implementation in the future, you can always find it in the git history.
src/domain/journey/space/createSpace/CreateSpaceDialog.tsx (1)
81-84: Add confirmation dialog for canceling during creation.The TODO comment indicates a missing confirmation dialog when attempting to close during creation.
Would you like me to generate the confirmation dialog implementation to prevent accidental cancellation during space creation?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/core/apollo/generated/apollo-hooks.tsis excluded by!**/generated/**src/core/apollo/generated/graphql-schema.tsis excluded by!**/generated/**
📒 Files selected for processing (12)
src/core/i18n/en/translation.en.json(2 hunks)src/domain/journey/space/SpaceDashboard/SpaceDashboardView.tsx(5 hunks)src/domain/journey/space/SpaceDashboard/SpaceWelcomeDialog.tsx(1 hunks)src/domain/journey/space/createSpace/CreateSpaceDialog.tsx(7 hunks)src/domain/journey/space/createSpace/PlansTable.graphql(1 hunks)src/domain/journey/space/createSpace/plansTable/PlansTableDialog.tsx(2 hunks)src/domain/journey/space/createSpace/plansTable/usePlanAvailability.ts(0 hunks)src/domain/journey/space/createSpace/usePlanAvailability.ts(1 hunks)src/domain/journey/space/createSpace/useSpacePlans.ts(1 hunks)src/domain/journey/space/createSpace/utils.ts(1 hunks)src/main/topLevelPages/myDashboard/newVirtualContributorWizard/ChooseCommunity.tsx(1 hunks)src/main/topLevelPages/myDashboard/newVirtualContributorWizard/useVirtualContributorWizard.tsx(0 hunks)
💤 Files with no reviewable changes (2)
- src/domain/journey/space/createSpace/plansTable/usePlanAvailability.ts
- src/main/topLevelPages/myDashboard/newVirtualContributorWizard/useVirtualContributorWizard.tsx
🧰 Additional context used
📓 Path-based instructions (3)
`src/**/*.{graphql,gql}`: Review the GraphQL schema and quer...
src/**/*.{graphql,gql}: Review the GraphQL schema and queries for best practices, potential bugs, and adherence to the project's GraphQL standards.
Ensure that the schema is well-defined and queries are optimized.
Check for security vulnerabilities.
src/domain/journey/space/createSpace/PlansTable.graphql
`src/**/*.{ts,tsx,js}`: Review the React.js/TypeScript/JavaS...
src/**/*.{ts,tsx,js}: Review the React.js/TypeScript/JavaScript code for best practices and potential bugs.
Ensure that the code adheres to TypeScript's typing system and modern standards.
Use the following code guide documentation in the codebase: 'docs/code-guidelines.md'.
Ensure sufficient error handling and logging is present, but note:
- We have a global error handler in place. So local
try/catchblocks are not required unless they provide value beyond what the global error handler already covers.- Avoid flagging the absence of local error handling in cases where the global error handler suffices.
- Avoid flagging the absence of logging in cases where the global logger is already in place (graphql usage).
Inform about the recommendation of not using non-null assertion,
except on GraphQL queries where the variable is asserted non-null and followed by a
skip:with the same variable.
Examples:
- Not Required: there's a skip
{
variables: { templateId: templateId! },
skip: !templateId,
}Check for common security vulnerabilities such as:
- SQL Injection
- XSS (Cross-Site Scripting)
- CSRF (Cross-Site Request Forgery)
- Insecure dependencies
- Sensitive data exposure
src/main/topLevelPages/myDashboard/newVirtualContributorWizard/ChooseCommunity.tsxsrc/domain/journey/space/createSpace/plansTable/PlansTableDialog.tsxsrc/domain/journey/space/createSpace/useSpacePlans.tssrc/domain/journey/space/SpaceDashboard/SpaceWelcomeDialog.tsxsrc/domain/journey/space/SpaceDashboard/SpaceDashboardView.tsxsrc/domain/journey/space/createSpace/utils.tssrc/domain/journey/space/createSpace/CreateSpaceDialog.tsxsrc/domain/journey/space/createSpace/usePlanAvailability.ts
`src/**/*.json`: Review the JSON files for correct syntax an...
src/**/*.json: Review the JSON files for correct syntax and structure.
Ensure that the configuration and data are accurate and follow the project's standards.
Check for sensitive data exposure and ensure that the data is properly validated.
src/core/i18n/en/translation.en.json
🔇 Additional comments (12)
src/main/topLevelPages/myDashboard/newVirtualContributorWizard/ChooseCommunity.tsx (1)
107-107: LGTM! The button disable condition is now more logical.The updated condition correctly enforces space selection when spaces are available, improving the user experience by providing clearer feedback.
src/domain/journey/space/createSpace/plansTable/PlansTableDialog.tsx (1)
16-16: LGTM! Import path updated correctly.The import path has been updated to use the parent directory path.
src/domain/journey/space/SpaceDashboard/SpaceDashboardView.tsx (5)
39-40: LGTM! Required imports added.The imports for cache utilities and SpaceWelcomeDialog component are correctly added.
93-93: LGTM! State management for welcome dialog.The state is correctly initialized for managing the welcome dialog visibility.
111-114: LGTM! Welcome dialog close handler.The close handler correctly updates the state and cleans up the cache.
128-135: LGTM! Welcome dialog initialization logic.The useEffect hook correctly checks for cached welcome state when spaceId changes.
212-212: LGTM! Welcome dialog rendering.The SpaceWelcomeDialog is correctly rendered based on spaceId and openWelcome state.
src/domain/journey/space/createSpace/CreateSpaceDialog.tsx (2)
64-68: LGTM! Improved state management.The state management has been improved with separate states for loading, terms acceptance, and tutorial callouts.
267-275: LGTM! Enhanced loading state feedback.The LoadingButton component provides better visual feedback during space creation.
src/domain/journey/space/createSpace/PlansTable.graphql (1)
33-35: Review: Validate New License Field InclusionThe new
licensefield withavailableEntitlementshas been added inside theaccountobject of themequery. Please confirm that the backend schema supports this field and that the associated resolver handles possible null or missing values gracefully.src/core/i18n/en/translation.en.json (2)
496-497: Review: New Support Email Localization KeyThe
"supportEmail": "support@alkem.io"key has been introduced to provide a consistent support contact across the application. Verify that its placement within the JSON structure aligns with the overall localization strategy and that all UI components referencing this key correctly handle the string.
1690-1693: Review: New Space Welcome Dialog LocalizationThe addition of the
"spaceWelcomeDialog"section—with its title, detailed HTML-formatted description, and button text—enhances the onboarding and user experience during space creation. Please ensure that the HTML tags (e.g.,<contact>) are parsed correctly in the target UI components and that this section is consistently integrated with the other translation keys.
On space change (if cache available) a new SpaceWelcomeDialog is shown;
A few TODOs were left:
Summary by CodeRabbit
New Features
Improvements