chore(j-s): Use lawyer registry from api #17434
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. WalkthroughThis pull request involves the removal of several API endpoint files related to lawyer retrieval ( Changes
Possibly related PRs
Suggested reviewers
Finishing Touches
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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17434 +/- ##
==========================================
- Coverage 35.60% 35.60% -0.01%
==========================================
Files 7019 7010 -9
Lines 150408 150237 -171
Branches 42961 42943 -18
==========================================
- Hits 53553 53489 -64
+ Misses 96855 96748 -107
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 52 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 2 Total Test Services: 0 Failed, 2 Passed Test Services
|
![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
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/judicial-system/web/pages/api/lawyers/getLawyer/getLawyer.ts(0 hunks)apps/judicial-system/web/pages/api/lawyers/getLawyer/index.ts(0 hunks)apps/judicial-system/web/pages/api/lawyers/getLawyers/getLawyers.ts(0 hunks)apps/judicial-system/web/pages/api/lawyers/getLawyers/index.ts(0 hunks)apps/judicial-system/web/src/utils/hooks/useLawyers/useLawyers.ts(2 hunks)
💤 Files with no reviewable changes (4)
- apps/judicial-system/web/pages/api/lawyers/getLawyer/index.ts
- apps/judicial-system/web/pages/api/lawyers/getLawyers/index.ts
- apps/judicial-system/web/pages/api/lawyers/getLawyer/getLawyer.ts
- apps/judicial-system/web/pages/api/lawyers/getLawyers/getLawyers.ts
🧰 Additional context used
📓 Path-based instructions (1)
apps/judicial-system/web/src/utils/hooks/useLawyers/useLawyers.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (2)
apps/judicial-system/web/src/utils/hooks/useLawyers/useLawyers.ts (2)
13-14: LGTM! The API endpoint change is well implemented.The updated endpoint aligns with the PR objectives, and the SWR configuration is appropriate for this use case.
Line range hint
1-52: Verify the impact of API endpoint changes.The changes to the API endpoints might affect components using these hooks.
Let's verify the usage of these hooks and ensure all components are updated:
✅ Verification successful
API endpoint changes are properly encapsulated ✅
The changes to API endpoints are safely contained within the hooks implementation, with all components using these hooks correctly through proper imports. No direct usage of old API endpoints was found in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for components using the old API endpoints echo "Checking for old API endpoint usage..." rg -l "api/lawyers/getLawyers\|api/lawyers/getLawyer" "apps/judicial-system/web/src/components" # Search for components using these hooks echo "Checking hook usage..." rg -l "useGetLawyers\|useGetLawyer" "apps/judicial-system/web/src/components"Length of output: 315
Script:
#!/bin/bash # Search for any API endpoint patterns in the entire web directory echo "Checking for API endpoint patterns..." rg -l "api/lawyers|api/defender" "apps/judicial-system/web/src" # Use ast-grep to find hook usage echo "Checking hook usage with AST..." ast-grep --pattern 'useGetLawyer($$$)' ast-grep --pattern 'useGetLawyers($$$)' # Search for imports of these hooks echo "Checking hook imports..." rg -l "import.*useGetLawyer" "apps/judicial-system/web/src"Length of output: 1334
apps/judicial-system/web/src/utils/hooks/useLawyers/useLawyers.ts
Outdated
Show resolved
Hide resolved
apps/judicial-system/web/src/utils/hooks/useLawyers/useLawyers.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/judicial-system/web/src/utils/hooks/useLawyers/useLawyers.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/judicial-system/web/src/utils/hooks/useLawyers/useLawyers.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (3)
apps/judicial-system/web/src/utils/hooks/useLawyers/useLawyers.ts (3)
20-21: API endpoint update looks good!The endpoint change correctly implements the new lawyer registry path as specified in the PR objectives.
42-51: Remove duplicated fetcher implementation.This fetcher implementation should be replaced with the shared implementation suggested above.
54-57:⚠️ Potential issueAdd error handling to match useGetLawyers implementation.
The hook is missing error handling that exists in the useGetLawyers hook.
- const { data } = useSWR<Lawyer>( + const { data, error } = useSWR<Lawyer>( nationalId && shouldFetch ? `/api/defender/lawyerRegistry/${nationalId}` : null, fetcher, { revalidateIfStale: false, revalidateOnFocus: false, revalidateOnReconnect: false, errorRetryCount: 2, }, ) + if (error) { + toast.error(formatMessage(errorMessages.fetchLawyer)) + return undefined + } return dataLikely invalid or redundant comment.
apps/judicial-system/web/src/utils/hooks/useLawyers/useLawyers.ts
Outdated
Show resolved
Hide resolved
apps/judicial-system/web/src/utils/hooks/useLawyers/useLawyers.ts
Outdated
Show resolved
Hide resolved
oddsson
added
the
deprecated:automerge
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/judicial-system/web/messages/Core/errors.ts (1)
97-102: LGTM with a minor suggestion for consistency.The new error message is well-structured and aligns with the PR's goal of centralizing lawyer registry access. It complements the existing
fetchLawyersmessage and follows the established patterns.Consider using "við að" consistently in the
defaultMessage. Other similar messages use this pattern:- defaultMessage: 'Upp kom villa við að sækja lögmann úr lögmanna skrá', + defaultMessage: 'Upp kom villa við að sækja lögmann frá lögmanna skrá',
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/judicial-system/web/messages/Core/errors.ts(1 hunks)apps/judicial-system/web/src/utils/hooks/useLawyers/useLawyers.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/judicial-system/web/src/utils/hooks/useLawyers/useLawyers.ts
🧰 Additional context used
📓 Path-based instructions (1)
apps/judicial-system/web/messages/Core/errors.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
oddsson
dismissed stale reviews from thorhildurt and coderabbitai[bot]
via
c4576a2
Use lawyer registry from api
Asana
What
When we first integrated with the lawyer registry from LMFI we did so by creating an API route and calling the endpoint from there. Then when we were working on showing subpoenas on minar sidur we created the same module in our API layer because minar sidur needed to get the lawyer registry from us.
This PR cleans this code up a bit by removing the API routes and using the module in the API layer as well.
Why
This way, we don't have code that does the same thing living in two different places.
Screenshots / Gifs
Here you can see we're calling
/api/defender/lawyerRegistryinstead of/api/lawyers/getLawyersand here we're calling
/api/defenders/lawyerRegistry/:idinstead of/api/lawyers/getLawyer?nationalId=:idChecklist:
Summary by CodeRabbit
API Changes
/api/defender/lawyerRegistry.Refactor
New Features
Impact