Bug 12601 - aqorders.budget_id should be a foreign key
Summary: aqorders.budget_id should be a foreign key
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 5334
  Show dependency treegraph
 
Reported: 2014-07-18 10:26 UTC by Jonathan Druart
Modified: 2017-05-02 21:23 UTC (History)
4 users (show)

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


Attachments
Bug 12601: Add a foreign key for aqorders.budget_id (3.45 KB, patch)
2014-07-18 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12601: Add a foreign key for aqorders.budget_id (5.14 KB, patch)
2014-08-19 09:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12601: Add a foreign key for aqorders.budget_id (5.30 KB, patch)
2014-10-20 15:55 UTC, Paola Rossi
Details | Diff | Splinter Review
Bug 12601: Add a foreign key for aqorders.budget_id (5.18 KB, patch)
2014-10-26 20:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12601: Add a foreign key for aqorders.budget_id (5.40 KB, patch)
2014-10-31 13:15 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
k (5.24 KB, patch)
2014-11-21 19:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12601: Add a foreign key for aqorders.budget_id (5.24 KB, patch)
2014-11-21 19:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED OFF] Bug 12691: Use Koha.Preference in Self-Checkout (9.75 KB, patch)
2017-05-02 21:22 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2014-07-18 10:26:54 UTC
When creating an order, the fund value is mandatory but the DB structure does not show this constraint.
Comment 1 Jonathan Druart 2014-07-18 10:41:54 UTC Comment hidden (obsolete)
Comment 2 Srdjan Jankovic 2014-08-18 06:29:52 UTC
And what about kohastructure.sql?
Comment 3 Jonathan Druart 2014-08-19 09:50:44 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2014-08-19 09:51:28 UTC
(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.
Comment 5 Paola Rossi 2014-10-20 15:55:00 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall (khall) 2014-10-24 14:38:58 UTC
Database update stanza contains a db revision instead of XXX. Causing a merge conflict.
Comment 7 Katrin Fischer 2014-10-26 20:55:29 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2014-10-26 20:56:13 UTC
I fixed the conflict. Kyle, could you take another look?
Comment 9 Kyle M Hall (khall) 2014-10-31 13:15:27 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2014-11-21 19:23:09 UTC Comment hidden (obsolete)
Comment 11 Katrin Fischer 2014-11-21 19:23:58 UTC
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>
Comment 12 Katrin Fischer 2014-11-21 19:24:29 UTC
Rebased and fixed conflict in updatedatabase.
Comment 13 Tomás Cohen Arazi (tcohen) 2014-11-21 23:24:50 UTC
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.
Comment 14 Jonathan Druart 2014-11-24 11:00:03 UTC
(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.
Comment 15 Tomás Cohen Arazi (tcohen) 2015-02-05 18:30:16 UTC
Patch pushed to master.

Thanks Jonathan!
Comment 16 Katrin Fischer 2017-05-02 21:22:32 UTC
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 17 Katrin Fischer 2017-05-02 21:23:18 UTC
Comment on attachment 62992 [details] [review]
[SIGNED OFF] Bug 12691: Use Koha.Preference in Self-Checkout

wrong bug number!