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: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
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)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


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
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 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 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 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 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!