Disallow negative sample amount #7209
Conversation
| { | ||
| providedValue = value + " (" + _sampleType.getMetricUnit() + ")"; | ||
| } | ||
| validateValue(col, value, providedValue); |
There was a problem hiding this comment.
a lot of the code in this PR is related to tracking / passing of the provided value. However, doesn't the error message end up as "Amounts must be non-negative." anyway? I don't see where it includes the provided or converted value? Seems like if that values isn't being used in the error message why worry about piping it through all of these places?
There was a problem hiding this comment.
The "Amounts must be non-negative." is appended (instead of replacing) to the default "Value '-1.1' for field 'Amount' is invalid." error. So the full error would be: "Value '-1.1' for field 'Amount' is invalid. Amounts must be non-negative.".
There was a problem hiding this comment.
I see. That makes more sense. Still a bummer that we have to pass through this provided value to so many places. Was the specifically requested? In the app editable grid, you see a different error because of the client side check (I assume, haven't tested). So would this converted value only show in the error for a file import case?
There was a problem hiding this comment.
The error is shown not just for file import, but also in UI edits. The app behavior is inconsistent. Editable grid would prevent you from submit because there is cell validation that fails for negative values. For bulk/detail edit, due to the change done in for https://www.labkey.org/issues/home/Developer/issues/details.view?issueId=53979 that uses text instead of numeric input type, we instead rely on server side validation. At the moment, bulk update will show user entered amount in the error message. But detail edit instead shows converted (sampleTypeUnit).
Because we are getting rid of row-by-row update code very soon, I didn't bother making the 2 code path consistent and went with the easier approach for row-by-row.
2 participants
Rationale
Related Pull Requests
Changes