-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(parental-leave): Refactor Sync - VMST #14436
Conversation
…tion in EDIT_OR_ADD_EMPLOYERS_AND_PERIODS
WalkthroughThe changes focus on enhancing period handling and synchronization in the Parental Leave application. Updates include removing the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Datadog ReportAll test runs ✅ 4 Total Test Services: 0 Failed, 4 Passed Test Services
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14436 +/- ##
==========================================
+ Coverage 36.92% 36.93% +0.01%
==========================================
Files 6322 6322
Lines 128754 128732 -22
Branches 36738 36754 +16
==========================================
+ Hits 47536 47552 +16
+ Misses 81218 81180 -38
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 5 Total Test Services: 0 Failed, 5 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (1)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (19)
- libs/api/domains/directorate-of-labour/src/lib/directorate-of-labour.resolver.ts (1 hunks)
- libs/api/domains/directorate-of-labour/src/models/applicationInformationPeriod.model.ts (1 hunks)
- libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts (1 hunks)
- libs/application/templates/parental-leave/src/constants.ts (3 hunks)
- libs/application/templates/parental-leave/src/fields/EditOrAddEmployersAndPeriodsReview/review-groups/Periods.tsx (1 hunks)
- libs/application/templates/parental-leave/src/fields/PeriodsRepeater/index.tsx (2 hunks)
- libs/application/templates/parental-leave/src/fields/Review/review-groups/Periods.tsx (1 hunks)
- libs/application/templates/parental-leave/src/fields/components/Timeline/Panel.tsx (3 hunks)
- libs/application/templates/parental-leave/src/fields/components/Timeline/Timeline.tsx (1 hunks)
- libs/application/templates/parental-leave/src/forms/EditsRequireAction.ts (2 hunks)
- libs/application/templates/parental-leave/src/graphql/queries.ts (2 hunks)
- libs/application/templates/parental-leave/src/lib/ParentalLeaveTemplate.ts (15 hunks)
- libs/application/templates/parental-leave/src/lib/answerValidationSections/validateLatestPeriodValidationSection.ts (1 hunks)
- libs/application/templates/parental-leave/src/lib/messages.ts (2 hunks)
- libs/application/templates/parental-leave/src/lib/parentalLeaveTemplateUtils.ts (2 hunks)
- libs/application/templates/parental-leave/src/lib/parentalLeaveUtils.spec.ts (1 hunks)
- libs/application/templates/parental-leave/src/lib/parentalLeaveUtils.ts (6 hunks)
- libs/application/templates/parental-leave/src/types.ts (2 hunks)
- libs/clients/vmst/src/clientConfig.yaml (1 hunks)
Files skipped from review due to trivial changes (3)
- libs/application/templates/parental-leave/src/fields/EditOrAddEmployersAndPeriodsReview/review-groups/Periods.tsx
- libs/application/templates/parental-leave/src/fields/Review/review-groups/Periods.tsx
- libs/application/templates/parental-leave/src/lib/parentalLeaveUtils.spec.ts
Additional Context Used
Path-based Instructions (16)
libs/api/domains/directorate-of-labour/src/models/applicationInformationPeriod.model.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/graphql/queries.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/fields/components/Timeline/Timeline.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/application/templates/parental-leave/src/forms/EditsRequireAction.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/types.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/fields/components/Timeline/Panel.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/application/templates/parental-leave/src/lib/answerValidationSections/validateLatestPeriodValidationSection.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/parentalLeaveTemplateUtils.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/directorate-of-labour/src/lib/directorate-of-labour.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/application/templates/parental-leave/src/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/templates/parental-leave/src/fields/PeriodsRepeater/index.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/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."
libs/application/templates/parental-leave/src/lib/ParentalLeaveTemplate.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/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."
Additional comments not posted (39)
libs/application/templates/parental-leave/src/graphql/queries.ts (1)
46-47
: Ensure the new fieldsdays
andapproved
are handled correctly in the resolver.Verify that the resolver for the
GetApplicationInformation
query correctly handles the newdays
andapproved
fields.libs/application/templates/parental-leave/src/forms/EditsRequireAction.ts (1)
7-7
: Good use ofDefaultEvents
for consistency.Using
DefaultEvents
improves consistency and maintainability.libs/application/templates/parental-leave/src/fields/components/Timeline/Panel.tsx (2)
4-4
: Good addition of theTooltip
component for better user experience.The addition of the
Tooltip
component enhances the user experience by providing additional context.
54-110
: Ensure the new period state handling logic is thoroughly tested.Verify that the new logic for handling different period states (deletable, paid, in progress) is thoroughly tested to ensure correct behavior.
libs/application/templates/parental-leave/src/lib/answerValidationSections/validateLatestPeriodValidationSection.ts (1)
160-164
: Ensure the new condition for VMST-synced periods is thoroughly tested.Verify that the new condition to stop checking periods synced from VMST is thoroughly tested to ensure correct behavior.
libs/application/templates/parental-leave/src/lib/parentalLeaveTemplateUtils.ts (1)
97-142
: Ensure the newrestructureVMSTPeriods
function is thoroughly tested.Verify that the new
restructureVMSTPeriods
function is thoroughly tested to ensure it correctly restructures VMST periods.libs/api/domains/directorate-of-labour/src/lib/directorate-of-labour.resolver.ts (11)
Line range hint
14-21
: ThegetApplicationInformation
method looks good and follows the expected logic.
Line range hint
23-30
: ThegetParentalLeavesEntitlements
method looks good and follows the expected logic.
Line range hint
32-37
: ThegetParentalLeaves
method looks good and follows the expected logic.
Line range hint
39-46
: ThegetParentalLeavesEstimatedPaymentPlan
method looks good and follows the expected logic.
Line range hint
48-55
: ThegetParentalLeavesApplicationPaymentPlan
method looks good and follows the expected logic.
Line range hint
57-63
: ThegetParentalLeavesPeriodEndDate
method looks good and follows the expected logic.
Line range hint
65-71
: ThegetParentalLeavesPeriodLength
method looks good and follows the expected logic.
Line range hint
73-77
: ThegetUnions
method looks good and follows the expected logic.
Line range hint
79-83
: ThegetPensionFunds
method looks good and follows the expected logic.
Line range hint
85-89
: ThegetPrivatePensionFunds
method looks good and follows the expected logic.
Line range hint
91-95
: ThegetPregnancyStatus
method looks good and follows the expected logic.libs/application/templates/parental-leave/src/constants.ts (3)
Line range hint
33-36
: The removal of theMODIFY
event fromPLEvents
looks good and aligns with the refactoring objectives.
Line range hint
38-45
: The removal of theMODIFY
event type fromEvents
looks good and aligns with the refactoring objectives.
69-69
: The addition of thesetVMSTPeriods
action toApiModuleActions
looks good and aligns with the new functionality.libs/application/templates/parental-leave/src/fields/PeriodsRepeater/index.tsx (2)
74-74
: The change to useskip: !shouldCall
in theuseQuery
call simplifies the logic and looks good.
88-88
: The addition of the ESLint comment to theuseEffect
dependency array is appropriate and looks good.libs/clients/vmst/src/clientConfig.yaml (1)
436-437
: The addition of thedays
field to theApplicationPeriod
schema looks good and aligns with the new functionality.libs/application/templates/parental-leave/src/lib/parentalLeaveUtils.ts (4)
160-177
: Ensurepaid
andapproved
fields are correctly handled.Verify that the
paid
andapproved
fields are correctly used in the rest of the codebase whereformatPeriods
is called.
573-577
: EnsureVMSTPeriods
is correctly integrated and used.Verify that the
VMSTPeriods
field is correctly used in the rest of the codebase wheregetApplicationExternalData
is called.Also applies to: 588-588
1571-1573
: EnsuredaysToUse
,paid
, andapproved
fields are correctly handled.Verify that the
daysToUse
,paid
, andapproved
fields are correctly used in the rest of the codebase wheresynchronizeVMSTPeriods
is called.Verification successful
Ensure
daysToUse
,paid
, andapproved
fields are correctly handled.The
synchronizeVMSTPeriods
function includes thedaysToUse
,paid
, andapproved
fields in the period object. These fields are used within the function to determine the synchronization logic for periods.
daysToUse
: This field is directly assigned fromperiod.days
.paid
: This field is directly assigned fromperiod.paid
and is used in conditional checks to determine if the period should be added tonewPeriods
.approved
: This field is directly assigned fromperiod.approved
and is used in conditional checks to determine if synchronization is necessary.The fields are correctly handled within the
synchronizeVMSTPeriods
function.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of `daysToUse`, `paid`, and `approved` fields in the codebase. # Test: Search for the usage of `synchronizeVMSTPeriods` function. Expect: Correct handling of `daysToUse`, `paid`, and `approved` fields. rg --type typescript 'synchronizeVMSTPeriods'Length of output: 80
Script:
#!/bin/bash # Description: Verify usage of `daysToUse`, `paid`, and `approved` fields in the codebase. # Test: Search for the usage of `synchronizeVMSTPeriods` function. Expect: Correct handling of `daysToUse`, `paid`, and `approved` fields. rg --type ts 'synchronizeVMSTPeriods'Length of output: 361
Script:
#!/bin/bash # Description: Extract relevant lines from `PeriodsRepeater/index.tsx` to verify handling of `daysToUse`, `paid`, and `approved` fields. # Test: Extract lines around the usage of `synchronizeVMSTPeriods` function. rg --type ts -A 10 -B 10 'synchronizeVMSTPeriods' libs/application/templates/parental-leave/src/fields/PeriodsRepeater/index.tsxLength of output: 1319
Script:
#!/bin/bash # Description: Extract the implementation of `synchronizeVMSTPeriods` to verify handling of `daysToUse`, `paid`, and `approved` fields. # Test: Extract the implementation of `synchronizeVMSTPeriods` function. ast-grep --lang ts --pattern $'export const synchronizeVMSTPeriods = ($_$) => { $$$ }' libs/application/templates/parental-leave/src/lib/parentalLeaveUtils.tsLength of output: 161
Script:
#!/bin/bash # Description: Extract the implementation of `synchronizeVMSTPeriods` to verify handling of `daysToUse`, `paid`, and `approved` fields. # Test: Extract the implementation of `synchronizeVMSTPeriods` function. ast-grep --lang ts --pattern 'export const synchronizeVMSTPeriods = ($$$) => { $$$ }' libs/application/templates/parental-leave/src/lib/parentalLeaveUtils.tsLength of output: 15040
1646-1648
: Ensure synchronization logic correctly handles new fields.Verify that the synchronization logic correctly handles the new
paid
andapproved
fields.Verification successful
Synchronization logic correctly handles
paid
andapproved
fieldsThe
synchronizeVMSTPeriods
function inlibs/application/templates/parental-leave/src/lib/parentalLeaveUtils.ts
includes logic to handle thepaid
andapproved
fields. The function creates objects that include these fields and uses them in conditional checks to determine how periods are synchronized.
- The
paid
field is included in the object created for each period.- The
approved
field is also included in the object created for each period.- The function contains logic that uses the
paid
field to decide whether to push the period object into thenewPeriods
array.Thus, the synchronization logic appears to correctly handle the new
paid
andapproved
fields.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify synchronization logic for `paid` and `approved` fields. # Test: Search for the synchronization logic in `synchronizeVMSTPeriods` function. Expect: Correct handling of `paid` and `approved` fields. rg --type typescript 'synchronizeVMSTPeriods'Length of output: 80
Script:
#!/bin/bash # Description: Verify synchronization logic for `paid` and `approved` fields. # Test: Search for the synchronization logic in `synchronizeVMSTPeriods` function. Expect: Correct handling of `paid` and `approved` fields. rg 'synchronizeVMSTPeriods' --glob '*.ts' # If the function is found, display its implementation. if rg -q 'synchronizeVMSTPeriods' --glob '*.ts'; then rg 'synchronizeVMSTPeriods' --glob '*.ts' -A 20 fiLength of output: 2545
Script:
#!/bin/bash # Description: Verify synchronization logic for `paid` and `approved` fields. # Test: Search for the synchronization logic in `synchronizeVMSTPeriods` function. Expect: Correct handling of `paid` and `approved` fields. rg 'synchronizeVMSTPeriods' --glob '*.ts' # If the function is found, display its implementation. if rg -q 'synchronizeVMSTPeriods' --glob '*.ts'; then rg 'synchronizeVMSTPeriods' --glob '*.ts' -A 50 fiLength of output: 5710
libs/application/templates/parental-leave/src/lib/ParentalLeaveTemplate.ts (9)
569-575
: The addition of thesetVMSTPeriods
action in theonExit
array of theVINNUMALASTOFNUN_APPROVAL
state looks good.
873-879
: The addition of thesetVMSTPeriods
action in theonExit
array of theAPPROVED
state looks good.
1055-1060
: The addition of thesetVMSTPeriods
action in theonExit
array of theEMPLOYER_WAITING_TO_ASSIGN_FOR_EDITS
state looks good.
1112-1117
: The addition of thesetVMSTPeriods
action in theonExit
array of theEMPLOYER_APPROVE_EDITS
state looks good.
1212-1217
: The addition of thesetVMSTPeriods
action in theonExit
array of theEMPLOYER_EDITS_ACTION
state looks good.
1306-1317
: The addition of thesetVMSTPeriods
action in theonExit
array of theVINNUMALASTOFNUN_APPROVE_EDITS
state looks good.
1386-1391
: The addition of thesetVMSTPeriods
action in theonExit
array of theVINNUMALASTOFNUN_EDITS_ACTION
state looks good.
944-944
: The addition of thesetVMSTPeriods
action in theentry
array of theEDIT_OR_ADD_EMPLOYERS_AND_PERIODS
state looks good.
2098-2114
: ThesetVMSTPeriods
action function is correctly implemented to copy VMST periods to the application's periods. Ensure that therestructureVMSTPeriods
utility function is thoroughly tested.libs/application/templates/parental-leave/src/lib/messages.ts (3)
915-919
: New message definition for deleting a period looks good.
920-929
: New message definitions for paid periods and periods in progress look good.
1931-1935
: New message definition for VMST periods looks good.
...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
libs/api/domains/directorate-of-labour/src/models/applicationInformationPeriod.model.ts
Show resolved
Hide resolved
libs/application/templates/parental-leave/src/fields/components/Timeline/Timeline.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
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 (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/application/template-api-modules/src/lib/modules/templates/parental-leave/parental-leave.service.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- libs/application/templates/parental-leave/src/constants.ts (3 hunks)
- libs/application/templates/parental-leave/src/lib/ParentalLeaveTemplate.ts (12 hunks)
Files skipped from review as they are similar to previous changes (2)
- libs/application/templates/parental-leave/src/constants.ts
- libs/application/templates/parental-leave/src/lib/ParentalLeaveTemplate.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great job!
* Small change after merge main * Changed MODIFY event to EDIT and added tiggerEvent to validateApplication in EDIT_OR_ADD_EMPLOYERS_AND_PERIODS * Removed shouldNotCall from getApplicationInformation query, use skip instead * [UUFV-415] Refactor Sync - VMST * Removed unused comments and made some changes * Fix SummaryTimeline label * Fixed merge conflict - 2 * Updated sync - use days from VMST and removed comments * Changes requested by coderabbitai * Updated setVMSTPeriods action so it only updates periods in specific states --------- Co-authored-by: karenbjorg <[email protected]>
* Small change after merge main * Changed MODIFY event to EDIT and added tiggerEvent to validateApplication in EDIT_OR_ADD_EMPLOYERS_AND_PERIODS * Removed shouldNotCall from getApplicationInformation query, use skip instead * [UUFV-415] Refactor Sync - VMST * Removed unused comments and made some changes * Fix SummaryTimeline label * Fixed merge conflict - 2 * Updated sync - use days from VMST and removed comments * Changes requested by coderabbitai * Updated setVMSTPeriods action so it only updates periods in specific states --------- Co-authored-by: karenbjorg <[email protected]>
* Small change after merge main * Changed MODIFY event to EDIT and added tiggerEvent to validateApplication in EDIT_OR_ADD_EMPLOYERS_AND_PERIODS * Removed shouldNotCall from getApplicationInformation query, use skip instead * [UUFV-415] Refactor Sync - VMST * Removed unused comments and made some changes * Fix SummaryTimeline label * Fixed merge conflict - 2 * Updated sync - use days from VMST and removed comments * Changes requested by coderabbitai * Updated setVMSTPeriods action so it only updates periods in specific states --------- Co-authored-by: karenbjorg <[email protected]>
[TS-336] Refactor Sync - VMST
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
days
field to application information.setVMSTPeriods
action for managing VMST periods.Enhancements
Bug Fixes
shouldNotCall
parameter.User Interface
Documentation
Internal Improvements