fix(application-system): Add support for forwarding query params through DelegationsScreen #17593
Conversation
…ugh DelegationsScreen
WalkthroughThe changes modify the Changes
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: 1
🧹 Nitpick comments (1)
libs/application/ui-shell/src/components/DelegationsScreen.tsx (1)
Line range hint
38-43: Enhance type documentation for better reusability.While the component uses TypeScript effectively, adding JSDoc comments for the props interface would improve reusability across different NextJS apps.
Consider adding documentation:
+/** + * Props for the DelegationsScreen component + * @property {Array<{nationalId: string}>} alternativeSubjects - Optional list of alternative subjects + * @property {Dispatch<SetStateAction<boolean>>} checkDelegation - Callback to update delegation check state + * @property {string} slug - The application slug used to determine the type + */ interface DelegationsScreenProps { alternativeSubjects?: { nationalId: string }[] checkDelegation: Dispatch<SetStateAction<boolean>> slug: string }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/application/ui-shell/src/components/DelegationsScreen.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/application/ui-shell/src/components/DelegationsScreen.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (1)
libs/application/ui-shell/src/components/DelegationsScreen.tsx (1)
192-192: LGTM! Verify parameter forwarding in different scenarios.The navigation logic successfully forwards query parameters while maintaining the delegationChecked state.
Run this script to verify parameter handling in different scenarios:
✅ Verification successful
Parameter handling implementation follows codebase patterns
The navigation logic with query parameter forwarding is consistent with similar implementations across the codebase, particularly in delegation-related components and admin modules.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify parameter handling patterns in the codebase # Look for other navigation calls that might need similar parameter forwarding # Search for navigation patterns rg "navigate\(" --type typescript # Search for useSearchParams usage to ensure consistent parameter handling rg "useSearchParams" --type typescriptLength of output: 144
Script:
#!/bin/bash # Search for navigation patterns and parameter handling in TypeScript files rg "navigate\(" -t ts -t tsx -A 2 # Look for useSearchParams usage and parameter handling rg "useSearchParams" -t ts -t tsx -A 2 # Search for query parameter manipulation patterns rg "searchParams" -t ts -t tsx -A 2Length of output: 194
Script:
#!/bin/bash # Find TypeScript/React files containing navigation patterns fd "\.(ts|tsx)$" | xargs rg "navigate\(" -l # Look for parameter handling patterns fd "\.(ts|tsx)$" | xargs rg "useSearchParams|searchParams|paramString" -l # Get detailed context around parameter handling fd "\.(ts|tsx)$" | xargs rg "(\?.*=.*\$\{.*\}|\${paramString})" -A 2 -B 2Length of output: 6533
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/application/ui-shell/src/components/DelegationsScreen.tsx (1)
192-192: Consider using URLSearchParams for robust URL construction.While the current implementation works, using URLSearchParams would provide a more robust solution for handling URL parameters.
- navigate(`?delegationChecked=true${paramString}`) + const newParams = new URLSearchParams(searchParams) + newParams.set('delegationChecked', 'true') + navigate(`?${newParams.toString()}`)This approach:
- Maintains all existing parameters
- Handles parameter encoding consistently
- Follows web standards for URL manipulation
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/application/ui-shell/src/components/DelegationsScreen.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/application/ui-shell/src/components/DelegationsScreen.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (2)
libs/application/ui-shell/src/components/DelegationsScreen.tsx (2)
25-25: LGTM! Clean integration of URL parameter handling.The addition of
useSearchParamshook and its usage follows React conventions and aligns with the PR objective.Also applies to: 56-57
58-63: Well-implemented URL parameter handling.The implementation follows best practices by:
- Properly handling empty searchParams
- Using URL encoding for parameter values
- Efficiently constructing the parameter string
|
Affected services are: application-system-form, Deployed services: application-system-form. |
…ugh DelegationsScreen (#17593) * fix(application-system): Add support for forwarding query params through DelegationsScreen * PR comment
...
Attach a link to issue if relevant
What
Allow query params to pass through the delegations screen
Why
To enable passing params to applications that use the delegation screen.
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
useSearchParamshook.Refactor