Tests for #4649 and #4650 #389
Conversation
WalkthroughThis pull request involves multiple file modifications across test and GraphQL directories. The primary changes include removing a GraphQL fragment ( Changes
Possibly related PRs
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/functional-api/callout/call-for-whiteboards/whiteboard-collection-callout.params.request.ts (1)
54-56: Consider using a more meaningful test display name.The hardcoded display name '111' could be more descriptive to better represent its purpose in tests.
Suggestion:
profile: { - displayName: '111', + displayName: 'test-whiteboard', },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
test/generated/alkemio-schema.tsis excluded by!**/generated/**test/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (5)
graphql/fragments/features/featureFlagRules.graphql(0 hunks)test/functional-api/callout/call-for-whiteboards/whiteboard-collection-callout.params.request.ts(1 hunks)test/functional-api/callout/whiteboard/whiteboard-callout.params.request.ts(1 hunks)test/functional-api/callout/whiteboard/whiteboardRt-callout.params.request.ts(1 hunks)test/functional-api/templates/whiteboard/whiteboard-templates.request.params.ts(1 hunks)
💤 Files with no reviewable changes (1)
- graphql/fragments/features/featureFlagRules.graphql
🔇 Additional comments (3)
test/functional-api/callout/whiteboard/whiteboardRt-callout.params.request.ts (1)
30-33: Verify the necessity of duplicate profile information.
There appears to be profile information defined at two levels:
framing.profilewith a dynamicdisplayNameframing.whiteboard.profilewith a hardcoded 'whiteboard'displayName
This might lead to confusion about which profile information takes precedence.
Let's verify if this is the expected behavior by checking other whiteboard-related tests:
✅ Verification successful
Based on the search results, I can now provide a definitive response about the whiteboard profile structure:
The whiteboard profile structure is correct and intentional
The codebase shows that whiteboard callouts consistently use this nested profile structure:
framing.profile- Contains the main callout profile informationframing.whiteboard.profile- Contains whiteboard-specific profile information
This is evidenced by multiple test files showing this pattern, particularly in:
- Whiteboard template creation
- Whiteboard callout creation
- Storage/document handling for whiteboards
The hardcoded 'whiteboard' displayName in framing.whiteboard.profile appears to be a deliberate identifier for the whiteboard component, while the dynamic displayName in framing.profile represents the user-facing callout name.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar whiteboard profile structures in test files
rg -A 5 "whiteboard.*profile.*displayName" test/
Length of output: 50
Script:
#!/bin/bash
# Let's try a broader search for profile structures in test files
rg -B 2 -A 5 'profile.*displayName' test/functional-api/
Length of output: 46822
Script:
#!/bin/bash
# Search for whiteboard-related test files to understand the context better
fd whiteboard test/functional-api/ --exec cat {}
Length of output: 91823
Script:
#!/bin/bash
# Look for framing structures in test files
rg -B 2 -A 5 'framing.*profile' test/functional-api/
Length of output: 79694
test/functional-api/callout/whiteboard/whiteboard-callout.params.request.ts (1)
30-33: Same profile duplication concern as in whiteboardRt-callout.params.request.ts
test/functional-api/callout/call-for-whiteboards/whiteboard-collection-callout.params.request.ts (1)
54-56: Verify profile structure consistency across different contexts.
The profile structure is used in different contexts (framing vs contributionData). Let's verify the consistency of this pattern across the codebase.
✅ Verification successful
Let me analyze the search results to verify the profile structure consistency:
- In framing context:
- The profile consistently contains
displayNamefield across all test files - The profile structure is defined in schema (test/generated/graphql.ts) with
displayName: string - Used consistently in callout creation tests with
framing: { profile: { displayName: string } }
- In whiteboard context:
- Profile contains
storageBucketandvisualfields - Used consistently in both public and private space document auth tests
- Same structure used in both framing.whiteboard.profile and contributions[].whiteboard.profile paths
- No direct contributionData.profile usage found in the search results
The profile structure appears to be consistent across different contexts, with each context having its specific required fields that are used consistently throughout the test files. The displayName field being used in the test file matches the schema definition and existing usage patterns.
Profile structure is consistent with codebase patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for profile structure usage in different contexts
echo "=== Profile in framing context ==="
rg -A 3 "framing.*profile" test/
echo "=== Profile in contributionData context ==="
rg -A 3 "contributionData.*profile" test/
echo "=== Profile in whiteboard context ==="
rg -A 3 "whiteboard.*profile" test/
Length of output: 68943
| profile: { | ||
| displayName: 'Whiteboard Template', | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Inconsistency found in GraphQL mutations for template creation
The change from profileData to profile is inconsistent across the codebase:
- In test files, the new
profilestructure is correctly used - However, GraphQL mutations still use
profileDatain their template creation:createWhiteboardTemplate.graphql: Input parameter is$profilebut used asprofileDatain mutationcreateTemplate.graphql: Same inconsistencycreateTemplateFromCollaboration.graphql: Still using oldprofileDatanaming
🔗 Analysis chain
LGTM! Property rename aligns with schema changes.
The change from profileData to profile correctly aligns with the PR objectives and maintains consistency with the framing > profile structure.
Let's verify the consistency of this change across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of profileData that might need updating
# and verify the new profile structure is used consistently
# Check for any remaining instances of profileData
echo "Checking for remaining profileData instances:"
rg "profileData" -l
# Verify the new profile structure usage
echo "Verifying profile structure usage:"
ast-grep --pattern 'profile: {
displayName: $_,
$$$
}'
Length of output: 2190
Script:
#!/bin/bash
# Let's examine the actual usage of profileData in test files to understand if they need updates
# Focus on template-related files first as they're most relevant to our context
echo "Checking profileData usage in template-related files:"
rg "profileData" -B 2 -A 2 test/functional-api/templates/
rg "profileData" graphql/mutations/templates/
echo -e "\nChecking current profile structure in template files:"
rg "profile.*{" -B 2 -A 2 test/functional-api/templates/
rg "profile.*{" -B 2 -A 2 graphql/mutations/templates/
Length of output: 3234
Script:
#!/bin/bash
# Let's fix the regex and check the profile structure properly
# Also examine the specific GraphQL mutations that still use profileData
echo "Checking profile structure in template files:"
rg "profile: \{" -B 2 -A 2 test/functional-api/templates/
rg "profile: \{" -B 2 -A 2 graphql/mutations/templates/
echo -e "\nExamining GraphQL mutations with profileData:"
cat graphql/mutations/templates/createWhiteboardTemplate.graphql
echo -e "\n---\n"
cat graphql/mutations/templates/createTemplate.graphql
echo -e "\n---\n"
cat graphql/mutations/templates/createTemplateFromCollaboration.graphql
Length of output: 6654
profileDatareceivesprofileto be consistent with theframing > profileand to be able to create a whiteboard callout with visualsprofiledoesn't haveavatarURLanymore, it hasvisualswhich is an array of{url: string, name: VisualType}to properly push all the visuals on any profile creation.inputCreatorservice now can return the visualsalkemio-schemafile): LicenseCredential, file extensions...Summary by CodeRabbit
Bug Fixes
featureFlagRulesGraphQL fragment, impacting related queries and mutations.New Features
createWhiteboardOnCallout,createWhiteboardCallout, andcreateWhiteboardTemplatefunctions to standardize property naming fromprofileDatatoprofile, enhancing data structure consistency.Documentation