feat(parental-leave): ApplicationRights #15901
Conversation
WalkthroughThe changes introduce enhancements to the parental leave module, including the addition of a new constant for parental leave rights descriptions in Icelandic, modifications to the 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
|
Datadog ReportAll test runs ✅ 70 Total Test Services: 0 Failed, 68 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (3) |
helgifr
force-pushed
the
feat/parental-leave-periods-applicationRights
branch
from
2a2591d to
a0124a4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Outside diff range and nitpick comments (4)
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.spec.ts (1)
285-398: Consistency in test descriptionsEnsure that the test descriptions accurately and consistently reflect the test cases. For example, standardize the capitalization in
'multiple Birth'to'multiple birth'for clarity.libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts (1)
900-908: Avoid redundant calculation ofrightsThe variable
rightsis derived fromrightsDTO, which is already available. Ensure that re-mappingrightsDTOto getrightsis necessary and optimized.If
rightsUnitis the only property needed, consider optimizing this step to improve performance for large datasets.libs/application/templates/parental-leave/src/lib/parentalLeaveUtils.ts (2)
257-276: Add unit tests for the new utility functionsThe newly added functions
getPersonalDays,getPersonalDaysInMonths, andgetTransferredDaysInMonthsare critical for calculating parental leave entitlements. To ensure their correctness and prevent future regressions, please add corresponding unit tests.Also applies to: 278-279, 424-427
271-274: Ensure variables are properly typed to avoid implicit any typeThe variables used in the calculation may be implicitly of type
any. Consider explicitly typingmaximumDaysToSpend,maximumAdditionalSingleParentDaysToSpend,maximumMultipleBirthsDaysToSpend, andtransferredDaysto enhance type safety.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- libs/application/template-api-modules/src/lib/modules/templates/parental-leave/constants.ts (1 hunks)
- libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.spec.ts (1 hunks)
- libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts (8 hunks)
- libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.utils.ts (3 hunks)
- libs/application/templates/parental-leave/src/lib/parentalLeaveUtils.ts (2 hunks)
- libs/clients/vmst/src/clientConfig.yaml (1 hunks)
- libs/clients/vmst/src/lib/utils.ts (3 hunks)
Additional context used
Path-based instructions (7)
libs/clients/vmst/src/lib/utils.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/template-api-modules/src/lib/modules/templates/parental-leave/constants.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/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.utils.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/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.spec.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/vmst/src/clientConfig.yaml (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/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.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/application/templates/parental-leave/src/lib/parentalLeaveUtils.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."
Biome
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.utils.ts
[error] 467-468: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
Additional comments not posted (12)
libs/clients/vmst/src/lib/utils.ts (3)
5-5: Importing 'ApplicationRights'The
ApplicationRightstype is correctly imported from'../../gen/fetch', enabling its usage within this module.
51-51: Adding 'applicationRights' to 'Init' typeIncluding
applicationRights: ApplicationRights[]in theInittype extends the data structure to accommodate application rights, which aligns with the PR objective of integrating new calculation methods.
84-84: Ensure 'applicationRights' does not expose sensitive data in logsAdding
applicationRightsto the list of picked attributes for logging might unintentionally include sensitive information. Please verify that the fields withinapplicationRightsare safe to log and do not contain Personally Identifiable Information (PII) or confidential data.Run the following script to inspect the fields of
ApplicationRightsand ensure they do not contain sensitive data:Verification successful
Logging 'applicationRights' appears safe, but exercise caution
Based on the examination of the 'ApplicationRights' model, it doesn't contain inherently sensitive information. The fields (rightsUnit, rightsDescription, months, days, daysLeft) are general descriptors and time-related data, not personal or confidential information.
However, to maintain best practices in data privacy:
- Consider logging only necessary fields from 'applicationRights' rather than the entire object.
- Conduct a runtime review to ensure no sensitive information is inadvertently stored in these fields during actual use.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Inspect the fields of 'ApplicationRights' to verify they do not contain sensitive information. # Search for the definition of 'ApplicationRights' interface ast-grep --lang ts --pattern 'export interface ApplicationRights { $$$ }'Length of output: 75
Script:
#!/bin/bash # Search for any mention of 'ApplicationRights' in the codebase rg "ApplicationRights" -n # Search for type definitions or interfaces that might be related to 'ApplicationRights' ast-grep --lang ts --pattern 'type $_ = { $$$ applicationRights $$$ }' ast-grep --lang ts --pattern 'interface $_ { $$$ applicationRights $$$ }' # Search for usages of 'applicationRights' to understand its structure rg "applicationRights" -n -A 5Length of output: 10559
Script:
#!/bin/bash # View the contents of the ApplicationRights model cat libs/api/domains/directorate-of-labour/src/models/applicationRights.model.ts # Search for usage of ApplicationRights in logging contexts rg "ApplicationRights.*log" -n # Search for comments near ApplicationRights that might indicate sensitivity rg "ApplicationRights" -n -C 3 | rg "sensitive|private|confidential" -nLength of output: 513
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.utils.ts (4)
31-31: ImportingApplicationRightsThe addition of
ApplicationRightsto the imports ensures that the type is available for use within the module. This aligns with the PR objective of integrating application rights into the parental leave application process.
375-375: Updating Function Signature withapplicationRightsThe
transformApplicationToParentalLeaveDTOfunction now includes an optional parameterapplicationRights?: ApplicationRights[]. This enhancement allows the function to handle application rights, supporting the new calculation method as per the PR objectives.
468-468: IncludingapplicationRightsin Return ObjectAdding
applicationRightsto the return object integrates the application rights into the DTO, which is essential for the new calculation method. This change is appropriate and necessary for the functionality enhancement.Tools
Biome
[error] 467-468: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
375-375: Verify Function Calls are UpdatedWith the updated function signature, please ensure that all calls to
transformApplicationToParentalLeaveDTOthroughout the codebase are updated to include the newapplicationRightsparameter where necessary.You can run the following script to identify all usages of the function and check if they include the
applicationRightsparameter:libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.spec.ts (4)
296-297: Ensure correct data types formultipleBirthsRequestDaysandmultipleBirthsThe properties
multipleBirthsRequestDaysandmultipleBirthsare set with numeric values. Please verify that these fields are intended to be numbers and that downstream code handles these numeric values correctly.
304-322: Validate the calculations fordays,daysLeft, andmonthsin test expectationsThe expected values for
days,daysLeft, andmonthsshould accurately reflect the application's logic. For instance, verify thatmonths: '12'corresponds correctly todays: '180'in the context of the rights being tested.
348-350: Confirm accuracy ofdaysLeftandmonthsfor multiple birthsIn the test case for multiple births, ensure that the
daysLeftandmonthsvalues correctly represent the calculations based onmultipleBirthsRequestDays. Specifically, check thatdaysLeft: '34'andmonths: '3'are accurate fordays: '79'.
375-380: Add missing properties in the period objectThe period object set at
answers.periods[0]lacks theendDateproperty. This might lead to issues when the application processes periods without anendDate.Apply this change to include the missing
endDate:set(application, 'answers.periods[0]', { ratio: '100', useLength: 'no', startDate: '2025-03-12', + endDate: '2025-09-14', })Likely invalid or redundant comment.
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts (1)
93-93: ImportedparseISOfor date parsingThe import of
parseISOfrom 'date-fns' is appropriate for parsing ISO date strings.
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/constants.ts
Show resolved
Hide resolved
...cation/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.utils.ts
Show resolved
Hide resolved
...template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.spec.ts
Outdated
Show resolved
Hide resolved
...template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.spec.ts
Show resolved
Hide resolved
libs/application/templates/parental-leave/src/lib/parentalLeaveUtils.ts
Outdated
Show resolved
Hide resolved
libs/application/templates/parental-leave/src/lib/parentalLeaveUtils.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
Files selected for processing (2)
- libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts (8 hunks)
- libs/application/templates/parental-leave/src/lib/parentalLeaveUtils.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/application/templates/parental-leave/src/lib/parentalLeaveUtils.ts
Additional context used
Path-based instructions (1)
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.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."
...tion/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts (8 hunks)
Additional context used
Path-based instructions (1)
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.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 not posted (1)
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts (1)
827-841: Ensure proper error handling for invalid inputs incalculatePeriodDaysThe
calculatePeriodDaysfunction lacks input validation forstartDate,endDate, andratio. Invalid or improperly formatted inputs could lead to unexpected errors, such asInvalid Dateobjects orNaNvalues.
...tion/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts
Outdated
Show resolved
Hide resolved
...tion/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.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
Files selected for processing (2)
- libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts (8 hunks)
- libs/clients/vmst/src/clientConfig.yaml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/clients/vmst/src/clientConfig.yaml
Additional context used
Path-based instructions (1)
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.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."
Biome
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts
[error] 838-838: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
[error] 838-838: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
[error] 842-842: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
Additional comments not posted (2)
libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts (2)
754-768: Great job eliminating code duplication withaddMultipleBirthsRightsRefactoring repeated logic into the
addMultipleBirthsRightshelper function improves code maintainability and clarity.
878-892: Successfully refactored shared logic intopreparePeriodsAndRightsDTOExtracting common code into
preparePeriodsAndRightsDTOenhances modularity and reduces duplication acrosssendApplicationandvalidateApplicationmethods.
...tion/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts
Show resolved
Hide resolved
helgifr
force-pushed
the
feat/parental-leave-periods-applicationRights
branch
from
8b84e0b to
65c69d7
Compare
...tion/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts
Outdated
Show resolved
Hide resolved
...tion/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts
Show resolved
Hide resolved
...tion/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts
Outdated
Show resolved
Hide resolved
helgifr
force-pushed
the
feat/parental-leave-periods-applicationRights
branch
from
87138b8 to
bdfc6b0
Compare
Added applicationRights to parental-leave when sending application. Since we are using a new way of calculating periods
helgifr
force-pushed
the
feat/parental-leave-periods-applicationRights
branch
from
712b849 to
217edc9
Compare
veronikasif
added
the
deprecated:automerge
* 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>
…-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]>
Added applicationRights to parental-leave when sending application. Since we are using a new way of calculating periods
...
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
Refactor