Bug 13007

Summary: suggestions.budgetid should be a foreign key
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: katrin.fischer, paola.rossi, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12601
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 12627    
Bug Blocks: 5334, 13014, 13675    
Attachments: Bug 13007: Add a foreign key for suggestions.budgetid
Bug 13007: Add a foreign key for suggestions.budgetid
Bug 13007: Add a foreign key for suggestions.budgetid
Bug 13007: Add a foreign key for suggestions.budgetid
Bug 13007: Add a foreign key for suggestions.budgetid
Bug 13007: Special case budgetid == ''
Bug 13007: Add a foreign key for suggestions.budgetid
Bug 13007: Special case budgetid == ''
[PASSED QA] Bug 13007: Add a foreign key for suggestions.budgetid
[PASSED QA] Bug 13007: Special case budgetid == ''
Bug 13007: Add a foreign key for suggestions.budgetid
Bug 13007: Special case budgetid == ''

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!