bcgov / silviculture-ipc

Forestry Sector Operator Screening for Infection Prevention and Control Protocols. Being replaced by: https://github.com/bcgov/common-forms-toolkit
https://github.com/bcgov/common-forms-toolkit
Apache License 2.0
1 stars 8 forks source link

Operation Type - temporary fix #111

Closed TimCsaky closed 4 years ago

TimCsaky commented 4 years ago

Description

Types of changes

New feature (non-breaking change which adds functionality)

Checklist

Further comments

loneil commented 4 years ago

There's a potential edge case for this (I just tested it out on this PR). The API and DB are updated, but the user is still holding the form in their browser without the operation type filled in. If this happens and the field is filled as NULL then it appears to fail loading on the submission page for the application.

Now, I'm not clear on in this case if the Model default will handle it in sequelize and set that to Silv, but that might matter depending on it what's posted has '' for the type value, or doesn't pass the field to the API from the front end if it's not present.

Regardless, it's probably best, since this is a nullable field, to make sure the Submission admin page can handle the case where it's null to be safe. Otherwise it's an unrecoverable state with only a client side error. The PDF seems to be fine in this case. Can see what I mean at https://silviculture-ipc-dev.pathfinder.gov.bc.ca/pr-111/#/admin/submission/9a471fde-bd0d-4a35-86f2-25a3a7e3eb3d

Also should bump the form version number for this change, that should be done for any field changes like this (it's what we can use to troubleshoot problems like above)

TimCsaky commented 4 years ago

There's a potential edge case for this (I just tested it out on this PR). The API and DB are updated, but the user is still holding the form in their browser without the operation type filled in. If this happens and the field is filled as NULL then it appears to fail loading on the submission page for the application.

Now, I'm not clear on in this case if the Model default will handle it in sequelize and set that to Silv, but that might matter depending on it what's posted has '' for the type value, or doesn't pass the field to the API from the front end if it's not present.

Regardless, it's probably best, since this is a nullable field, to make sure the Submission admin page can handle the case where it's null to be safe. Otherwise it's an unrecoverable state with only a client side error. The PDF seems to be fine in this case. Can see what I mean at https://silviculture-ipc-dev.pathfinder.gov.bc.ca/pr-111/#/admin/submission/9a471fde-bd0d-4a35-86f2-25a3a7e3eb3d

Also should bump the form version number for this change, that should be done for any field changes like this (it's what we can use to troubleshoot problems like above)

good catch!