From 5c45dc6fa16b7ebc3b5f839ea0c4ba24ffee5052 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@biblibre.com>
Date: Mon, 10 Nov 2014 10:45:25 +0100
Subject: [PATCH] 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: jeremie.benarros <jeremie.benarros@inlibro.com>
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
---
 admin/aqbudgets.pl                                 |    6 ++++++
 installer/data/mysql/updatedatabase.pl             |   22 ++++++++++++++++++++
 .../prog/en/modules/admin/aqbudgets.tt             |   11 +++++++++-
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/admin/aqbudgets.pl b/admin/aqbudgets.pl
index 8d3f3e8..6a8bba2f 100755
--- a/admin/aqbudgets.pl
+++ b/admin/aqbudgets.pl
@@ -231,6 +231,10 @@ if ($op eq 'add_form') {
                                                     # END $OP eq DELETE_CONFIRM
 # called by delete_confirm, used to effectively confirm deletion of data in DB
 } elsif ( $op eq 'delete_confirmed' ) {
+    if ( HasChildren( $budget_id ) ) {
+        # We should never be here, the interface does not provide this action.
+        die("Delete a fund with children is not possible");
+    }
     my $rc = DelBudget($budget_id);
     $op = 'list';
 } elsif( $op eq 'add_validate' ) {
@@ -330,6 +334,8 @@ if ( $op eq 'list' ) {
         @budget_hierarchy = reverse(@budget_hierarchy);
 
         $budget->{budget_hierarchy} = \@budget_hierarchy;
+
+        $budget->{budget_has_children} = BudgetHasChildren( $budget->{budget_id} );
     }
 
     my $budget_period_total = $period->{budget_period_total};
diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl
index 5ada385..c439b19 100755
--- a/installer/data/mysql/updatedatabase.pl
+++ b/installer/data/mysql/updatedatabase.pl
@@ -9437,6 +9437,28 @@ if ( CheckVersion($DBversion) ) {
    print "Upgrade to $DBversion done (Bug 9043: Add system preference OpacAdvSearchOptions and OpacAdvSearchMoreOptions)\n";
    SetVersion ($DBversion);
 }
+$DBversion = "3.17.00.XXX";
+if ( CheckVersion($DBversion) ) {
+    my $orphan_budgets = $dbh->selectall_arrayref(q|
+        SELECT budget_id, budget_name, budget_code
+        FROM aqbudgets
+        WHERE   budget_parent_id IS NOT NULL
+            AND budget_parent_id NOT IN (
+                SELECT DISTINCT budget_id FROM aqbudgets
+            )
+    |, { Slice => {} } );
+
+    if ( @$orphan_budgets ) {
+        for my $b ( @$orphan_budgets ) {
+            print "Fund $b->{budget_name} (code:$b->{budget_code}, id:$b->{budget_id}) does not have a parent, it may cause problem\n";
+        }
+        print "Upgrade to $DBversion done (Bug 12905: Check budget integrity: FAIL)\n";
+    } else {
+        print "Upgrade to $DBversion done (Bug 12905: Check budget integrity: OK)\n";
+    }
+    SetVersion($DBversion);
+}
+
 
 =head1 FUNCTIONS
 
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
index b97d0d8..63be8fa 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
@@ -218,6 +218,11 @@ var MSG_PARENT_BENEATH_BUDGET = "- " + _("New budget-parent is beneath budget")
         $("#filterbutton").click(function() {
             $("#fundfilters").slideToggle(0);
         });
+
+        $(".deletefund-disabled").on("click", function(e){
+            e.preventDefault();
+            alert("This fund has children. It cannot be deleted.");
+        });
     });
 //]]>
 </script>
@@ -391,7 +396,11 @@ var MSG_PARENT_BENEATH_BUDGET = "- " + _("New budget-parent is beneath budget")
                 </a>
                 <ul class="dropdown-menu pull-right" role="menu" aria-labelledby="budgetactions[% budget.budget_id %]_[% budget.budget_period_id %]">
                     <li><a href="/cgi-bin/koha/admin/aqbudgets.pl?op=add_form&amp;budget_id=[% budget.budget_id %]&amp;budget_period_id=[% budget.budget_period_id %]" >Edit</a></li>
-                    <li><a href="/cgi-bin/koha/admin/aqbudgets.pl?op=delete_confirm&amp;budget_id=[% budget.budget_id %]&amp;budget_period_id=[% budget.budget_period_id %]">Delete</a></li>
+                    [% IF budget.budget_has_children %]
+                        <li class="disabled"><a href="#" class="deletefund-disabled" data-toggle="tooltip" data-placement="left" title="This fund has children">Delete</a></li>
+                    [% ELSE %]
+                        <li><a href="/cgi-bin/koha/admin/aqbudgets.pl?op=delete_confirm&amp;budget_id=[% budget.budget_id %]&amp;budget_period_id=[% budget.budget_period_id %]">Delete</a></li>
+                    [% END %]
                     <li><a href="/cgi-bin/koha/admin/aqbudgets.pl?op=add_form&amp;budget_parent_id=[% budget.budget_id %]&amp;budget_period_id=[% budget.budget_period_id %]">Add child fund</a></li>
                 </ul>
             </div>
-- 
1.7.9.5