fix(regulations-admin): date format signature, remove self affect, disclaimer text #16288
Conversation
thordurhhh
added
the
deprecated:automerge
WalkthroughThe changes in this pull request involve several updates across multiple files within the regulations administration module. A new utility function 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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16288 +/- ##
==========================================
- Coverage 36.86% 36.86% -0.01%
==========================================
Files 6798 6796 -2
Lines 140482 140480 -2
Branches 39934 39935 +1
==========================================
- Hits 51792 51790 -2
Misses 88690 88690
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 6 Total Test Services: 0 Failed, 6 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: 0
🧹 Outside diff range and nitpick comments (6)
libs/portals/admin/regulations-admin/src/components/impacts/ImpactBaseSelection.tsx (1)
47-49: Approved: Good addition to prevent rendering empty select input.The new condition effectively prevents rendering the Select component when there are no options available. This improves the component's behavior and aligns with best practices for reusable components.
Consider adding a message or placeholder element when there are no options, to provide feedback to the user. For example:
if (!mentionedOptions.length) { return <Text>{t(impactMsgs.noOptionsAvailable)}</Text>; }This would enhance the user experience by explicitly communicating the lack of options.
libs/portals/admin/regulations-admin/src/utils/hooks.ts (1)
94-98: Consider adding a comment explaining the commented-out codeThe removal of the self-affect option aligns with the PR objectives. However, since the PR summary mentions keeping this available for testing purposes, it would be beneficial to add a comment explaining why this code is commented out and how it can be used for testing if needed.
Consider adding a comment like this above the commented-out code:
// Commented out for now as per PR #16288. Kept for potential testing purposes. // Uncomment to re-enable self-affect option if needed.This will provide context for future developers and align with the stated intention in the PR objectives.
libs/portals/admin/regulations-admin/src/utils/formatAmendingUtils.ts (1)
120-130: LGTM: Well-implementedformatDatefunction with a minor suggestion.The
formatDatefunction is well-implemented, adhering to TypeScript usage and providing a reusable utility for date formatting with Icelandic locale. It handles potential undefined dates and efficiently removes leading zeros.A minor suggestion for improvement:
Consider using optional chaining and nullish coalescing for a more concise implementation:
export const formatDate = (date: Date) => { return format(date, 'dd. MMMM yyyy', { locale: is }) .replace(/^0+/, '') // Remove leading zeros ?? ''; }This approach eliminates the need for the explicit
if-elsestatement and the creation of a newDateobject.libs/portals/admin/regulations-admin/src/components/EditSignature.tsx (1)
Line range hint
30-44: Approved: SimplifiedgetDefaultSignatureTextfunction with consistent date formatting.The changes to the
getDefaultSignatureTextfunction are well-implemented:
- The function signature is simplified by removing the
dateFormatterparameter.- The use of the imported
formatDatefunction ensures consistent date formatting across the application.These changes align with the PR objectives and improve the overall code quality.
Consider adding a brief comment explaining the purpose of the
formatDatefunction for better code documentation:// Format the current date for the signature text .replace('{dags}', formatDate(new Date()))libs/portals/admin/regulations-admin/src/utils/formatAmendingRegulation.ts (1)
Line range hint
89-91: Improved code organization with externalformatDatefunctionThe removal of the internal
formatDatefunction and use of the imported one improves code organization and reduces duplication. This change enhances maintainability and aligns with the principle of separation of concerns.Consider adding a type annotation for the
dateparameter to enhance type safety:formatDate(item.date as Date)This ensures that the
formatDatefunction receives the expected type and helps prevent potential runtime errors.libs/portals/admin/regulations-admin/src/lib/messages.ts (1)
412-412: Approved with a minor suggestion for clarity.The updated message provides clearer guidance on when and how to reference other regulations. This aligns well with the PR objectives of revising the disclaimer text.
To further enhance clarity, consider adding a brief example:
- 'ATH: Sé ætlunin að breyta annarri reglugerð, þarf að minnast á þá reglugerð með skýrum hætti í þessari stofnreglugerð.', + 'ATH: Sé ætlunin að breyta annarri reglugerð, þarf að minnast á þá reglugerð með skýrum hætti í þessari stofnreglugerð. Til dæmis: "Reglugerð nr. XXX/YYYY er hér með breytt sem hér segir: ..."',This addition provides a concrete example of how to reference another regulation, which could help users better understand and follow the guideline.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
- libs/portals/admin/regulations-admin/src/components/EditSignature.tsx (3 hunks)
- libs/portals/admin/regulations-admin/src/components/impacts/ImpactBaseSelection.tsx (1 hunks)
- libs/portals/admin/regulations-admin/src/lib/messages.ts (1 hunks)
- libs/portals/admin/regulations-admin/src/utils/formatAmendingRegulation.ts (1 hunks)
- libs/portals/admin/regulations-admin/src/utils/formatAmendingUtils.ts (2 hunks)
- libs/portals/admin/regulations-admin/src/utils/hooks.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
libs/portals/admin/regulations-admin/src/components/EditSignature.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."
libs/portals/admin/regulations-admin/src/components/impacts/ImpactBaseSelection.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."
libs/portals/admin/regulations-admin/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/portals/admin/regulations-admin/src/utils/formatAmendingRegulation.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/portals/admin/regulations-admin/src/utils/formatAmendingUtils.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/portals/admin/regulations-admin/src/utils/hooks.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 (9)
libs/portals/admin/regulations-admin/src/components/impacts/ImpactBaseSelection.tsx (1)
Line range hint
1-63: LGTM: Component adheres to coding guidelines and best practices.The
ImpactBaseSelectioncomponent is well-structured and follows the coding guidelines for files in thelibsdirectory:
- It uses TypeScript effectively for prop definitions and type exports.
- The component is designed for reusability across different NextJS apps.
- Its small size and focused functionality contribute to effective tree-shaking and bundling.
The component's implementation is clean and maintainable, making good use of hooks and external utilities.
libs/portals/admin/regulations-admin/src/utils/formatAmendingUtils.ts (2)
2-3: LGTM: New imports are appropriate and follow best practices.The new imports for the
formatfunction and Icelandic locale fromdate-fnsare correctly added. They support the newformatDatefunction and follow the existing import style. The specific imports also facilitate effective tree-shaking.
Line range hint
1-130: Confirms adherence to coding guidelines forlibs/**/*files.The changes in this file comply with the specified coding guidelines:
- The new
formatDatefunction is reusable across different NextJS apps.- TypeScript is used to define the input type of the function.
- The specific imports from
date-fnssupport effective tree-shaking and bundling practices.libs/portals/admin/regulations-admin/src/components/EditSignature.tsx (3)
23-23: LGTM: New import statement forformatDate.The addition of the
formatDateimport is appropriate for the changes made in thegetDefaultSignatureTextfunction. It follows good practices by importing only the necessary function and using a relative import path.
162-162: LGTM: UpdatedgetDefaultSignatureTextfunction call.The change in the
EditorInputcomponent is consistent with the updatedgetDefaultSignatureTextfunction signature. It correctly removes theformatDateFnsargument, simplifying the component usage while maintaining the existing functionality.
Line range hint
1-238: Overall assessment: Well-implemented changes that improve code quality and consistency.The modifications in this file successfully address the PR objectives related to updating the date format in the signature. The changes demonstrate good coding practices:
- Improved reusability by centralizing date formatting logic.
- Simplified API by removing unnecessary parameters.
- Consistent use of TypeScript for type definitions.
These updates enhance the maintainability and consistency of the regulations admin module.
To ensure that the changes don't introduce any unintended side effects, please run the following verification script:
✅ Verification successful
Verification Successful: All instances of
dateFormatterhave been removed, andgetDefaultSignatureTextis correctly implemented inEditSignature.tsx.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the formatDate function is used consistently across the codebase # Test: Search for any remaining instances of dateFormatter to ensure complete migration rg --type typescript 'dateFormatter' # Test: Check for any other usages of getDefaultSignatureText to ensure they've been updated rg --type typescript 'getDefaultSignatureText'Length of output: 153
Script:
#!/bin/bash # Description: Verify that the formatDate function is used consistently across the codebase # Test: Search for any remaining instances of dateFormatter to ensure complete migration rg 'dateFormatter' --glob '*.ts' --glob '*.tsx' # Test: Check for any other usages of getDefaultSignatureText to ensure they've been updated rg 'getDefaultSignatureText' --glob '*.ts' --glob '*.tsx'Length of output: 341
libs/portals/admin/regulations-admin/src/utils/formatAmendingRegulation.ts (2)
13-13: Improved modularity withformatDateimportThe addition of the
formatDateimport from a separate utility file enhances code modularity and reusability. This change aligns well with best practices for maintaining clean and efficient code structure.
Line range hint
1-589: Compliance with coding guidelines and best practicesThe changes in this file adhere to the coding guidelines for the
libsdirectory:
- The modifications maintain reusability across different NextJS apps.
- TypeScript is used consistently for type definitions and exports.
- The changes don't introduce new exports, preserving the existing interface which is good for tree-shaking.
The overall structure of the file remains consistent, and the changes improve modularity without compromising the existing functionality.
libs/portals/admin/regulations-admin/src/lib/messages.ts (1)
Line range hint
1-1024: File review summary: Meets coding guidelines and best practices.This file adheres to the coding guidelines for
libs/**/*:
- It supports reusability across different NextJS apps by exporting message descriptors.
- TypeScript is used effectively for defining props and exporting types.
- The structure of the file, with separate
defineMessagescalls, supports effective tree-shaking and bundling.The changes made improve the clarity of the regulations amendment process without compromising the file's overall structure or best practices.
thordurhhh
added
deprecated:automerge
…sclaimer text (#16288) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
…-pages (#16234) * Service portal removal. Add portals my pages * minor fixes * Fix * path fix * fix(portals-admin): locklist (#16279) * fix(portals-admin): locklist * tweak * msg id fix * tweak --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(service-portal): feature flag resolver for documents (#16285) * fix: def info and alert * feat: add feature flag to resolver * fix: move ff call to seperate function --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(vehicles-bulk-mileage): Fixes after testing review (#16295) * fix: testing fixes v1 * fix: testing comments v2 * fix: better message * fix: function name * fix: duplicate loading --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(tests): New @island/testing/e2e library (#16287) * Add @swc-node/register and @swc/core * Add testing/e2e library * update project.json for testing/e2e * fix import for libTestingE2e --------- Co-authored-by: Kristofer <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(parental-leave): ApplicationRights (#15901) * feat(parental-leave): ApplicationRights Added applicationRights to parental-leave when sending application. Since we are using a new way of calculating periods * Fix days used by period calculation * Tests for new periods * rename function with proper camelCase * Refactor: Made duplicate code into a function * Make ApplicationRights nullable * refactor: function instead of duplicate code * remove console.log * error handling for period data * clientConfig nullable fix * Fixes for calculation of months. And using clamp to get correct value of daysLeft * Multiply amount of months by 30 for period calculation with month durations * Fix old calculation of endDate with months --------- Co-authored-by: hfhelgason <[email protected]> Co-authored-by: veronikasif <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(passport-application): Updated readme (#16296) * updated readme * updated readme * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(regulations-admin): date format signature, remove self affect, disclaimer text (#16288) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(regulations-admin): No diff no addition in appendix (#16293) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(web): Global alert banner - Handle null case (#16298) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(web): Change custom syslumenn pages config for header (#16299) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(j-s): Digital mailbox API (#16301) * feat(j-s): Block create subpoena on staging and dev * Update subpoena.service.ts * fix(j-s): Fix mailbox API * remove changes not meant for this branch * Update subpoena.service.ts * fix(j-s): reverting changes from other branch * Update subpoena.response.ts * Update subpoena.response.ts * Update subpoena.response.ts * Update subpoena.response.ts --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(signature-collection): Fix list reviewed toggle (#16300) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * chore(scripts): Stricter shell script checking (#16242) * Set style level for shellcheck * Linting & formatting scripts * Remove _podman.sh script * Format all scripts * Add reviewdog/action-shfmt step * Configure shfmt * Merge from main * Linting * Move shfmt to before lint * Remove reviewdog * Allow external sources in shellcheck * Use Reviewdog for shellcheck * Set version for Reviewdog --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * chore(new-primary-school): Update messages namespace (#16302) Co-authored-by: veronikasif <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(driving-license): check if 65+ renewal is possible (#16292) * check if 65 renewal is possible * remove console log * cleanup * coderabbit tweaks * coderabbit changes * quick fix * add type? --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(service-portal): default defender and has chosen fields for subpoena (#16306) * fix: def info and alert * feat: add feature flag to resolver * fix: move ff call to seperate function * feat: add default choices ans has chosen + loading states * fix: use type * fix: undefined type issue * fix: simplify check * Update service setup for my pages infra * chore: charts update dirty files * Remove from infra * undo rename --------- Co-authored-by: albinagu <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: Ásdís Erna Guðmundsdóttir <[email protected]> Co-authored-by: Þorkell Máni Þorkelsson <[email protected]> Co-authored-by: Svanhildur Einarsdóttir <[email protected]> Co-authored-by: Kristofer <[email protected]> Co-authored-by: helgifr <[email protected]> Co-authored-by: hfhelgason <[email protected]> Co-authored-by: veronikasif <[email protected]> Co-authored-by: Rafn Árnason <[email protected]> Co-authored-by: andes-it <[email protected]> Co-authored-by: Rúnar Vestmann <[email protected]> Co-authored-by: mannipje <[email protected]> Co-authored-by: unakb <[email protected]> Co-authored-by: juni-haukur <[email protected]> Co-authored-by: birkirkristmunds <[email protected]> Co-authored-by: Kristján Albert <[email protected]>
What
Update date format in signature, remove self affect, update disclaimer text.
Commented out self affect removal kept in for purpose, we're not sure if we want to keep it, but it will be out for testing purposes.
Why
Updating after comments from review.
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation