Bug 40948 - Transport cost matrix needs form validation
Summary: Transport cost matrix needs form validation
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-10-03 14:06 UTC by Brendan Lawlor
Modified: 2025-10-06 16:11 UTC (History)
3 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 40948: Add form validation to transport cost matrix (2.26 KB, patch)
2025-10-03 15:28 UTC, Brendan Lawlor
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Brendan Lawlor 2025-10-03 14:06:10 UTC
When testing bug 40655, I noticed that while there is an alert that warns you if you enter an invalid value in a cell "Cost must be expressed as a decimal number >= 0" it only runs onblur, but you can still submit the form with invalid values.

If you submit the form with an empty cell, you get an error "There were problems with your submission Invalid value for Centerville -> Fairfield" and the cell does not render an input anymore. It also let's you enter non numeric values which could cause  unexpected results.

To recreate:
1. Enable UseTransportCostMatrix and go to Administration > Transport cost matrix
2. Click in any cell
3. Uncheck the disabled box (don't click in the cell)
4. Click save
5. Notice the error "There were problems with your submission
Invalid value for Centerville -> Fairview"
6. Notice the form is broken for that cell
Comment 1 Brendan Lawlor 2025-10-03 15:28:44 UTC
Created attachment 187409 [details] [review]
Bug 40948: Add form validation to transport cost matrix
Comment 2 Brendan Lawlor 2025-10-03 15:38:35 UTC
Attaching this patch for feedback.

It replaces the js alert with html form validation using the pattern attribute and a regular expression. It sets the input as required so you cannot submit an empty value.

The pattern matches numeric values with up to two decimal places.

Allows:
0       - zero
100     - integers
0.5     - leading 0 before a demical place
.5      - single decimal place
20.75   - two decimal places

Rejects:
001     - leading zeros before integer
10.     - no digits after decimal
5.255   - three decimal places

I also thought about making this a number type input with step=".01"

Custom validation with Bootstrap might be nice too, but this was the simplest solution.
Comment 3 Brendan Lawlor 2025-10-03 16:01:54 UTC
Another thought is that we could still trigger the form validation on blur of the input so that the user would get immediate feedback for the field as soon as they finish editing it like:

$("body").on("blur", ".cost_input", function () {
    if ( ! $('#cost_matrix_form')[0].checkValidity() ) {
        $('#cost_matrix_form')[0].reportValidity();
    }
});
Comment 4 Lucas Gass (lukeg) 2025-10-06 15:19:31 UTC
Brendan, is this ready for testing?
Comment 5 Brendan Lawlor 2025-10-06 16:11:31 UTC
(In reply to Lucas Gass (lukeg) from comment #4)
> Brendan, is this ready for testing?

I didn't set this to needs sign off only because I'm not sure if this is the best solution, but it does work.

From a ux point of view I think the onblur listener actually is a good idea, especially since a user could be editing many cells incorrectly before they submit the form. So I had the thought to add the onblur listener back, but instead of the alert it could check if the form is valid. 

The matrix is a tricky form because it's so dense. I'm not sure if the default html validation pop ups function as well here as they do on a more normal looking form, but it does the basic job of preventing unwanted values.

I'd appreciate any feedback to consider the best way forward with this one. Thanks!