Bug 12905 - Deleting parent fund will orphan child funds, leaving them un-editable
Summary: Deleting parent fund will orphan child funds, leaving them un-editable
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 4292 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-09-10 19:53 UTC by Barton Chittenden
Modified: 2015-12-03 22:02 UTC (History)
7 users (show)

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


Attachments
Bug 12905: funds with children could not be deleted (5.15 KB, patch)
2014-11-10 09:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12905: funds with children could not be deleted (5.51 KB, patch)
2014-11-11 15:39 UTC, Jeremie
Details | Diff | Splinter Review
Bug 12905: funds with children could not be deleted (5.19 KB, patch)
2014-11-12 08:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12905: funds with children could not be deleted (5.32 KB, patch)
2014-12-04 17:31 UTC, Paola Rossi
Details | Diff | Splinter Review
[PASSED QA] Bug 12905: funds with children could not be deleted (5.27 KB, patch)
2014-12-12 19:13 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Barton Chittenden 2014-09-10 19:53:35 UTC
It is possible to create two aqbudgets records with a parent-child relationship (i.e. budget_parent_id of the 'child' record is the 'budget_id' of the parent), then delete the parent record. The Child record will then be un-editable, because it will always trigger a 'Fund amount exceeds parent allocation' message.

Replicating issue:

From acqui-home.pl on staff client,

1) Click 'funds' link, navigating to cgi-bin/koha/admin/aqbudgets.pl
2) Click 'New budget' link.
3) Populate Start Date, End date, description and amount. Click 'Make budget active' and save changes.
4) Click on budget description, this will navigate to cgi-bin/koha/admin/aqbudgets.pl
5) Click 'New', select 'New fund for ...'
6) Populate 
   Fund code (I used PARENT01)
   Fund name (Parent 1)
   Amount (Will be less than or equal to budget amound populated earlier).
7) Click submit
8) For Parent 1, click 'Actions', then 'Add Child Fund'.
9) Populate Child:
   Fund code (I used CHILD01)
   Fund name (Child 1)
   Amount (Will be less than or equal to amount for Parent 1)
10) Click submit
11) Choose Actions->delete for 'Parent 1' fund. Choose 'Delete' when prompted.
12) Find 'Child 1' fund, click actions->edit
13) Click Submit. A pop-up will appear with the following text:


Form not submitted because of the following problem(s)
------------------------------------------------------------------------------------

- Fund amount exceeds parent allocation

I see the following problems

* There should be a referential integrity check for aqbudgets.budget_parent_id: if it exists, it must point to a valid parent record.
* Deleting an aqbudgets record with a child should issue an explicit warning that the fund has children.
 * The user should have the choice of deleting children with the parent or removing the link to the parent, thus promoting them to first class funds.
* function budgetExceedsParent in acq.js should be able to handle an unknown error
* cgi-bin/admin/check_parent_total.pl should check that the parent record exists, and should *not* pass $returncode = 1 if the parent record does not exist,  even if budget_parent_id is not NULL.
Comment 1 Jonathan Druart 2014-11-10 09:53:34 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-11-10 09:54:51 UTC
*** Bug 4292 has been marked as a duplicate of this bug. ***
Comment 3 Jeremie 2014-11-11 15:39:58 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2014-11-12 08:41:46 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2014-11-12 08:43:04 UTC
(In reply to Jeremie from comment #3)
> Javascript verification is working, but i got an error when i'm 'hacking'
> the JS :
> Undefined subroutine &main::HasChildren called at
> /inlibro/git/koha-try4jeremie-dev-inlibro/admin/aqbudgets.pl line 234

Hello Jeremie,
Thanks for testing it!
Yes, the routine's name is BudgetHasChildren, not HasChildren...
Fixed in the last patch, please retest.
Comment 6 Christopher Brannon 2014-11-26 22:09:20 UTC
Jonathan and Barton,
Can you verify that this problem still exists?

Without the patch on master, I go through steps 1 through 11, and when I delete the parent, the child fund is deleted as well.  There is no longer any sign of the child.

Christopher
Comment 7 Jonathan Druart 2014-11-27 08:15:25 UTC
(In reply to Christopher Brannon from comment #6)
> Jonathan and Barton,
> Can you verify that this problem still exists?
> 
> Without the patch on master, I go through steps 1 through 11, and when I
> delete the parent, the child fund is deleted as well.  There is no longer
> any sign of the child.
> 
> Christopher

Hi Christopher,
I tested 2 weeks ago and the problems were there.
They are:
1/ You don't see the orphan fund in the fund list
2/ You cannot edit the orphan fund amount ('Fund amount exceeds parent
allocation').

for 1/ you need to use the MySQL CLI.

Please back to "In discussion" if you confirm that you did not reproduce these 2 problems.
Comment 8 Paola Rossi 2014-12-04 17:31:35 UTC Comment hidden (obsolete)
Comment 9 Kyle M Hall 2014-12-12 19:13:42 UTC
Created attachment 34364 [details] [review]
[PASSED QA] Bug 12905: funds with children could not be deleted

The interface should prevent to delete funds with children.
Otherwise the relationship is broken and problems occur:
1/ You don't see the orphan fund in the fund list
2/ You cannot edit the orphan fund amount ('Fund amount exceeds parent
allocation').

This patch:
- adds a JS check, template side
- adds a check in the perl script (should never be true)
- adds an updatedatabase check, in order to alert users with inconsistent data.

Test plan:
Verify you are not allow to delete a fund with children.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Tomás Cohen Arazi 2015-02-05 18:01:14 UTC
Patch pushed to master.

Thanks Jonathan!
Comment 11 Chris Cormack 2015-02-10 21:48:53 UTC
Pushed to 3.18.x will be in 3.18.4