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

psp-6864 missing fields required message. #3492

Merged
merged 2 commits into from
Sep 27, 2023

Conversation

devinleighsmith
Copy link
Collaborator

No description provided.

@github-actions
Copy link
Contributor

✅ No secrets were detected in the code.

@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

Merging #3492 (bc3f1f5) into dev (c415b0e) will increase coverage by 3.73%.
The diff coverage is 65.21%.

❗ Current head bc3f1f5 differs from pull request most recent head 00a4f72. Consider uploading reports for the commit 00a4f72 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #3492      +/-   ##
==========================================
+ Coverage   71.32%   75.05%   +3.73%     
==========================================
  Files        1343      881     -462     
  Lines       31681    17673   -14008     
  Branches     5960     4957    -1003     
==========================================
- Hits        22596    13265    -9331     
+ Misses       8840     4408    -4432     
+ Partials      245        0     -245     
Flag Coverage Δ
unittests 75.05% <65.21%> (+3.73%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...tend/src/features/leases/add/AddLeaseContainer.tsx 80.00% <ø> (ø)
...eatures/mapSideBar/acquisition/AcquisitionView.tsx 47.16% <ø> (ø)
...ideBar/acquisition/add/AddAcquisitionContainer.tsx 94.59% <ø> (ø)
...ation/update/UpdateCompensationRequisitionForm.tsx 66.29% <ø> (ø)
...ition/tabs/expropriation/form8/UpdateForm8Form.tsx 51.35% <100.00%> (+1.35%) ⬆️
...c/features/mapSideBar/project/ProjectContainer.tsx 85.71% <100.00%> (+1.50%) ⬆️
...res/mapSideBar/project/add/AddProjectContainer.tsx 100.00% <100.00%> (ø)
...features/mapSideBar/project/add/AddProjectForm.tsx 100.00% <100.00%> (ø)
...d/src/features/mapSideBar/shared/SidebarFooter.tsx 76.92% <100.00%> (+1.92%) ⬆️
...eBar/shared/update/properties/UpdateProperties.tsx 77.02% <ø> (ø)
... and 5 more

... and 462 files with indirect coverage changes

Comment on lines +61 to +62
await formikRef.current?.submitForm();
setIsValid(formikRef.current?.isValid ?? false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see other places doing:

var result = await (formikRef.current?.submitForm() ?? Promise.resolve());
setIsValid(...)

why not use the same pattern here (awaiting the promise)?

@@ -117,6 +118,7 @@ export const ResearchContainer: React.FunctionComponent<
if (formikRef !== undefined) {
formikRef.current?.setSubmitting(true);
formikRef.current?.submitForm();
setIsValid(formikRef.current?.isValid || false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Other code awaits the result of submitForm() but this one doesn't. Would it make sense to apply the same pattern here?

@github-actions
Copy link
Contributor

✅ No secrets were detected in the code.

1 similar comment
@github-actions
Copy link
Contributor

✅ No secrets were detected in the code.

@devinleighsmith devinleighsmith merged commit 26db0f6 into bcgov:dev Sep 27, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants