PHACDataHub / cpho-phase2

A data collection and retrieval application to automate and standardize the data intake for the yearly Health of Canadians report
5 stars 3 forks source link

fix: ignore form validation on deleted rows. #301

Closed msarar closed 6 months ago

msarar commented 6 months ago

forms affected: Benchmarking Form Trend Form Indicator Datum Form

AlexCLeduc commented 6 months ago

If I delete the year in the trend section, I get a 500:

null value in column "year" of relation "cpho_trendanalysis" violates not-null constraint DETAIL: Failing row contains (8, null, 39.8, 1.63598, null, , t, 2024-05-01 14:09:28.794161, 2, null, null, caution, ).

It looks like it might be saving the form despite it being deleted? Should we prevent this form from being saved, or relax the constraint? I think the former is preferred.

Kind of unrelated, but I also noticed you can save an empty string as an age-group

msarar commented 6 months ago

If I delete the year in the trend section, I get a 500:

null value in column "year" of relation "cpho_trendanalysis" violates not-null constraint DETAIL: Failing row contains (8, null, 39.8, 1.63598, null, , t, 2024-05-01 14:09:28.794161, 2, null, null, caution, ).

It looks like it might be saving the form despite it being deleted? Should we prevent this form from being saved, or relax the constraint? I think the former is preferred.

Kind of unrelated, but I also noticed you can save an empty string as an age-group

Good catch! This is probably because we don't have a form level validation for empty values on the year field but a "not null" DB constraint. As it currently stands if we remove the year and save it, it would still give us the same error even if we didn't check the delete checkbox. The way we have the current delete set up is that it never actually deletes anything (for the audit trail) it just doesn't show the data if deleted, using the active_objects manager.

I think the best solution for this one is to add a form level field validation to the field and allow nulls to be saved in the db