Use proper entitlements for authorizing template saving on collaboration #7216
Conversation
WalkthroughThe changes introduce a new utility function, Changes
Possibly related PRs
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: 0
🧹 Outside diff range and nitpick comments (5)
src/domain/license/plans/utils/filterAndMapEnabledEntitlements.ts (2)
3-7: Add JSDoc documentation for the Entitlement typeWhile the type definition is clear, adding JSDoc documentation would improve developer experience and code maintainability.
+/** + * Represents a license entitlement with its activation status + * @property {string} id - Unique identifier of the entitlement + * @property {LicenseEntitlementType} type - Type of the entitlement + * @property {boolean} enabled - Whether the entitlement is currently enabled + */ type Entitlement = { id: string; type: LicenseEntitlementType; enabled: boolean; };
9-13: Add input validation and improve documentationThe function implementation is clean but could benefit from additional safety checks and documentation.
+/** + * Filters out disabled entitlements and maps the enabled ones to their types + * @param {Entitlement[]} entitlements - Array of entitlements to process + * @returns {LicenseEntitlementType[]} Array of entitlement types that are enabled + */ export const filterAndMapEnabledEntitlements = (entitlements: Entitlement[] = []): LicenseEntitlementType[] => { return entitlements + .filter(Boolean) // Guard against null/undefined array elements .filter(entitlement => entitlement.enabled) // Filter enabled entitlements .map(entitlement => entitlement.type); // Map to their types };src/domain/collaboration/authorization/useCollaborationAuthorization.ts (1)
34-34: Consider clarifying the relationship between save template flagsThe code maintains two similar-looking flags:
canSaveAsTemplate: Based on entitlementsentitledToSaveAsTemplate: Based on direct entitlement checkThis could lead to confusion. Consider:
- Adding documentation to explain the difference
- Consolidating these checks if they serve the same purpose
+ // Determines if template saving is allowed based on space-level entitlements const canSaveAsTemplate = collaborationEntitlements.includes(LicenseEntitlementType.SpaceFlagSaveAsTemplate); const license = collaborationData?.lookup.collaboration?.license; + // Determines if the specific entitlement exists and is enabled, regardless of space-level settings const saveAsEntitlement = license?.entitlements.find( entitlement => entitlement.type === LicenseEntitlementType.SpaceFlagSaveAsTemplate ); const entitledToSaveAsTemplate = saveAsEntitlement?.enabled ?? false;Also applies to: 38-42
src/domain/community/user/providers/UserProvider/UserProvider.tsx (2)
16-16: Consider using path aliases to improve maintainabilityThe deep relative import path with multiple parent directory traversals (
../../../../) makes the code fragile and harder to maintain. Consider setting up TypeScript path aliases to make imports more readable and maintainable.Example configuration in
tsconfig.json:{ "compilerOptions": { "baseUrl": "src", "paths": { "@license/*": ["domain/license/*"], "@community/*": ["domain/community/*"] } } }Then the import could be simplified to:
import { filterAndMapEnabledEntitlements } from '@license/plans/utils/filterAndMapEnabledEntitlements';
Line range hint
42-93: Add comprehensive error handling and cleanupWhile the component is well-structured, consider these improvements for robustness:
- Add error handling for GraphQL queries:
const { data: meData, loading: loadingMe, error: meError } = useUserProviderQuery({ skip: !isAuthenticated, }); // Handle query errors if (meError || platformLevelAuthorizationError) { return <ErrorPage error={meError || platformLevelAuthorizationError} />; }
- Add cleanup in useEffect to prevent memory leaks:
useEffect(() => { let mounted = true; if (isAuthenticated && !loadingMe && !meData?.me?.user) { createUserProfile(); } return () => { mounted = false; }; }, [meData?.me.user, loadingMe, createUserProfile, isAuthenticated]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (2)
src/core/apollo/generated/apollo-helpers.tsis excluded by!**/generated/**src/core/apollo/generated/graphql-schema.tsis excluded by!**/generated/**
📒 Files selected for processing (3)
src/domain/collaboration/authorization/useCollaborationAuthorization.ts(2 hunks)src/domain/community/user/providers/UserProvider/UserProvider.tsx(2 hunks)src/domain/license/plans/utils/filterAndMapEnabledEntitlements.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/domain/collaboration/authorization/useCollaborationAuthorization.ts (1)
Pattern 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.
Ensure sufficient error handling and logging is present.
Check for common security vulnerabilities such as:
- SQL Injection
- XSS (Cross-Site Scripting)
- CSRF (Cross-Site Request Forgery)
- Insecure dependencies
- Sensitive data exposure
src/domain/community/user/providers/UserProvider/UserProvider.tsx (1)
Pattern 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.
Ensure sufficient error handling and logging is present.
Check for common security vulnerabilities such as:
- SQL Injection
- XSS (Cross-Site Scripting)
- CSRF (Cross-Site Request Forgery)
- Insecure dependencies
- Sensitive data exposure
src/domain/license/plans/utils/filterAndMapEnabledEntitlements.ts (1)
Pattern 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.
Ensure sufficient error handling and logging is present.
Check for common security vulnerabilities such as:
- SQL Injection
- XSS (Cross-Site Scripting)
- CSRF (Cross-Site Request Forgery)
- Insecure dependencies
- Sensitive data exposure
🔇 Additional comments (4)
src/domain/license/plans/utils/filterAndMapEnabledEntitlements.ts (1)
9-13: Verify the integration with dependent components
Let's confirm that the function is properly integrated with UserProvider and useCollaborationAuthorization.
✅ Verification successful
Function integration verified successfully
The utility function filterAndMapEnabledEntitlements is properly integrated in both identified locations:
- In
UserProvider.tsxto filter account entitlements from user data - In
useCollaborationAuthorization.tsto filter collaboration entitlements for authorization checks
The implementation and usage are consistent across both files, with proper null-safety through optional chaining.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the integration of filterAndMapEnabledEntitlements
# Check the usage in UserProvider and useCollaborationAuthorization
echo "Checking usage in dependent components..."
rg -A 5 "filterAndMapEnabledEntitlements" \
--glob "src/domain/community/user/providers/UserProvider/UserProvider.tsx" \
--glob "src/domain/collaboration/authorization/useCollaborationAuthorization.ts"
# Verify there are no other usages that might need updating
echo "Checking for other potential usages..."
rg -l "entitlements.*enabled" --type ts --type tsx
Length of output: 3174
Script:
#!/bin/bash
# Let's try a different approach to verify the integration
# Check for all usages of filterAndMapEnabledEntitlements
echo "Checking all usages of filterAndMapEnabledEntitlements..."
rg "filterAndMapEnabledEntitlements" -A 5
# Check for files containing entitlements.enabled pattern with correct file type flags
echo "Checking for entitlements.enabled pattern..."
rg -l "entitlements.*enabled" -t typescript -t tsx
# Look for any potential test files
echo "Checking for related test files..."
fd -e test.ts -e test.tsx -e spec.ts -e spec.tsx | rg -i "entitlement"
Length of output: 3838
src/domain/collaboration/authorization/useCollaborationAuthorization.ts (2)
3-3: LGTM! Clean import of the utility function
The import statement follows proper organization and naming conventions.
29-31: Verify handling of empty or undefined entitlements
The code safely handles null/undefined through optional chaining, but let's verify the behavior in edge cases.
✅ Verification successful
Edge cases for entitlements are properly handled
The implementation of filterAndMapEnabledEntitlements found in src/domain/license/plans/utils/filterAndMapEnabledEntitlements.ts shows that it safely handles undefined and empty cases by:
- Using a default empty array parameter:
entitlements: Entitlement[] = [] - Applying filter and map operations which work safely on empty arrays
The code is well-designed to handle all edge cases:
- Undefined input is handled through optional chaining in the call
- Empty or undefined entitlements result in an empty array
- The function is consistently used across the codebase (e.g., in UserProvider)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for test cases covering empty/undefined entitlements
rg -l "SpaceFlagSaveAsTemplate.*undefined|SpaceFlagSaveAsTemplate.*\[\]" --type ts
# Search for error handling patterns in similar hooks
ast-grep --pattern 'const $_ = use$_Query({
$$$
skip: $_,
});'
Length of output: 153
Script:
#!/bin/bash
# Let's search for the filterAndMapEnabledEntitlements function implementation
rg -A 10 "filterAndMapEnabledEntitlements" --type ts
# Search for test files related to collaboration authorization
fd "collaboration.*test" -e ts
# Search for any usage of entitlements in test files
rg "entitlements.*undefined|entitlements.*\[\]" --type test
Length of output: 5791
src/domain/community/user/providers/UserProvider/UserProvider.tsx (1)
86-86: Verify edge case handling in filterAndMapEnabledEntitlements
The function is correctly used with optional chaining, but we should verify its behavior with null/undefined values to ensure robust error handling.
✅ Verification successful
Let me try a different search strategy to locate the function implementation.
Edge case handling in filterAndMapEnabledEntitlements is properly implemented
The function implementation has been found and it safely handles null/undefined values:
- It uses a default empty array parameter:
entitlements: Entitlement[] = [] - The chaining of filter and map operations is safe with empty arrays
- The usage in UserProvider is also properly guarded with optional chaining
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the implementation of filterAndMapEnabledEntitlements for null handling
# Search for the function implementation and its tests
echo "Searching for function implementation:"
ast-grep --pattern 'export function filterAndMapEnabledEntitlements' -A 10
echo "\nSearching for related test files:"
fd "filterAndMapEnabledEntitlements" -e test.ts -e test.tsx -e spec.ts -e spec.tsx
Length of output: 327
Script:
#!/bin/bash
# Search for the function using ripgrep for better context
echo "Searching for function implementation with ripgrep:"
rg "filterAndMapEnabledEntitlements" -A 10 -B 2
echo "\nSearching for files containing the function name:"
fd -t f . | rg "filterAndMapEnabledEntitlements"
Length of output: 7331
Summary by CodeRabbit
New Features
Bug Fixes
Documentation