Skip to content
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

HL-642: Handler state check icons #2156

Merged
merged 5 commits into from
Aug 14, 2023
Merged

Conversation

mjturt
Copy link
Contributor

@mjturt mjturt commented Jul 23, 2023

Description ✨

In handling view, the check icons can now be clicked.

image

@codecov-commenter
Copy link

Codecov Report

Merging #2156 (03c5a99) into develop (059d184) will decrease coverage by 15.85%.
The diff coverage is 20.77%.

@@             Coverage Diff              @@
##           develop    #2156       +/-   ##
============================================
- Coverage    27.75%   11.90%   -15.85%     
============================================
  Files          759      163      -596     
  Lines        15484     4535    -10949     
  Branches      3454     1125     -2329     
============================================
- Hits          4297      540     -3757     
+ Misses       10618     3824     -6794     
+ Partials       569      171      -398     
Impacted Files Coverage Δ
...components/applicationReview/ApplicationReview.tsx 0.00% <0.00%> (ø)
...pplicationActions/useHandlingApplicationActions.ts 0.00% <ø> (ø)
...ents/applicationReview/benefitView/BenefitView.tsx 100.00% <ø> (ø)
...onNegotiationsView/CoOperationNegotiationsView.tsx 100.00% <ø> (ø)
...licationReview/companyInfoView/CompanyInfoView.tsx 78.57% <ø> (ø)
...tionReview/contactPersonView/ContactPersonView.tsx 87.50% <ø> (ø)
.../applicationReview/deminimisView/DeminimisView.tsx 95.65% <ø> (ø)
...ts/applicationReview/employeeView/EmployeeView.tsx 92.30% <ø> (ø)
...applicationReview/employmentView/EmpoymentView.tsx 92.85% <ø> (ø)
...pplicationReview/paySubsidyView/PaySubsidyView.tsx 68.75% <ø> (ø)
... and 6 more

... and 599 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link
Contributor

Helsinkibenefit-bf-hdlr is deployed to: https://helsinkibenefit-bf-hdlr-2156.test.kuva.hel.ninja 🚀🚀🚀

@github-actions
Copy link
Contributor

Helsinkibenefit-bf-appl is deployed to: https://helsinkibenefit-bf-appl-2156.test.kuva.hel.ninja 🚀🚀🚀

@github-actions
Copy link
Contributor

Helsinkibenefit-bf-bknd is deployed to: https://helsinkibenefit-bf-bknd-2156.test.kuva.hel.ninja 🚀🚀🚀

@github-actions
Copy link
Contributor

TestCafe result is success for https://helsinkibenefit-bf-appl-2156.test.kuva.hel.ninja! 😆🎉🎉🎉

@sirtawast
Copy link
Collaborator

Not sure why context / provider pattern is needed here as data is handled on application item level only and not needed globally (my guess at least?). Other than that, looks good to me! Good work 👍

@mjturt
Copy link
Contributor Author

mjturt commented Aug 10, 2023

Not sure why context / provider pattern is needed here as data is handled on application item level only and not needed globally (my guess at least?). Other than that, looks good to me! Good work +1

It's for the review state and related update function. Context provider reduces a lot of prop drilling here at least.

@mjturt mjturt requested a review from sirtawast August 10, 2023 16:36
{
onSuccess: () => {
void queryClient.invalidateQueries('reviewState');
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isUpdatingReviewStateError is used here on component level. I've previosly implemented all the error handling on hook level with onError callback. If toasting is the only thing done when an error occurs, I'd maybe use onError as it's more of a network / data error thing than a GUI feature

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can ditch the whole useEffect actually, if you use onError.

@mjturt mjturt merged commit ce5da72 into develop Aug 14, 2023
@mjturt mjturt deleted the HL-642-handler-check-icons branch August 14, 2023 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants