Description
Jonathan Druart
2014-07-18 10:26:54 UTC
Created attachment 29820 [details] [review] Bug 12601: Add a foreign key for aqorders.budget_id When creating an order, the fund value is mandatory but the DB structure does not show this constraint. The aqorders.budget_id field should be linked to aqbudgets.budget_id. The updatedatabase.pl (entry 3.01.00.077) commented this constraint, certainly for a retro compatibility reason. Actually I found some cases (in production) where aqorders.budget_id is set to "0". To add this constraint, we should add a temporary fund to linked orphan orders. Test plan: 0/ Verify it is not possible to create an order not linked to a fund via the Koha interface. 1/ Using your SQL CLI (or equivalent), create or update 1+ orders and set "0" in the budget_id field. 2/ Execute the updatedabase script. 3/ Verify that your order is linked to a new fund (which is linked to a new budget). 4/ Verify the constraint has correctly been added (show create table aqorders). And what about kohastructure.sql? Created attachment 30977 [details] [review] Bug 12601: Add a foreign key for aqorders.budget_id When creating an order, the fund value is mandatory but the DB structure does not show this constraint. The aqorders.budget_id field should be linked to aqbudgets.budget_id. The updatedatabase.pl (entry 3.01.00.077) commented this constraint, certainly for a retro compatibility reason. Actually I found some cases (in production) where aqorders.budget_id is set to "0". To add this constraint, we should add a temporary fund to linked orphan orders. Test plan: 0/ Verify it is not possible to create an order not linked to a fund via the Koha interface. 1/ Using your SQL CLI (or equivalent), create or update 1+ orders and set "0" in the budget_id field. 2/ Execute the updatedabase script. 3/ Verify that your order is linked to a new fund (which is linked to a new budget). 4/ Verify the constraint has correctly been added (show create table aqorders). (In reply to Srdjan Jankovic from comment #2) > And what about kohastructure.sql? Oops, added! I also added a warning message on deleting a fund. Created attachment 32543 [details] [review] Bug 12601: Add a foreign key for aqorders.budget_id I've applied the patch against master 3.17.00.029 The patch needs a trivial rebase. Everything is OK. So I pass the patch to "Signed Off" status. Database update stanza contains a db revision instead of XXX. Causing a merge conflict. Created attachment 32777 [details] [review] Bug 12601: Add a foreign key for aqorders.budget_id When creating an order, the fund value is mandatory but the DB structure does not show this constraint. The aqorders.budget_id field should be linked to aqbudgets.budget_id. The updatedatabase.pl (entry 3.01.00.077) commented this constraint, certainly for a retro compatibility reason. Actually I found some cases (in production) where aqorders.budget_id is set to "0". To add this constraint, we should add a temporary fund to linked orphan orders. Test plan: 0/ Verify it is not possible to create an order not linked to a fund via the Koha interface. 1/ Using your SQL CLI (or equivalent), create or update 1+ orders and set "0" in the budget_id field. 2/ Execute the updatedabase script. 3/ Verify that your order is linked to a new fund (which is linked to a new budget). 4/ Verify the constraint has correctly been added (show create table aqorders). Signed-off-by: Paola Rossi <paola.rossi@cineca.it> I fixed the conflict. Kyle, could you take another look? Created attachment 33067 [details] [review] [PASSED QA] Bug 12601: Add a foreign key for aqorders.budget_id When creating an order, the fund value is mandatory but the DB structure does not show this constraint. The aqorders.budget_id field should be linked to aqbudgets.budget_id. The updatedatabase.pl (entry 3.01.00.077) commented this constraint, certainly for a retro compatibility reason. Actually I found some cases (in production) where aqorders.budget_id is set to "0". To add this constraint, we should add a temporary fund to linked orphan orders. Test plan: 0/ Verify it is not possible to create an order not linked to a fund via the Koha interface. 1/ Using your SQL CLI (or equivalent), create or update 1+ orders and set "0" in the budget_id field. 2/ Execute the updatedabase script. 3/ Verify that your order is linked to a new fund (which is linked to a new budget). 4/ Verify the constraint has correctly been added (show create table aqorders). Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 33798 [details] [review] k Created attachment 33799 [details] [review] Bug 12601: Add a foreign key for aqorders.budget_id When creating an order, the fund value is mandatory but the DB structure does not show this constraint. The aqorders.budget_id field should be linked to aqbudgets.budget_id. The updatedatabase.pl (entry 3.01.00.077) commented this constraint, certainly for a retro compatibility reason. Actually I found some cases (in production) where aqorders.budget_id is set to "0". To add this constraint, we should add a temporary fund to linked orphan orders. Test plan: 0/ Verify it is not possible to create an order not linked to a fund via the Koha interface. 1/ Using your SQL CLI (or equivalent), create or update 1+ orders and set "0" in the budget_id field. 2/ Execute the updatedabase script. 3/ Verify that your order is linked to a new fund (which is linked to a new budget). 4/ Verify the constraint has correctly been added (show create table aqorders). Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Rebased and fixed conflict in updatedatabase. Please check the failing tests because of this patches: t/db_dependent/Acquisition/CancelReceipt.t t/db_dependent/Acquisition/Invoices.t t/db_dependent/Acquisition/TransferOrder.t Probably problem with tests sample data, but cannot check/fix right now. (In reply to Tomás Cohen Arazi from comment #13) > Please check the failing tests because of this patches: > > t/db_dependent/Acquisition/CancelReceipt.t > t/db_dependent/Acquisition/Invoices.t > t/db_dependent/Acquisition/TransferOrder.t > > Probably problem with tests sample data, but cannot check/fix right now. It looks good here. Did you execute the updatedatabase entry? Maybe it does not worth to push this one before the release. Patch pushed to master. Thanks Jonathan! Created attachment 62992 [details] [review] [SIGNED OFF] Bug 12691: Use Koha.Preference in Self-Checkout For calls to SCOUserJS, SCOUserCSS, OPACUserCSS, AllowSelfCheckReturns, OpacFavicon, ShowPatronImageInWebBasedSelfCheck, SelfCheckoutByLogin Sponsored-by: Catalyst IT Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Comment on attachment 62992 [details] [review] [SIGNED OFF] Bug 12691: Use Koha.Preference in Self-Checkout wrong bug number! |