To test: 1) Add a new circulation rule, give 4.50 for overdue fines cap. > Verify the system saves it to be 5.00 2) Edit an existing circulation rule, give 3.40 for overdue fines cap. > Verify the system saves it to be 3.00 Obviously, that's not good and especially bad because most of our libraries have standardized on 4.50. Maybe there is a problem with the datatype definition in the kohastructure.sql? We have: overduefinescap decimal default NULL, -- the maximum amount of an overdue fine In opposite to: reservecharge decimal(28,6) default NULL, I couldn't test it, but the definition is the same for kohastructure and update database. So a bit worried that we might change the data here on update instead of copying it correctly from the system preference into issuingrules. The feature was added with bug 7420.
Nice catch. Also, oops. Also, we really ought to be counting everything in pennies and not use decimal data types for currencies. Regardless, I think altering the column type to decimal(28,6) should do the trick and preserve existing values: MariaDB [koha]> create table tmp (overduefinescap decimal null); Query OK, 0 rows affected (0.05 sec) MariaDB [koha]> insert into tmp values (1.25); Query OK, 1 row affected, 1 warning (0.00 sec) MariaDB [koha]> select * from tmp; +-----------------+ | overduefinescap | +-----------------+ | 1 | +-----------------+ 1 row in set (0.00 sec) MariaDB [koha]> alter table tmp modify column overduefinescap decimal(28,6) null; Query OK, 1 row affected (0.06 sec) Records: 1 Duplicates: 0 Warnings: 0 MariaDB [koha]> select * from tmp; +-----------------+ | overduefinescap | +-----------------+ | 1.000000 | +-----------------+ 1 row in set (0.00 sec)
Created attachment 19325 [details] [review] Bug 10490: Overdue fines cap can't store decimal values When overduefinescap was added to the issuingrules the datatype given was decimal. This translates in MySQL to decimal(10,0). This doesn't allow you to store decimal values and therefore values like 4.5 are saved as 5 in the database. To test: On a current installation: 1) Try to enter 4.5 as Overdue fines cap. Verify that the value is not correctly saved. 2) Apply patch and run database update. 3) Try adding/changing an issuing rule setting Overdue fines cap to 4.5 again. 4) Verify the value is saved correctly. Create a new Koha installation from scratch: 1) Verify that the issuingrules table has been created correctly and that you can add/mofidy issuingrules correctly. Because this bug can create data loss, the old database update has also been changed to avoid this problem for people updating at a later point in time. Checkout an older version of Koha pre 3.09.00.027. 1) Run the database updates. 2) Verify again, that adding/modifying issuingrules works correctly.
Created attachment 19510 [details] [review] [SIGNED-OFF] Bug 10490: Overdue fines cap can't store decimal values When overduefinescap was added to the issuingrules the datatype given was decimal. This translates in MySQL to decimal(10,0). This doesn't allow you to store decimal values and therefore values like 4.5 are saved as 5 in the database. To test: On a current installation: 1) Try to enter 4.5 as Overdue fines cap. Verify that the value is not correctly saved. 2) Apply patch and run database update. 3) Try adding/changing an issuing rule setting Overdue fines cap to 4.5 again. 4) Verify the value is saved correctly. Create a new Koha installation from scratch: 1) Verify that the issuingrules table has been created correctly and that you can add/mofidy issuingrules correctly. Because this bug can create data loss, the old database update has also been changed to avoid this problem for people updating at a later point in time. Checkout an older version of Koha pre 3.09.00.027. 1) Run the database updates. 2) Verify again, that adding/modifying issuingrules works correctly. Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
I don't think the patch should be altering a past database update the way it is. I don't believe it would have any negative effects, but I've had patches fail qa for this reason in the past.
Kyle, if we don't alter the old database update we will have people losing data. If your maxfine was 4.5 it will be 5 in the circulation matrix on update with the maxfine being lost. That's why I chose to also update the old updatedatabase. Running the alter database for already correct installations won't change a thing for them.
Created attachment 19601 [details] [review] Bug 10490: Overdue fines cap can't store decimal values When overduefinescap was added to the issuingrules the datatype given was decimal. This translates in MySQL to decimal(10,0). This doesn't allow you to store decimal values and therefore values like 4.5 are saved as 5 in the database. To test: On a current installation: 1) Try to enter 4.5 as Overdue fines cap. Verify that the value is not correctly saved. 2) Apply patch and run database update. 3) Try adding/changing an issuing rule setting Overdue fines cap to 4.5 again. 4) Verify the value is saved correctly. Create a new Koha installation from scratch: 1) Verify that the issuingrules table has been created correctly and that you can add/mofidy issuingrules correctly. Because this bug can create data loss, the old database update has also been changed to avoid this problem for people updating at a later point in time. Checkout an older version of Koha pre 3.09.00.027. 1) Run the database updates. 2) Verify again, that adding/modifying issuingrules works correctly. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Pushed to master. Thanks, Katrin!
This patch has been pushed to 3.12.x, will be in 3.12.2. Thanks Katrin!
Pushed to 3.10.x, will be in 3.10.10
Pushed to 3.8.x, will be in 3.8.17
(In reply to Chris Hall from comment #10) > Pushed to 3.8.x, will be in 3.8.17 Chris, This patch produces an error (see http://lists.koha-community.org/pipermail/koha-devel/2013-September/039649.html) updating the database. Since bug 7420 is not pushed in 3.8.x, I think this fix should not be pushed.