feat(api): add extraxt strings for api law and order #16853
Conversation
WalkthroughThe changes in this pull request introduce a new target called 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: 10
🧹 Outside diff range and nitpick comments (6)
libs/api/domains/law-and-order/project.json (1)
28-32: Consider improving the extract-strings target configuration.While the basic structure is good, consider these improvements for better maintainability:
- Define
outputsfor proper Nx caching- Use variables for paths to avoid hardcoding
- Add error handling for the extraction process
"extract-strings": { "executor": "nx:run-commands", "options": { - "command": "yarn ts-node -P libs/localization/tsconfig.lib.json libs/localization/scripts/extract 'libs/api/domains/law-and-order/src/lib/messages.ts'" + "command": "yarn ts-node -P libs/localization/tsconfig.lib.json libs/localization/scripts/extract 'libs/api/domains/law-and-order/src/lib/messages.ts' || exit 1" + }, + "outputs": [ + "{projectRoot}/src/lib/extracted-messages" } }libs/api/domains/law-and-order/src/lib/messages.ts (2)
1-3: Consider improving type safety and documentationThe current implementation could benefit from:
- A more descriptive export name than 'm' (e.g.,
lawAndOrderMessages)- TypeScript type definitions
- JSDoc documentation for better reusability
import { defineMessages } from 'react-intl' -export const m = defineMessages({ +/** + * Internationalized messages for law and order domain + * @description Contains messages related to summons and legal representation choices + */ +export const lawAndOrderMessages = defineMessages({
24-41: Add descriptions for translatorsThe legal choice messages would benefit from descriptions to provide context for translators, especially for legal terminology.
waiveMessage: { id: 'api.law-and-order:no-defender', + description: 'Option shown when defendant chooses to waive their right to a defender', defaultMessage: 'Ég óska ekki eftir verjanda', }, chooseMessage: { id: 'api.law-and-order:choosing-lawyer', + description: 'Option shown when defendant wants to choose their own lawyer as defender', defaultMessage: 'Ég óska þess að valinn lögmaður verði skipaður verjandi minn', },libs/clients/judicial-system-sp/src/lib/judicialSystemSPClient.service.ts (1)
Rename
getSummontogetSubpoenafor naming consistency
The methodgetSummondoes not align with the widespread usage ofsubpoenain the codebase, which may cause confusion and maintenance challenges.
- File:
libs/clients/judicial-system-sp/src/lib/judicialSystemSPClient.service.ts- Method:
getSummon🔗 Analysis chain
Line range hint
49-56: Verify naming consistency between method and API endpointThe method name
getSummondoesn't match the underlying API endpoint namecaseControllerGetSubpoena. This inconsistency could cause confusion for future maintenance.Let's check if this is part of a broader renaming pattern:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of subpoena/summon to verify naming consistency echo "Searching for subpoena references:" rg -i "subpoena" --type ts echo -e "\nSearching for summon references:" rg -i "summon" --type tsLength of output: 554
Script:
#!/bin/bash # Search for type declarations and method names containing 'Subpoena' or 'Summon' echo "Searching for type declarations with 'Subpoena' or 'Summon':" rg -i -e "type.*Subpoena" -e "type.*Summon" --type ts echo -e "\nSearching for method names with 'Subpoena' or 'Summon':" rg -i -e "function.*Subpoena" -e "function.*Summon" --type ts echo -e "\nSearching for class names with 'Subpoena' or 'Summon':" rg -i -e "class.*Subpoena" -e "class.*Summon" --type tsLength of output: 12067
libs/api/domains/law-and-order/src/lib/helpers/mappers.ts (1)
Line range hint
115-127: Consider consistent message handling for all choicesWhile most choices now use imported messages, the DELEGATE choice still uses a hardcoded message. Consider moving this message to the messages module for consistency and easier maintenance.
DELEGATE: { - message: { - id: 'api.law-and-order:choose-for-me', - defaultMessage: 'Ég fel dómara málsins að tilnefna og skipa mér verjanda', - }, + message: m.delegateMessage, },libs/api/domains/law-and-order/src/lib/law-and-order.service.ts (1)
94-94: Handle case sensitivity appropriately when matching stringsThe use of
toLowerCase()on bothitem.labelandsummonStringworks for case-insensitive comparison. However, be cautious with localization, as case handling might differ in some locales. Consider using locale-aware string comparison if necessary.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
libs/api/domains/law-and-order/project.json(1 hunks)libs/api/domains/law-and-order/src/lib/helpers/mappers.ts(4 hunks)libs/api/domains/law-and-order/src/lib/law-and-order.resolver.ts(2 hunks)libs/api/domains/law-and-order/src/lib/law-and-order.service.ts(5 hunks)libs/api/domains/law-and-order/src/lib/messages.ts(1 hunks)libs/clients/judicial-system-sp/src/lib/judicialSystemSPClient.service.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
libs/api/domains/law-and-order/project.json (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."
libs/api/domains/law-and-order/src/lib/helpers/mappers.ts (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."
libs/api/domains/law-and-order/src/lib/law-and-order.resolver.ts (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."
libs/api/domains/law-and-order/src/lib/law-and-order.service.ts (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."
libs/api/domains/law-and-order/src/lib/messages.ts (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."
libs/clients/judicial-system-sp/src/lib/judicialSystemSPClient.service.ts (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 (7)
libs/clients/judicial-system-sp/src/lib/judicialSystemSPClient.service.ts (2)
Line range hint 1-62: Overall structure follows best practices
The service implementation follows good practices:
- Proper dependency injection
- Consistent error handling with middleware
- Clear separation of concerns
- Type-safe implementation
Line range hint 58-62: Consider updating type definitions to match new terminology
While the method has been renamed to patchSummon, it still uses CaseControllerUpdateSubpoenaRequest type and caseControllerUpdateSubpoena endpoint. Consider:
- Creating new types with updated terminology
- Deprecating old types if backward compatibility is needed
Let's check the type definitions and their usage:
#!/bin/bash
# Search for type definitions and their usage
echo "Searching for SubpoenaRequest type usage:"
rg "CaseControllerUpdateSubpoenaRequest" --type ts
echo -e "\nChecking generated API types:"
fd "fetch/index.ts" --exec cat {} \; | grep -A 5 "export.*Subpoena"libs/api/domains/law-and-order/src/lib/helpers/mappers.ts (2)
9-9: LGTM: Clean import of messages module
The addition of the messages import follows good tree-shaking practices and aligns with the PR's objective of improving string extraction for translations.
Line range hint 1-127: Well-structured TypeScript implementation
The implementation demonstrates good practices:
- Strong typing with proper interfaces and enums
- Pure mapping functions that are easily reusable
- Clear separation of concerns
libs/api/domains/law-and-order/src/lib/law-and-order.service.ts (3)
136-138: Ensure mapping functions handle all defense choices
You're using mapDefenseChoiceForSummon and mapDefenseChoiceForSummonDefaultChoice. Confirm that these functions correctly map all possible defense choices and have been updated to reflect any changes in the defense choice enumeration.
65-69: Verify localization keys in messages.ts
The variables summonString, seeSummonString, seeSummonInMailboxString, mailboxLink, and summonLink are using localization messages from m. Ensure that the corresponding keys (m.summon, m.seeSummon, etc.) are defined in messages.ts and accurately translated.
23-24: Confirm the implementation of new mapping functions
You've imported mapDefenseChoiceForSummon and mapDefenseChoiceForSummonDefaultChoice. Ensure these new functions are properly implemented in helpers/mappers.ts and handle all necessary cases for the summon process.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16853 +/- ##
=======================================
Coverage 35.59% 35.59%
=======================================
Files 6920 6920
Lines 146139 146139
Branches 41512 41512
=======================================
Hits 52021 52021
Misses 94118 94118
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
|
Datadog ReportBranch report: ✅ 0 Failed, 4 Passed, 0 Skipped, 3.24s Total Time |
disaerna
added
the
deprecated:automerge
kodiakhq
bot
removed
the
deprecated:automerge
|
This PR currently has a merge conflict. Please resolve this and then re-add the |
disaerna
added
the
deprecated:automerge
kodiakhq
bot
removed
the
deprecated:automerge
|
This PR currently has a merge conflict. Please resolve this and then re-add the |
disaerna
added
the
deprecated:automerge
Api - Law and order
What
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation