Bug 40655

Summary: Transport cost matrix doesn't save changes
Product: Koha Reporter: Lisette Scheer <lisette>
Component: System AdministrationAssignee: Lucas Gass (lukeg) <lucas>
Status: Needs documenting --- QA Contact: Brendan Lawlor <blawlor>
Severity: major    
Priority: P5 - low CC: baptiste.wojtkowski, blawlor, gmcharlt, kyle, lucas, mspinney
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40620
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40948
GIT URL: Initiative type: ---
Sponsorship status: --- Comma delimited list of Sponsors:
Crowdfunding goal: 0 Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This fixes a problem with the transport cost matrix where fields that were disabled could not be made enabled via the interface,
Version(s) released in:
25.11.00,25.05.05
Circulation function:
Attachments: Bug 40655: Fix logic of adding the disabled class
Bug 40655: Fix logic of adding the disabled class
Bug 40655: Fix logic of adding the disabled class

Description Lisette Scheer 2025-08-14 20:11:50 UTC
When editing transport-cost-matrix, it will save the numbers but not if you've enabled transfers.

It also doesn't reflect disabled/enabled correctly from the database, so if you go in and make changes it re-disables them. 

To recreate:

1. Turn on the UseTransportCostMatrix system preference
2. Got to Administration/transport cost matrix
3. Edit a number of boxes to add numbers and uncheck the 'disabled' box
4. save
5. Observe that all (or most?) of the boxes are still disabled.
6. Run a report: select * from transport_cost where cost !='0.00' 
7. Observe that they are disabled. 
8. in koha-mysql: update transport_cost set disable_transfer = 0 where cost !=0.00;
9. Repeat step 6, they should have 0 instead of 1 for disable_transfer
10. Navigate to the transport cost matrix (don't refresh the page!)
11. Observe that the boxes are still disabled
12. Save (no changes needed)
13. Repeat step 6
14. Observe they are once again marked as disable_transfer 1
Comment 1 Lucas Gass (lukeg) 2025-09-30 17:44:55 UTC
Created attachment 187117 [details] [review]
Bug 40655: Fix logic of adding the disabled class
Comment 2 Kyle M Hall (khall) 2025-10-01 15:54:20 UTC
Created attachment 187218 [details] [review]
Bug 40655: Fix logic of adding the disabled class

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 3 Brendan Lawlor 2025-10-02 20:12:15 UTC
This fixes the two issues described by Lisette, but there is still a bad bug with saving the matrix.

To recreate:
1. Apply the patch
2. Enable UseTransportCostMatrix and go to Administration > Transport cost matrix
3. Click in a box, uncheck disable, add a number and click save
   > The number saves as expected
4. In the same box click again, check disable and click save
  > Disabled is saved, but the number is not
5. In the same box click again, uncheck disabled and click save
  > This time when the page loads that cell can't be edited anymore
    and you get an error like:
    There were problems with your submission
    Invalid value for Centerville -> Fairfield
6. Refresh the page, same error and you still can't edit that cell. The user can break the ability to edit any cell like this.

For number 4, is the value supposed to be cleared when you disable it? 

The function check_transport_cost(e) only runs on blur. That doesn't prevent the user from clicking the save button when there's an empty value there and that breaks the way the form is rendered. 

This may be a separate bug , but even if the alert pops up, the user can just ignore it and put any string value and click save, it's not doing proper form validation.
Comment 4 Lucas Gass (lukeg) 2025-10-03 13:34:53 UTC
(In reply to Brendan Lawlor from comment #3)
> This fixes the two issues described by Lisette, but there is still a bad bug
> with saving the matrix.
> 
> To recreate:
> 1. Apply the patch
> 2. Enable UseTransportCostMatrix and go to Administration > Transport cost
> matrix
> 3. Click in a box, uncheck disable, add a number and click save
>    > The number saves as expected
> 4. In the same box click again, check disable and click save
>   > Disabled is saved, but the number is not
> 5. In the same box click again, uncheck disabled and click save
>   > This time when the page loads that cell can't be edited anymore
>     and you get an error like:
>     There were problems with your submission
>     Invalid value for Centerville -> Fairfield
> 6. Refresh the page, same error and you still can't edit that cell. The user
> can break the ability to edit any cell like this.
> 
> For number 4, is the value supposed to be cleared when you disable it? 
> 
> The function check_transport_cost(e) only runs on blur. That doesn't prevent
> the user from clicking the save button when there's an empty value there and
> that breaks the way the form is rendered. 
> 
> This may be a separate bug , but even if the alert pops up, the user can
> just ignore it and put any string value and click save, it's not doing
> proper form validation.

Hey Brendan,

I notice this behavior is also present as far back as 24.11.x. I also wondered if the value should be lost when we disable it. 

Since the bugfix in the patch is bad, libraries cannot edit their TCM, can we file this as a seperate bug and get this one in ASAP?
Comment 5 Brendan Lawlor 2025-10-03 13:56:46 UTC
Created attachment 187396 [details] [review]
Bug 40655: Fix logic of adding the disabled class

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 6 Brendan Lawlor 2025-10-03 14:08:33 UTC
I filed a new bug for the form validation issue: bug 40948
Comment 7 Lucas Gass (lukeg) 2025-10-06 15:16:48 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 8 Paul Derscheid 2025-10-23 21:11:02 UTC
Nice work everyone!

Pushed to 25.05.x
Comment 9 Baptiste Wojtkowski (bwoj) 2025-10-31 15:30:34 UTC
Code is correct in 24.11.x, not backported