Bug 13007 - suggestions.budgetid should be a foreign key
Summary: suggestions.budgetid 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:
: 13005 (view as bug list)
Depends on: 12627
Blocks: 5334 13014 13675
  Show dependency treegraph
 
Reported: 2014-09-29 14:17 UTC by Jonathan Druart
Modified: 2015-12-03 22:11 UTC (History)
3 users (show)

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


Attachments
Bug 13007: Add a foreign key for suggestions.budgetid (2.57 KB, patch)
2014-09-29 14:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13007: Add a foreign key for suggestions.budgetid (3.41 KB, patch)
2014-09-30 13:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13007: Add a foreign key for suggestions.budgetid (3.49 KB, patch)
2014-11-05 10:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13007: Add a foreign key for suggestions.budgetid (3.49 KB, patch)
2014-11-05 10:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13007: Add a foreign key for suggestions.budgetid (3.60 KB, patch)
2014-11-18 09:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13007: Special case budgetid == '' (2.18 KB, patch)
2014-11-18 09:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13007: Add a foreign key for suggestions.budgetid (3.76 KB, patch)
2014-11-18 14:11 UTC, Paola Rossi
Details | Diff | Splinter Review
Bug 13007: Special case budgetid == '' (2.31 KB, patch)
2014-11-18 14:14 UTC, Paola Rossi
Details | Diff | Splinter Review
[PASSED QA] Bug 13007: Add a foreign key for suggestions.budgetid (3.72 KB, patch)
2014-11-22 10:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 13007: Special case budgetid == '' (2.34 KB, patch)
2014-11-22 10:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13007: Add a foreign key for suggestions.budgetid (3.74 KB, patch)
2015-01-29 09:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13007: Special case budgetid == '' (2.35 KB, patch)
2015-01-29 09:23 UTC, Jonathan Druart
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-09-29 14:17:06 UTC

    
Comment 1 Jonathan Druart 2014-09-29 14:21:51 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-09-30 13:26:44 UTC
*** Bug 13005 has been marked as a duplicate of this bug. ***
Comment 3 Jonathan Druart 2014-09-30 13:29:14 UTC Comment hidden (obsolete)
Comment 4 Paola Rossi 2014-10-21 13:06:33 UTC
I've tried to apply the patches against master 3.17.00.029
At first I've applied the 6 patches of 12627.
---------------------------------------
The patch "Bug 13007: Add a foreign key for suggestions.budgetid"
needs a trivial rebase.

Moreover, this wrong line of the patch 13007:
+    print "Upgrade to $DBversion done (Bug 13007 - Add new foreign key suggestions.budgetid\n");

should be changed in the following right one:

+    print "Upgrade to $DBversion done (Bug 13007 - Add new foreign key suggestions.budgetid)\n";
---------------------------------------
About the first step:
0/ Verify it is not possible to create a suggestion not linked to a fund via
the Koha interface.

in intranet, "any" is an option in the dropdown list on "Acquisition information" for selecting a fund to link to the suggestion. So IMO it's still easy to create suggestions not linked to a fund.

So I pass the patch to "Failed QA" STATUS.
Comment 5 Jonathan Druart 2014-11-05 10:19:07 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2014-11-05 10:50:33 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2014-11-05 10:51:39 UTC
(In reply to Paola Rossi from comment #4)
> About the first step:
> 0/ Verify it is not possible to create a suggestion not linked to a fund via
> the Koha interface.
> 
> in intranet, "any" is an option in the dropdown list on "Acquisition
> information" for selecting a fund to link to the suggestion. So IMO it's
> still easy to create suggestions not linked to a fund.
> 
> So I pass the patch to "Failed QA" STATUS.

I don't remember why I wrote that...
If you don't choose a  fund, the value in DB will be NULL. Which is acceptable and normal.
I updated the test plan.
Comment 8 Paola Rossi 2014-11-17 16:30:26 UTC
I've applied the patch against master 3.17.00.055

The patch needed some trivial rebasing. 
The number of tests is not increased (error).

When I tried to add a new suggestion linked to "Any" fund, koha said:

Software error:

DBIx::Class::ResultSet::create(): Cannot add or update a child row: a foreign key constraint fails (`koha`.`suggestions`, CONSTRAINT `suggestions_budget_id_fk` FOREIGN KEY (`budgetid`) REFERENCES `aqbudgets` (`budget_id`) ON DELETE SET NULL ON UPDATE CASCADE) at /var/root-koha/.../C4/Suggestions.pm line 449

On the contrary, adding a new suggestion linked to a real fund is OK.

So I pass the patch to "Failed QA" status.

The "show create table suggestions" command gave:

CONSTRAINT `suggestions_budget_id_fk` FOREIGN KEY (`budgetid`) REFERENCES `aqbudgets` (`budget_id`) ON DELETE SET NULL ON UPDATE CASCADE
)
Comment 9 Jonathan Druart 2014-11-18 09:22:52 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2014-11-18 09:23:02 UTC Comment hidden (obsolete)
Comment 11 Paola Rossi 2014-11-18 14:11:13 UTC Comment hidden (obsolete)
Comment 12 Paola Rossi 2014-11-18 14:14:31 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2014-11-22 10:37:36 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2014-11-22 10:37:49 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2015-01-28 22:08:00 UTC
Hi Jonathan, tried testing 13014, but this dependency no longer applies, can you please rebase?
Comment 16 Jonathan Druart 2015-01-29 09:23:15 UTC
Created attachment 35601 [details] [review]
Bug 13007: Add a foreign key for suggestions.budgetid

The DB field suggestions.budgetid should be linked to
aqbudgets.budget_id.

If the fund is removed, this column should be set to NULL.

Test plan:
1/ Using your SQL CLI (or equivalent), create or update 1+ suggestions and
set "0" in the budgetid field (or a nonexistent budget id).
2/ Execute the updatedabase script.
3/ Verify that your suggestion is unlinked to the nonexistent fund.
4/ Verify the constraint has correctly been added (show create table
suggestions).
5/ Check that this change does not affect the behavior on adding a
suggestion (linked to a fund or not).

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Jonathan Druart 2015-01-29 09:23:31 UTC
Created attachment 35602 [details] [review]
Bug 13007: Special case budgetid == ''

In this special case (the suggestion is linked to "all funds"), the
budgetid value should be NULL in DB.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes old and new tests.
Comment 18 Jonathan Druart 2015-01-29 09:24:18 UTC
Last patches fixed conflict with bug 13378.
Comment 19 Tomás Cohen Arazi 2015-02-09 18:59:37 UTC
Patches pushed to master.

Thanks Jonathan!