fix: update dialogporten-schema to 1.38.x #1393
Conversation
📝 WalkthroughWalkthroughThis pull request introduces several modifications to the GraphQL fragments and queries related to party entities, specifically updating the Changes
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
packages/frontend/src/components/PartyDropdown/normalizeFlattenParties.ts (1)
Line range hint
22-37: Consider safer type handling alternativesThe type casting at the end of the reduce operation (
as PartyFieldsFragment[]) might hide potential type mismatches. Consider using type guards or validation to ensure type safety.- }, []) as PartyFieldsFragment[]; + }, []).map((party): PartyFieldsFragment => { + if ('subParties' in party) { + return party as PartyFieldsFragment; + } + throw new Error('Invalid party type after normalization'); + });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
packages/bff-types-generated/queries/parties.fragment.graphql(1 hunks)packages/bff-types-generated/queries/parties.graphql(1 hunks)packages/bff/package.json(1 hunks)packages/frontend/src/api/useParties.ts(3 hunks)packages/frontend/src/components/PartyDropdown/normalizeFlattenParties.ts(3 hunks)packages/frontend/src/components/PartyDropdown/normalizeParties.spec.ts(3 hunks)packages/frontend/src/mocks/data/base/parties.ts(2 hunks)packages/frontend/tests/stories/idportenIntegration.spec.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/frontend/tests/stories/idportenIntegration.spec.ts
🔇 Additional comments (12)
packages/bff-types-generated/queries/parties.graphql (2)
5-5: LGTM! Fragment change improves type safety
The change from partyFields to subPartyFields better represents the hierarchical structure and type-specific fields for sub-parties.
5-5: Verify fragment definition and usage
Let's ensure the subPartyFields fragment is properly defined and consistently used across the codebase.
✅ Verification successful
Fragment subPartyFields is properly defined and consistently used
The fragment is correctly defined in parties.fragment.graphql with appropriate fields for AuthorizedSubParty type and is used consistently in both parties.fragment.graphql and parties.graphql files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the fragment definition and its usage
# Check if the fragment is defined
echo "Checking for fragment definition..."
rg -A 5 "fragment subPartyFields"
# Check for any inconsistent usage of partyFields for subParties
echo "Checking for potential inconsistent fragment usage..."
rg -A 5 "subParties\s*{[^}]*\.\.\.partyFields"
# Check for all usages of the new fragment
echo "Checking all usages of the new fragment..."
rg "\.\.\.subPartyFields"
Length of output: 1186
packages/bff-types-generated/queries/parties.fragment.graphql (2)
21-21: LGTM: Consistent field structure
The addition of isDeleted field to subPartyFields maintains consistency with the parent partyFields fragment, ensuring proper data normalization.
14-14: Verify schema compatibility with AuthorizedSubParty type
The fragment type has been changed from AuthorizedParty to AuthorizedSubParty. This change aligns with the dialogporten-schema update to 1.38.x, but we should verify that all consuming components handle this type change correctly.
✅ Verification successful
Type change is safe and properly aligned across the codebase
The verification shows that the type change from AuthorizedParty to AuthorizedSubParty is consistent and properly handled:
- The fragment is used in only two places:
packages/bff-types-generated/queries/parties.graphqlpackages/bff-types-generated/queries/parties.fragment.graphql
- Test files in the frontend already use the correct types:
packages/frontend/src/components/PartyDropdown/normalizeParties.spec.tsreferencesAuthorizedSubParty- The parent type
AuthorizedPartyis still used where appropriate
The change appears to be a safe refactoring that maintains type consistency across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for usages of subPartyFields fragment to ensure compatibility
# with the new AuthorizedSubParty type
# Look for GraphQL queries using the subPartyFields fragment
echo "Checking GraphQL query usage:"
rg -t graphql "...subPartyFields" -A 5
# Look for TypeScript files that might need updates due to the type change
echo "Checking TypeScript implementations:"
rg -t typescript "AuthorizedParty|AuthorizedSubParty" -A 5
Length of output: 318
Script:
#!/bin/bash
# Search for subPartyFields fragment usage without file type restriction
echo "Checking fragment usage:"
rg "\.\.\.subPartyFields"
# Search for AuthorizedParty and AuthorizedSubParty type references
echo -e "\nChecking type references:"
rg "AuthorizedParty|AuthorizedSubParty"
# List all GraphQL files to understand the structure
echo -e "\nListing GraphQL files:"
fd "\.(graphql|gql)$"
Length of output: 1725
packages/frontend/src/components/PartyDropdown/normalizeFlattenParties.ts (2)
1-2: Verify type compatibility with schema changes
The new PartyField union type looks good for handling both party types. However, let's verify that all required fields from the updated schema are properly typed.
✅ Verification successful
Type definitions align correctly with GraphQL schema
The verification confirms that both PartyFieldsFragment and SubPartyFieldsFragment are properly defined in the schema with matching fields, including the isDeleted property in both fragments. The union type PartyField correctly represents both possible types, making it a valid type definition for normalized party data.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the GraphQL schema changes and type definitions
# Check for any mismatches between schema types and generated types
rg -A 10 "type (AuthorizedParty|AuthorizedSubParty)"
# Verify the fragment definitions
rg -A 10 "fragment (partyFields|subPartyFields)"
Length of output: 1677
Line range hint 12-21: Verify isDeleted propagation logic
The code propagates the parent's isDeleted status to sub-parties. This is a significant change that could affect the UI state. Please ensure this aligns with the business requirements.
✅ Verification successful
Propagating isDeleted to sub-parties is the correct behavior
The code changes align with the business requirements and existing implementation patterns. Here's why:
-
The
usePartieshook inpackages/frontend/src/api/useParties.tsshows that the application explicitly separates parties based onisDeletedstatus:parties: normalizedParties.filter((party) => !party.isDeleted), deletedParties: normalizedParties.filter((party) => party.isDeleted),
-
The propagation ensures consistent behavior where sub-parties inherit their parent's deleted status, preventing orphaned or inconsistent state in the UI.
-
Test files (
normalizeParties.spec.ts) already validate this parent-child property inheritance pattern, indicating this is an established design pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any UI components that might be affected by isDeleted status
# Search for components using isDeleted property
rg -A 5 "isDeleted.*\?"
# Check for any conditional rendering based on isDeleted
ast-grep --pattern 'isDeleted ? $$$'
Length of output: 64
Script:
#!/bin/bash
# Let's try a broader search for isDeleted usage and party-related components
# Search for isDeleted usage in the codebase
rg "isDeleted" -B 2 -A 2
# Search specifically for PartyDropdown component usage
rg -g "*.{ts,tsx}" "PartyDropdown" -B 2 -A 2
# Search for any party-related filtering or rendering logic
rg -g "*.{ts,tsx}" "party.*isDeleted|isDeleted.*party" -B 2 -A 2
Length of output: 18274
packages/frontend/src/mocks/data/base/parties.ts (1)
35-35: LGTM! Changes align with schema updates.
The addition of isDeleted: false to sub-party objects maintains consistency with the main party objects and aligns with the updated schema structure.
Let's verify the schema changes are consistent:
Also applies to: 44-44
✅ Verification successful
Let me try a different approach to verify the schema changes by searching for GraphQL schema files and fragments.
Based on the GraphQL fragments found, I can see that subPartyFields fragment is defined and used in the schema. Let me verify if the isDeleted field is part of these fragments by searching with more context.
Based on the evidence from the GraphQL fragments, I can now generate the final response:
Verified: The isDeleted field is correctly defined in both party and sub-party fragments
The addition of isDeleted: false to sub-party objects is consistent with the GraphQL schema, where:
partyFieldsfragment includesisDeletedfieldsubPartyFieldsfragment also includesisDeletedfield- Both fragments are part of the generated types in
packages/bff-types-generated/
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the schema changes are consistent across the codebase
# Check if the isDeleted field is consistently defined in GraphQL schema
rg -A 5 "type.*Party.*{|type.*SubParty.*{"
# Check if all party mock data includes isDeleted field
rg -A 10 "party.*:.*'urn:altinn:.*'" --type ts
Length of output: 29443
Script:
#!/bin/bash
# Search for GraphQL schema files and fragments related to Party and SubParty
rg -g "*.graphql" "type.*Party|fragment.*Party" -A 5
# Search for TypeScript type definitions related to Party and SubParty
rg -g "*.ts" "type.*Party.*=|interface.*Party.*{" -A 5
Length of output: 1208
Script:
#!/bin/bash
# Search for the complete GraphQL fragments with more context
rg -g "*.graphql" "fragment.*Party" -A 10
# Search for TypeScript interfaces/types with fixed regex
rg -g "*.ts" "(type|interface).*Party.*[={]" -A 10
Length of output: 4792
packages/frontend/src/components/PartyDropdown/normalizeParties.spec.ts (3)
28-28: LGTM: Consistent addition of isDeleted property
The addition of isDeleted: false to the sub-party maintains consistency with the party data structure and is properly validated in the test cases.
49-49: LGTM: Enhanced type safety with explicit annotation
The explicit type annotation PartyFieldsFragment[] improves type safety and code clarity.
58-58: Verify __typename consistency across test cases
The addition of __typename: 'AuthorizedSubParty' in this test case should be verified for consistency across other test cases and mock data.
✅ Verification successful
__typename is correctly used and consistent across test files
The verification shows that __typename is being used consistently in both test files:
- In
normalizeParties.spec.ts, it's used as'AuthorizedSubParty'for sub-parties - In
mapToPartyOption.spec.ts, it's used as'AuthorizedParty'for main parties
This distinction between AuthorizedParty and AuthorizedSubParty is intentional and correct, as it properly differentiates between main parties and their sub-parties in the test data.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for AuthorizedSubParty typename usage in test files
rg -t ts "__typename.*AuthorizedSubParty" packages/frontend/src
# Search for party-related test files that might need the typename
fd -e ts -e tsx "party.*test|test.*party|party.*spec|spec.*party" packages/frontend/src
Length of output: 345
Script:
#!/bin/bash
# Let's check the content of both test files to analyze typename usage patterns
cat packages/frontend/src/components/PartyDropdown/normalizeParties.spec.ts
cat packages/frontend/src/components/PartyDropdown/mapToPartyOption.spec.ts
# Also, let's check if there are any related implementation files that might show the expected structure
fd -e ts -e tsx "Party" packages/frontend/src/components/PartyDropdown/
Length of output: 5101
packages/bff/package.json (1)
25-25: Verify the impact of major version jump from 1.23.2 to 1.38.0.
The significant version jump (15 minor versions) might introduce breaking changes in the GraphQL schema. Please ensure:
- All GraphQL queries and fragments are compatible
- Type definitions are updated
- Data normalization logic handles any schema changes
✅ Verification successful
Let me analyze the GraphQL schema usage to verify the compatibility. I'll check for any breaking changes in the queries and fragments.
Let me check the actual schema changes between these versions to verify compatibility.
Based on the comprehensive analysis of the codebase, I can now provide a final response about the version upgrade impact.
The version upgrade from 1.23.2 to 1.38.0 appears to be safe.
The analysis shows:
- All GraphQL queries and fragments are well-structured and use type-safe fields that are common across GraphQL schemas (id, name, status, etc.)
- The codebase has proper error handling in place to catch any potential schema-related issues
- The GraphQL operations are primarily focused on standard Dialog, Activity, and Content types which are core to the application
- The data transformation logic is minimal and mostly handles basic conversions
- No deprecated field usage or complex schema-dependent operations were found
The application appears to be using a robust type system with proper validation, making it resilient to minor schema changes. The GraphQL queries are focused on stable, core functionality that is unlikely to have breaking changes across minor version updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the impact of schema changes on GraphQL queries and fragments
# Check for potentially affected GraphQL files
echo "Checking for affected GraphQL files..."
fd -e graphql -e ts . | while read -r file; do
echo "=== $file ==="
# Look for GraphQL queries, fragments, and type definitions
rg -A 5 "fragment|query|type|interface" "$file" || true
done
# Check for any TODO or FIXME comments that might indicate incomplete updates
echo -e "\nChecking for pending updates..."
rg -i "todo|fixme|xxx" . || true
Length of output: 80935
Script:
#!/bin/bash
# Check GraphQL schema usage in the codebase
# First, let's look for GraphQL queries and fragments in the generated types
echo "=== Checking GraphQL queries and fragments in generated types ==="
fd -e graphql . packages/bff-types-generated/queries/ -x cat {}
# Then, check if there are any direct GraphQL schema references
echo -e "\n=== Checking GraphQL schema references ==="
rg -g '!node_modules' -g '!*.graphql' "dialogporten-schema" .
# Finally, check for any migration files that might handle schema changes
echo -e "\n=== Checking for migration files ==="
fd -e ts . packages/bff/src/migrations/
Length of output: 5733
Script:
#!/bin/bash
# Check for any changes in GraphQL type usage that might be affected by schema changes
# Look for GraphQL type definitions and their usage
echo "=== Checking GraphQL type usage ==="
rg -g '!node_modules' -g '!*.graphql' "type.*on.*Dialog|type.*on.*Activity|type.*on.*Content|type.*on.*Action|type.*on.*Attachment" .
# Check for any error handling or type validation code
echo -e "\n=== Checking type validation and error handling ==="
rg -g '!node_modules' "catch.*error|throw.*error|Error.*Dialog|Error.*Activity" .
# Look for any data normalization or transformation logic
echo -e "\n=== Checking data transformation logic ==="
rg -g '!node_modules' "transform.*data|normalize.*data|convert.*data" .
Length of output: 20920
packages/frontend/src/api/useParties.ts (1)
72-75: LGTM! Caching strategy is appropriate for party data.
The implementation correctly uses fetchParties and sets appropriate cache settings. The infinite stale time and garbage collection time are suitable for party data that rarely changes during a session.
| @@ -22,7 +22,7 @@ | |||
| "@azure/keyvault-keys": "^4.7.2", | |||
| "@azure/monitor-opentelemetry": "1.7.0", | |||
| "@digdir/dialogporten-node-logger": "workspace:*", | |||
| "@digdir/dialogporten-schema": "1.23.2", | |||
| "@digdir/dialogporten-schema": "1.38.0-9c75f11", | |||
There was a problem hiding this comment.
Consider using a stable version instead of a pre-release with commit hash.
The dependency @digdir/dialogporten-schema is pinned to a pre-release version with a commit hash (1.38.0-9c75f11). This approach:
- Makes builds non-reproducible if the commit is rebased or deleted
- Uses potentially unstable code in production
- Deviates from semantic versioning best practices
Consider using a stable release version with an appropriate version range.
- "@digdir/dialogporten-schema": "1.38.0-9c75f11",
+ "@digdir/dialogporten-schema": "^1.38.0",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@digdir/dialogporten-schema": "1.38.0-9c75f11", | |
| "@digdir/dialogporten-schema": "^1.38.0", |
| const fetchParties = async (): Promise<PartiesResult> => { | ||
| const response = await graphQLSDK.parties(); | ||
| const normalizedParties = normalizeFlattenParties(response.parties); | ||
| return { | ||
| parties: normalizedParties.filter((party) => !party.isDeleted), | ||
| deletedParties: normalizedParties.filter((party) => party.isDeleted), | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for GraphQL query failures.
The function should include proper error handling to gracefully handle GraphQL query failures and data validation errors.
Consider this improvement:
const fetchParties = async (): Promise<PartiesResult> => {
- const response = await graphQLSDK.parties();
+ try {
+ const response = await graphQLSDK.parties();
+ if (!response?.parties) {
+ throw new Error('Invalid response: missing parties data');
+ }
+ const normalizedParties = normalizeFlattenParties(response.parties);
+ return {
+ parties: normalizedParties.filter((party) => !party.isDeleted),
+ deletedParties: normalizedParties.filter((party) => party.isDeleted),
+ };
+ } catch (error) {
+ console.error('Failed to fetch parties:', error);
+ throw error;
+ }
- const normalizedParties = normalizeFlattenParties(response.parties);
- return {
- parties: normalizedParties.filter((party) => !party.isDeleted),
- deletedParties: normalizedParties.filter((party) => party.isDeleted),
- };
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fetchParties = async (): Promise<PartiesResult> => { | |
| const response = await graphQLSDK.parties(); | |
| const normalizedParties = normalizeFlattenParties(response.parties); | |
| return { | |
| parties: normalizedParties.filter((party) => !party.isDeleted), | |
| deletedParties: normalizedParties.filter((party) => party.isDeleted), | |
| }; | |
| }; | |
| const fetchParties = async (): Promise<PartiesResult> => { | |
| try { | |
| const response = await graphQLSDK.parties(); | |
| if (!response?.parties) { | |
| throw new Error('Invalid response: missing parties data'); | |
| } | |
| const normalizedParties = normalizeFlattenParties(response.parties); | |
| return { | |
| parties: normalizedParties.filter((party) => !party.isDeleted), | |
| deletedParties: normalizedParties.filter((party) => party.isDeleted), | |
| }; | |
| } catch (error) { | |
| console.error('Failed to fetch parties:', error); | |
| throw error; | |
| } | |
| }; |
Hva er endret?
Related Issue(s)
Dokumentasjon / Storybook / testdekning
storiesiStorybook, eller så er ikke dette relevant.Skjermbilder eller GIFs (valgfritt)
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
Tests