fix(application-system): Contentful strings removed #17478
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThis pull request involves modifications to the work accident notification template, specifically removing a description field from the announcement section. The changes include deleting a secondary heading field in the form's component and removing the corresponding message definition in the external data messages file. The alteration simplifies the form's structure while maintaining the core functionality of the work accident notification process. Changes
Possibly related PRs
Suggested labels
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
|
Ballioli
added
the
deprecated:automerge
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17478 +/- ##
==========================================
+ Coverage 35.66% 35.68% +0.01%
==========================================
Files 6977 6977
Lines 149793 149636 -157
Branches 42835 42769 -66
==========================================
- Hits 53425 53395 -30
+ Misses 96368 96241 -127
Flags with carried forward coverage won't be shown. Click here to find out more. see 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/application/templates/aosh/work-accident-notification/src/lib/messages/externalData.ts (1)
Line range hint
31-45: Improve message structure and readability.The message uses template literals for multiline content, but the formatting could be improved:
- Consider breaking down this long message into smaller, more manageable chunks.
- The line breaks and indentation in the template literal could be more consistent.
Consider this structure:
announcementDescription: { id: 'aosh.wan.application:externalData.dataProvider.announcementDescription', - defaultMessage: `Vinsamlega athugaðu að þú ert innskráð/ur sem einstaklingur. - Atvinnurekanda ber skylda til að sjá til þess að vinnuslys sé tilkynnt til - Vinnueftirlitsins þegar slys verður vegna eða við vinnu á vinnustað hans - og starfsmaður verður óvinnufær í einn eða fleiri daga, umfram þann dag - sem slysið varð. Einstaklingar eiga ekki að tilkynna eigin slys inn til - Vinnueftirlitsins eina undantekningin á þessari reglu eru einstaklingar - sem reka eigin starfsemi á sinni eigin kennitölu. - \n Ef þú ert að skrá þessa tilkynningu fyrir hönd fyrirtækis þá væri æskilegra - að þú værir innskráður á vefinn með auðkenni viðkomandi fyrirtækis eða með umboð - frá fyrirtækinu til að skrá vinnuslys fyrir hönd þess. Athugaðu að þú munt ekki hafa aðgang að neinum upplýsingum - um aðrar tilkynningar fyrirtækisins.`, + defaultMessage: [ + 'Vinsamlega athugaðu að þú ert innskráð/ur sem einstaklingur.', + 'Atvinnurekanda ber skylda til að sjá til þess að vinnuslys sé tilkynnt til ', + 'Vinnueftirlitsins þegar slys verður vegna eða við vinnu á vinnustað hans ', + 'og starfsmaður verður óvinnufær í einn eða fleiri daga, umfram þann dag ', + 'sem slysið varð. Einstaklingar eiga ekki að tilkynna eigin slys inn til ', + 'Vinnueftirlitsins eina undantekningin á þessari reglu eru einstaklingar ', + 'sem reka eigin starfsemi á sinni eigin kennitölu.', + '', + 'Ef þú ert að skrá þessa tilkynningu fyrir hönd fyrirtækis þá væri æskilegra ', + 'að þú værir innskráður á vefinn með auðkenni viðkomandi fyrirtækis eða með umboð ', + 'frá fyrirtækinu til að skrá vinnuslys fyrir hönd þess. Athugaðu að þú munt ekki ', + 'hafa aðgang að neinum upplýsingum um aðrar tilkynningar fyrirtækisins.' + ].join(''), description: 'Announcement description following prerequisites', },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/AnnouncementSection/index.ts(0 hunks)libs/application/templates/aosh/work-accident-notification/src/lib/messages/externalData.ts(2 hunks)
💤 Files with no reviewable changes (1)
- libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/AnnouncementSection/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
libs/application/templates/aosh/work-accident-notification/src/lib/messages/externalData.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 (1)
libs/application/templates/aosh/work-accident-notification/src/lib/messages/externalData.ts (1)
31-31: Verify the removal of announcementHeadingSecond message.The removal of this message should be coordinated with the removal of its usage in the announcement section's form component.
Let's verify that there are no remaining references to this message:
✅ Verification successful
The removal of announcementHeadingSecond message is safe
The message is not referenced anywhere in the codebase, and the AnnouncementSection component only uses other heading messages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to the removed message ID rg -i "externalData\.dataProvider\.announcementHeadingSecond" \ -g '!*.{test,spec}.*' # Search for any remaining references to "secondHeading" in form components rg -i "secondHeading" \ -g '**/WorkAccidentNotificationForm/**/*.{ts,tsx}' \ -g '!*.{test,spec}.*'Length of output: 180
Script:
#!/bin/bash # Search for files containing both "announcement" and "heading" rg -l -i "announcement.*heading|heading.*announcement" \ -g '!*.{test,spec}.*' \ -g '**/work-accident-notification/**' # Find all files importing from externalData rg -l "from.*messages/externalData" \ -g '**/work-accident-notification/**' \ -g '!*.{test,spec}.*' # Search for any component using "heading" in the work accident notification template rg -i "heading" \ -g '**/work-accident-notification/**/*.{tsx,ts}' \ -g '!*.{test,spec}.*' \ -g '!**/messages/**'Length of output: 4325
jonarnarbriem
approved these changes
Jan 13, 2025
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 9 Passed Test ServicesThis report shows up to 10 services
|
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
...
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
Release Notes
Bug Fixes
Documentation
The changes focus on streamlining the user interface and clarifying messaging for the work accident notification process.