fix(work-accident-notification): fixing duplicate message ids #16935
Conversation
sigruntg
added
the
deprecated:automerge
WalkthroughThe pull request introduces updates to message identifiers across three files related to employee, information, and overview messages. Specifically, it modifies the Changes
Possibly related PRs
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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16935 +/- ##
=======================================
Coverage 36.45% 36.45%
=======================================
Files 6903 6903
Lines 144598 144598
Branches 41288 41288
=======================================
Hits 52720 52720
Misses 91878 91878 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 ReportAll test runs ✅ 23 Total Test Services: 0 Failed, 22 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
libs/application/templates/aosh/work-accident-notification/src/lib/messages/employee.ts(2 hunks)libs/application/templates/aosh/work-accident-notification/src/lib/messages/information.ts(1 hunks)libs/application/templates/aosh/work-accident-notification/src/lib/messages/overview.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
libs/application/templates/aosh/work-accident-notification/src/lib/messages/employee.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/application/templates/aosh/work-accident-notification/src/lib/messages/information.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/application/templates/aosh/work-accident-notification/src/lib/messages/overview.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 (4)
libs/application/templates/aosh/work-accident-notification/src/lib/messages/employee.ts (1)
82-82: Verify usage of the updated message ID aosh.wan.application:employee.startOfWorkdayDate
The ID change from time to startOfWorkdayDate provides better semantic clarity for the workday start date context.
Let's verify that all usages of this message ID have been updated:
libs/application/templates/aosh/work-accident-notification/src/lib/messages/information.ts (1)
17-20: Potential duplicate message definitions
There appear to be two message definitions with the same defaultMessage: 'Tilkynningaraðili':
information.general.pageTitleinformation.labels.company.pageTitle
This could lead to maintenance issues and confusion. Consider:
- Using different messages if they serve different purposes
- Consolidating them if they serve the same purpose
Also applies to: 28-32
libs/application/templates/aosh/work-accident-notification/src/lib/messages/overview.ts (2)
50-82: LGTM! The message ID changes fix the duplicate ID issue.
The changes correctly resolve the duplicate message IDs by giving each field its own unique identifier that matches its content.
Line range hint 1-1: Verify message ID uniqueness across the codebase.
Let's ensure there are no remaining duplicate message IDs in other files.
| @@ -69,7 +69,7 @@ export const employee = { | |||
| description: 'Employment time in the same job', | |||
| }, | |||
| employmentRate: { | |||
| id: 'aosh.wan.application:employee.employmentTime', | |||
| id: 'aosh.wan.application:employee.employmentRate', | |||
There was a problem hiding this comment.
💡 Codebase verification
Update required: Inconsistent message ID usage found
The verification revealed an inconsistency in the codebase:
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/employee.tsis still using the old field nameemploymentTimewhile the message ID has been updated toemploymentRate- This mismatch needs to be fixed to maintain consistency between the message definitions and their usage
🔗 Analysis chain
Verify usage of the updated message ID aosh.wan.application:employee.employmentRate
The ID change from employmentTime to employmentRate better reflects the message's purpose for employment rate percentage.
Let's verify that all usages of this message ID have been updated:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to the old message ID
rg "aosh\.wan\.application:employee\.employmentTime"
# Search for references to the new message ID to ensure it's being used
rg "aosh\.wan\.application:employee\.employmentRate"
Length of output: 630
Script:
#!/bin/bash
# Search for any remaining references to the old message ID (employmentTime)
rg "aosh\.wan\.application:employee\.employmentTime" || echo "No matches found for old ID (employmentTime)"
# Search for references to the new message ID (employmentRate)
rg "aosh\.wan\.application:employee\.employmentRate" || echo "No matches found for new ID (employmentRate)"
# Search for files that might be using these message IDs
fd -e ts -e tsx -e js -e jsx | xargs grep -l "employee\." || echo "No files found with employee messages"
Length of output: 2223
Script:
#!/bin/bash
# Let's check the content of files that might be using these message IDs
rg -A 2 -B 2 "employmentTime|employmentRate" libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/employee.ts libs/application/templates/aosh/work-accident-notification/src/fields/Overview/index.tsx libs/application/templates/aosh/work-accident-notification/src/fields/PdfOverview/pdfDocument.tsx
Length of output: 2280
| id: 'aosh.wan.application:information.general.pageTitle', | ||
| defaultMessage: 'Tilkynningaraðili', | ||
| description: 'Title of company information section', | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update description to match the general context
The message ID has been moved from 'company' to 'general' context, but the description still references "company information section". This mismatch could cause confusion for other developers.
pageTitle: {
id: 'aosh.wan.application:information.general.pageTitle',
defaultMessage: 'Tilkynningaraðili',
- description: 'Title of company information section',
+ description: 'Title of general information section',
},📝 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.
| id: 'aosh.wan.application:information.general.pageTitle', | |
| defaultMessage: 'Tilkynningaraðili', | |
| description: 'Title of company information section', | |
| }, | |
| id: 'aosh.wan.application:information.general.pageTitle', | |
| defaultMessage: 'Tilkynningaraðili', | |
| description: 'Title of general information section', | |
| }, |
| id: 'aosh.wan.application:overview.labels.didAoshCome', | ||
| defaultMessage: 'Kom Vinnueftirlitið?', | ||
| description: 'Date and time label', | ||
| }, | ||
| didPoliceCome: { | ||
| id: 'aosh.wan.application:overview.labels.dateAndTime', | ||
| id: 'aosh.wan.application:overview.labels.didPoliceCome', | ||
| defaultMessage: 'Kom Lögreglan?', | ||
| description: 'Date and time label', | ||
| }, | ||
| injuredAmount: { | ||
| id: 'aosh.wan.application:overview.labels.dateAndTime', | ||
| id: 'aosh.wan.application:overview.labels.injuredAmount', | ||
| defaultMessage: 'Fjöldi slasaðra', | ||
| description: 'Date and time label', | ||
| }, | ||
| municipality: { | ||
| id: 'aosh.wan.application:overview.labels.dateAndTime', | ||
| id: 'aosh.wan.application:overview.labels.municipality', | ||
| defaultMessage: 'Sveitarfélag þar sem slysið átti sér stað', | ||
| description: 'Date and time label', | ||
| }, | ||
| exactLocation: { | ||
| id: 'aosh.wan.application:overview.labels.dateAndTime', | ||
| id: 'aosh.wan.application:overview.labels.exactLocation', | ||
| defaultMessage: 'Nákvæm staðsetning slyssins', | ||
| description: 'Date and time label', | ||
| }, | ||
| accidentDescription: { | ||
| id: 'aosh.wan.application:overview.labels.dateAndTime', | ||
| id: 'aosh.wan.application:overview.labels.accidentDescription', | ||
| defaultMessage: 'Tildrög slyssins', | ||
| description: 'Date and time label', | ||
| }, | ||
| locationOfAccident: { | ||
| id: 'aosh.wan.application:overview.labels.dateAndTime', | ||
| id: 'aosh.wan.application:overview.labels.locationOfAccident', | ||
| defaultMessage: 'Vettvangur slyssins', | ||
| description: 'Date and time label', |
There was a problem hiding this comment.
Update the description fields to match their respective messages.
The description field for several messages still incorrectly states "Date and time label". Each description should accurately reflect its corresponding message content.
Apply this diff to fix the descriptions:
didAoshCome: {
id: 'aosh.wan.application:overview.labels.didAoshCome',
defaultMessage: 'Kom Vinnueftirlitið?',
- description: 'Date and time label',
+ description: 'Label for whether AOSH came to the accident site',
},
didPoliceCome: {
id: 'aosh.wan.application:overview.labels.didPoliceCome',
defaultMessage: 'Kom Lögreglan?',
- description: 'Date and time label',
+ description: 'Label for whether police came to the accident site',
},
injuredAmount: {
id: 'aosh.wan.application:overview.labels.injuredAmount',
defaultMessage: 'Fjöldi slasaðra',
- description: 'Date and time label',
+ description: 'Label for number of injured people',
},
municipality: {
id: 'aosh.wan.application:overview.labels.municipality',
defaultMessage: 'Sveitarfélag þar sem slysið átti sér stað',
- description: 'Date and time label',
+ description: 'Label for municipality where accident occurred',
},
exactLocation: {
id: 'aosh.wan.application:overview.labels.exactLocation',
defaultMessage: 'Nákvæm staðsetning slyssins',
- description: 'Date and time label',
+ description: 'Label for exact location of accident',
},
accidentDescription: {
id: 'aosh.wan.application:overview.labels.accidentDescription',
defaultMessage: 'Tildrög slyssins',
- description: 'Date and time label',
+ description: 'Label for accident description',
},
locationOfAccident: {
id: 'aosh.wan.application:overview.labels.locationOfAccident',
defaultMessage: 'Vettvangur slyssins',
- description: 'Date and time label',
+ description: 'Label for accident location type',
},📝 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.
| id: 'aosh.wan.application:overview.labels.didAoshCome', | |
| defaultMessage: 'Kom Vinnueftirlitið?', | |
| description: 'Date and time label', | |
| }, | |
| didPoliceCome: { | |
| id: 'aosh.wan.application:overview.labels.dateAndTime', | |
| id: 'aosh.wan.application:overview.labels.didPoliceCome', | |
| defaultMessage: 'Kom Lögreglan?', | |
| description: 'Date and time label', | |
| }, | |
| injuredAmount: { | |
| id: 'aosh.wan.application:overview.labels.dateAndTime', | |
| id: 'aosh.wan.application:overview.labels.injuredAmount', | |
| defaultMessage: 'Fjöldi slasaðra', | |
| description: 'Date and time label', | |
| }, | |
| municipality: { | |
| id: 'aosh.wan.application:overview.labels.dateAndTime', | |
| id: 'aosh.wan.application:overview.labels.municipality', | |
| defaultMessage: 'Sveitarfélag þar sem slysið átti sér stað', | |
| description: 'Date and time label', | |
| }, | |
| exactLocation: { | |
| id: 'aosh.wan.application:overview.labels.dateAndTime', | |
| id: 'aosh.wan.application:overview.labels.exactLocation', | |
| defaultMessage: 'Nákvæm staðsetning slyssins', | |
| description: 'Date and time label', | |
| }, | |
| accidentDescription: { | |
| id: 'aosh.wan.application:overview.labels.dateAndTime', | |
| id: 'aosh.wan.application:overview.labels.accidentDescription', | |
| defaultMessage: 'Tildrög slyssins', | |
| description: 'Date and time label', | |
| }, | |
| locationOfAccident: { | |
| id: 'aosh.wan.application:overview.labels.dateAndTime', | |
| id: 'aosh.wan.application:overview.labels.locationOfAccident', | |
| defaultMessage: 'Vettvangur slyssins', | |
| description: 'Date and time label', | |
| id: 'aosh.wan.application:overview.labels.didAoshCome', | |
| defaultMessage: 'Kom Vinnueftirlitið?', | |
| description: 'Label for whether AOSH came to the accident site', | |
| }, | |
| didPoliceCome: { | |
| id: 'aosh.wan.application:overview.labels.didPoliceCome', | |
| defaultMessage: 'Kom Lögreglan?', | |
| description: 'Label for whether police came to the accident site', | |
| }, | |
| injuredAmount: { | |
| id: 'aosh.wan.application:overview.labels.injuredAmount', | |
| defaultMessage: 'Fjöldi slasaðra', | |
| description: 'Label for number of injured people', | |
| }, | |
| municipality: { | |
| id: 'aosh.wan.application:overview.labels.municipality', | |
| defaultMessage: 'Sveitarfélag þar sem slysið átti sér stað', | |
| description: 'Label for municipality where accident occurred', | |
| }, | |
| exactLocation: { | |
| id: 'aosh.wan.application:overview.labels.exactLocation', | |
| defaultMessage: 'Nákvæm staðsetning slyssins', | |
| description: 'Label for exact location of accident', | |
| }, | |
| accidentDescription: { | |
| id: 'aosh.wan.application:overview.labels.accidentDescription', | |
| defaultMessage: 'Tildrög slyssins', | |
| description: 'Label for accident description', | |
| }, | |
| locationOfAccident: { | |
| id: 'aosh.wan.application:overview.labels.locationOfAccident', | |
| defaultMessage: 'Vettvangur slyssins', | |
| description: 'Label for accident location type', |
Fixing duplicate message ids
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes