Bug 12164: Close a budget period (budget)
authorJonathan Druart <jonathan.druart@biblibre.com>
Mon, 2 Jun 2014 08:18:04 +0000 (10:18 +0200)
committerTomas Cohen Arazi <tomascohen@gmail.com>
Thu, 24 Jul 2014 17:17:15 +0000 (14:17 -0300)
This is the main patch.

On closing a budget period, all unreceived orders are moved from the
old/previous fiscal year into the new fiscal year.

You can rollover funds unused in the previous fiscal year to the new
fiscal year.

This patch set is based on bug 12168 (bugfix) and can be tested on top
of bug 11578 (easier to see the fund structure).

The patch set is cut in 6 main patches:

- Move the budget period clone logic into C4::Budgets
  The code is moved from the pl to Budgets.pm and unit tests are provided.
  The original code should certainly be buggy since a typo existed.
- On cloning budget period, mark original budget as inactive
  Cloning a budget period is already possible in Koha, this patch adds a
  checkbox to mark as inactive the original budget. That avoids to edit
  the budget and click the "inactive" checkbox. Both do the same action.
- On cloning budget periods, add a "reset all funds" option
  Same as before, a new checkbox is added on cloning a budget period. If
  you check it, all fund amounts will be set to 0. Otherwise, no change
  compared to the existing behavior.
- Close a budget period (budget)
  The goal of this patch set is to move unreceived orders from a budget to
  another. This patch adds a C4::Budgets::MoveOrders routine which does
  this job.
  This action is only possible if the fund structure is the same for both
  budgets, the budget_code field should be the same.
- On closing budget period, move unspent amount
  Unspent amount will be move from the previous budget structure to the
  new one.
- Add UI report
  This patch only adds a report when closing a budget is done.

Test plan:
Wording: below, budget is a "budget period" and fund is a "budget".
Prerequisite: Having 1 active budget with some funds (with different
levels and different amounts). Order and receive some orders (not all).
1/ Go on the budgets administration page (admin/aqbudgetperiods.pl) and
duplicate the structure of this budget ("Duplicate" link in the
"Actions" column).
2/ Enter start and end date for this budget and mark the original budget
as inactive.
3/ Note that a new budget is created, with the same fund structures (and
same value) and that the old one is marked as inactive (see
admin/aqbudgets.pl page with patches from bug 11578).
4/ Try to close the new budget: it is not possible, there is no
unreceived orders for this budget.
5/ You can close the inactive budget ("Close" link in the "Actions"
column).
6/ Verify the number of "Unreceived orders" is correct and select the
new budget in the budget list. Click on the "Move remaining unspent
funds" if you want to move unspent amounts.
7/ A report view is displayed and show you the ordernumber which have
been impacted (grouped by fund).
8/ Try different configuration, depending on the selected checkboxes.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
C4/Acquisition.pm
C4/Budgets.pm
admin/aqbudgetperiods.pl
koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt
t/db_dependent/Budgets.t

index d017d62..7904cbe 100644 (file)
@@ -1731,6 +1731,7 @@ sub SearchOrders {
     my $pending = $params->{pending};
     my $ordered = $params->{ordered};
     my $biblionumber = $params->{biblionumber};
+    my $budget_id = $params->{budget_id};
 
     my $dbh = C4::Context->dbh;
     my @args = ();
@@ -1824,6 +1825,11 @@ sub SearchOrders {
         push @args, $userenv->{'number'};
     }
 
+    if ( $budget_id ) {
+        $query .= ' AND aqorders.budget_id = ?';
+        push @args, $budget_id;
+    }
+
     $query .= ' ORDER BY aqbasket.basketno';
 
     my $sth = $dbh->prepare($query);
index aed5e82..075cef7 100644 (file)
@@ -1041,6 +1041,8 @@ sub CloneBudgetPeriod {
 
     $budget_period->{budget_period_startdate} = $budget_period_startdate;
     $budget_period->{budget_period_enddate}   = $budget_period_enddate;
+    # The new budget (budget_period) should be active by default
+    $budget_period->{budget_period_active}    = 1;
     my $original_budget_period_id = $budget_period->{budget_period_id};
     delete $budget_period->{budget_period_id};
     my $new_budget_period_id = AddBudgetPeriod( $budget_period );
@@ -1127,6 +1129,82 @@ sub CloneBudgetHierarchy {
     }
 }
 
+=head2 MoveOrders
+
+  my $report = MoveOrders({
+    from_budget_period_id => $from_budget_period_id,
+    to_budget_period_id   => $to_budget_period_id,
+  });
+
+Clone a budget hierarchy.
+
+=cut
+
+sub MoveOrders {
+    my ($params)              = @_;
+    my $from_budget_period_id = $params->{from_budget_period_id};
+    my $to_budget_period_id   = $params->{to_budget_period_id};
+    return
+      if not $from_budget_period_id
+          or not $to_budget_period_id
+          or $from_budget_period_id == $to_budget_period_id;
+
+    # Can't move orders to an inactive budget (budgetperiod)
+    my $budget_period = GetBudgetPeriod($to_budget_period_id);
+    return unless $budget_period->{budget_period_active};
+
+    my @report;
+    my $dbh     = C4::Context->dbh;
+    my $sth_update_aqorders = $dbh->prepare(
+        q|
+            UPDATE aqorders
+            SET budget_id = ?
+            WHERE ordernumber = ?
+        |
+    );
+    my $from_budgets = GetBudgetHierarchy($from_budget_period_id);
+    for my $from_budget (@$from_budgets) {
+        my $new_budget_id = $dbh->selectcol_arrayref(
+            q|
+                SELECT budget_id
+                FROM aqbudgets
+                WHERE budget_period_id = ?
+                    AND budget_code = ?
+            |, {}, $to_budget_period_id, $from_budget->{budget_code}
+        );
+        $new_budget_id = $new_budget_id->[0];
+        my $new_budget = GetBudget( $new_budget_id );
+        unless ( $new_budget ) {
+            push @report,
+              {
+                moved       => 0,
+                budget_code => $from_budget->{budget_code},
+                error       => 'budget_code_not_exists',
+              };
+            next;
+        }
+        my $orders_to_move = C4::Acquisition::SearchOrders(
+            {
+                budget_id => $from_budget->{budget_id},
+                pending   => 1,
+            }
+        );
+
+        my @orders_moved;
+        for my $order (@$orders_to_move) {
+            $sth_update_aqorders->execute( $new_budget->{budget_id}, $order->{ordernumber} );
+            push @orders_moved, $order->{ordernumber};
+        }
+
+        push @report,
+          {
+            budget => $new_budget,
+            orders_moved  => \@orders_moved,
+            moved         => 1,
+          };
+    }
+    return \@report;
+}
 
 END { }    # module clean-up code here (global destructor)
 
index 2d1376e..8ab29ec 100755 (executable)
@@ -206,6 +206,53 @@ elsif ( $op eq 'duplicate_budget' ){
     $op = 'else';
 }
 
+elsif ( $op eq 'close_form' ) {
+
+    my $budget_period = GetBudgetPeriod($budget_period_id);
+
+    my $active_budget_periods =
+      C4::Budgets::GetBudgetPeriods( { budget_period_active => 1 } );
+
+    # Remove the budget period from the list
+    $active_budget_periods =
+      [ map { ( $_->{budget_period_id} == $budget_period_id ) ? () : $_ }
+          @$active_budget_periods ];
+
+    my $budgets_to_move = GetBudgetHierarchy($budget_period_id);
+
+    # C4::Context->userenv->{branchcode}, $show_mine ? $borrower_id : '')
+
+    my $number_of_unreceived_orders = 0;
+    for my $budget (@$budgets_to_move) {
+
+        # We want to move funds from this budget
+        my $unreceived_orders = C4::Acquisition::SearchOrders(
+            { budget_id => $budget->{budget_id}, } );
+        $budget->{unreceived_orders} = $unreceived_orders;
+        $number_of_unreceived_orders += scalar(@$unreceived_orders);
+    }
+
+    $template->param(
+        close_form       => 1,
+        budget_period_id => $budget_period_id,
+        budget_period_description =>
+          $budget_period->{budget_period_description},
+        budget_periods              => $active_budget_periods,
+        budgets_to_move             => $budgets_to_move,
+        number_of_unreceived_orders => $number_of_unreceived_orders,
+    );
+}
+
+elsif ( $op eq 'close_confirmed' ) {
+    my $to_budget_period_id = $input->param('to_budget_period_id');
+    my $report              = C4::Budgets::MoveOrders(
+        {
+            from_budget_period_id => $budget_period_id,
+            to_budget_period_id   => $to_budget_period_id,
+        }
+    );
+}
+
 # DEFAULT - DISPLAY AQPERIODS TABLE
 # -------------------------------------------------------------------
 # display the list of budget periods
index 6dc9a4d..8904659 100644 (file)
@@ -4,6 +4,10 @@
 [% INCLUDE 'doc-head-close.inc' %]
 [% INCLUDE 'calendar.inc' %]
 [% INCLUDE 'datatables.inc' %]
+[% IF close_form %]
+    <link href="[% interface %]/lib/jquery/plugins/treetable/stylesheets/jquery.treetable.css" rel="stylesheet" type="text/css" />
+    <script type="text/javascript" src="[% interface %]/lib/jquery/plugins/treetable/jquery.treetable.js"></script>
+[% END %]
 <script type="text/javascript" src="[% themelang %]/js/acq.js"></script>
 <script type="text/javascript">
 // #################################################################################
                 { "aTargets": [ -1 ], "bSortable": false, "bSearchable": false },
                 { "sType": "title-string", "aTargets" : [ "title-string" ] }
             ],
-            "bPaginate": false
+            "bPaginate": false,
+            'bAutoWidth': false
         } ) );
+
+        [% IF close_form %]
+          $("#budgeth").dataTable($.extend(true, {}, dataTablesDefaults, {
+            sDom: "t"
+          }));
+          $("#move_form").submit(function(){
+            var budget_from = "[% budget_period_description %]";
+            var budget_to = $("#budget_period").find("option:selected").html();
+            var alert_message = _("You have chosen to move all unreceived orders from '%s' to '%s'.").format(budget_from, budget_to);
+            alert_message += _("\nThis action cannot be reversed. Do you wish to continue?");
+            return confirm ( alert_message );
+          });
+        [% END %]
     });
 </script>
 
     [% IF ( delete_confirmed ) %]&rsaquo;
         Data deleted
     [% END %]
+    [% IF close_form %]&rsaquo;
+      Close budget [% budget_period_description %]
+    [% END %]
 </title>
 
 
     [% IF ( duplicate_form ) %]
         <a href="/cgi-bin/koha/admin/aqbudgetperiods.pl">Budgets</a> &rsaquo; Duplicate budget
     [% END %]
+
+    <!-- close a budget -->
+    [% IF close_form %]
+        <a href="/cgi-bin/koha/admin/aqbudgetperiods.pl">Budgets</a> &rsaquo;
+        Close budget <a href="cgi-bin/koha/admin/aqbudgets.pl?budget_period_id=[% budget_period_id %]">[% budget_period_description %]</a>
+    [% END %]
     <!-- display budget periods list -->
     <!-- ########################################## -->
     [% IF ( else ) %]
 <div id="yui-main">
 <div class="yui-b">
 
-[% INCLUDE 'budgets-admin-toolbar.inc' %]
+[% UNLESS close_form %]
+  [% INCLUDE 'budgets-admin-toolbar.inc' %]
+[% END %]
 
 [% IF ( duplicate_form ) %]
 <h3>Duplicate budget</h3>
 
     </div>
 [% END %]
+
+[% IF close_form %]
+  [% IF budget_periods.size == 0 %]
+    You cannot move funds of this budget, there is no active budget.
+    Please create a new active budget and retry.
+    <a href="/cgi-bin/koha/admin/aqbudgetperiods.pl">Back</a>
+  [% ELSIF number_of_unreceived_orders == 0 %]
+    There is no unreceived orders for this budget.
+    <a href="/cgi-bin/koha/admin/aqbudgetperiods.pl">Back</a>
+  [% ELSE %]
+    <h3>Choose the funds you want to move unreceived orders:</h3>
+      Fund list of budget <a href="cgi-bin/koha/admin/aqbudgets.pl?budget_period_id=[% budget_period_id %]">[% budget_period_description %]</a>:
+    <table id="budgeth">
+      <thead>
+        <tr>
+            <th>Fund id</th>
+            <th>Fund code</th>
+            <th>Fund name</th>
+            <th>Unreceived orders</th>
+        </tr>
+      </thead>
+      <tbody>
+        [% FOREACH budget IN budgets_to_move %]
+          <tr>
+            <td>[% budget.budget_id %]</td>
+            <td>[% budget.budget_code_indent %]</td>
+            <td>[% budget.budget_name %]</td>
+            <td>[% budget.unreceived_orders.size %]</td>
+          </tr>
+        [% END %]
+      </tbody>
+    </table>
+    <form action="/cgi-bin/koha/admin/aqbudgetperiods.pl" name="f" method="post" id="move_form">
+      <fieldset class="rows">
+        <ol>
+          <li>
+            <label class="required" for="to_budget_period_id">Select a budget</label>
+            <select name="to_budget_period_id" id="to_budget_period_id" required="required">
+              <option value=""></option>
+              [% FOR budget_period IN budget_periods %]
+                <option value="[% budget_period.budget_period_id %]">[% budget_period.budget_period_description %]</option>
+              [% END %]
+            </select>
+          </li>
+        </ol>
+      </fieldset>
+      <fieldset class="action">
+          <input type="hidden" name="op" value="close_confirmed" />
+          <input type="hidden" name="budget_period_id" value="[% budget_period_id %]" />
+          <input type="submit" value="Move unreceive orders" />
+          <a class="cancel" href="/cgi-bin/koha/admin/aqbudgetperiods.pl">Cancel</a>
+      </fieldset>
+    </form>
+  [% END %]
+[% END %]
+
 <!--  DEFAULT  display budget periods list -->
 [% IF ( else ) %]
   <h2>Budgets administration</h2>
                   <a href="[% script_name %]?op=add_form&amp;budget_period_id=[% period_active.budget_period_id |html %]">Edit</a>
                   <a href="[% script_name %]?op=delete_confirm&amp;budget_period_id=[% period_active.budget_period_id %]">Delete</a>
                   <a href="[% script_name %]?op=duplicate_form&amp;budget_period_id=[% period_active.budget_period_id %]">Duplicate</a>
+                  <a href="[% script_name %]?op=close_form&amp;budget_period_id=[% period_active.budget_period_id %]">Close</a>
                   <a href="/cgi-bin/koha/admin/aqbudgets.pl?op=add_form&amp;budget_period_id=[% period_active.budget_period_id %]">Add fund</a>
                 </td>
                 </tr>
                       <a href="[% script_name %]?op=add_form&amp;budget_period_id=[% period_loo.budget_period_id |html %]">Edit</a>
                       <a href="[% script_name %]?op=delete_confirm&amp;budget_period_id=[% period_loo.budget_period_id %]">Delete</a>
                       <a href="[% script_name %]?op=duplicate_form&amp;budget_period_id=[% period_loo.budget_period_id %]">Duplicate</a>
+                      <a href="[% script_name %]?op=close_form&amp;budget_period_id=[% period_loo.budget_period_id %]">Close</a>
                   <a href="/cgi-bin/koha/admin/aqbudgets.pl?op=add_form&amp;budget_period_id=[% period_loo.budget_period_id %]">Add fund</a>
                   </td>
                   </tr>
index bf40821..5a22315 100755 (executable)
@@ -1,5 +1,5 @@
 use Modern::Perl;
-use Test::More tests => 71;
+use Test::More tests => 77;
 
 BEGIN {
     use_ok('C4::Budgets')
@@ -18,6 +18,12 @@ $dbh->{RaiseError} = 1;
 $dbh->do(q|DELETE FROM aqbudgetperiods|);
 $dbh->do(q|DELETE FROM aqbudgets|);
 
+# Mock userenv
+local $SIG{__WARN__} = sub { warn $_[0] unless $_[0] =~ /redefined/ };
+my $userenv;
+*C4::Context::userenv = \&Mock_userenv;
+$userenv = { flags => 1, id => 'my_userid', branch => 'CPL' };
+
 #
 # Budget Periods :
 #
@@ -295,6 +301,7 @@ my %budgets;
 my $invoiceid = AddInvoice(invoicenumber => 'invoice_test_clone', booksellerid => $booksellerid, unknown => "unknown");
 my $item_price = 10;
 my $item_quantity = 2;
+my $number_of_orders_to_move = 0;
 for my $infos (@order_infos) {
     for ( 1 .. $infos->{pending_quantity} ) {
         my ( undef, $ordernumber ) = C4::Acquisition::NewOrder(
@@ -316,6 +323,7 @@ for my $infos (@order_infos) {
             }
         );
         push @{ $budgets{$infos->{budget_id}} }, $ordernumber;
+        $number_of_orders_to_move++;
     }
     for ( 1 .. $infos->{spent_quantity} ) {
         my ( undef, $ordernumber ) = C4::Acquisition::NewOrder(
@@ -434,6 +442,47 @@ is( $number_of_budgets_not_reset, 0,
     'CloneBudgetPeriod has reset all budgets (funds)' );
 
 
+# MoveOrders
+my $number_orders_moved = C4::Budgets::MoveOrders();
+is( $number_orders_moved, undef, 'MoveOrders return undef if no arg passed' );
+$number_orders_moved =
+  C4::Budgets::MoveOrders( { from_budget_period_id => $budget_period_id } );
+is( $number_orders_moved, undef,
+    'MoveOrders return undef if only 1 arg passed' );
+$number_orders_moved =
+  C4::Budgets::MoveOrders( { to_budget_period_id => $budget_period_id } );
+is( $number_orders_moved, undef,
+    'MoveOrders return undef if only 1 arg passed' );
+$number_orders_moved = C4::Budgets::MoveOrders(
+    {
+        from_budget_period_id => $budget_period_id,
+        to_budget_period_id   => $budget_period_id
+    }
+);
+is( $number_orders_moved, undef,
+    'MoveOrders return undef if 2 budget period id are the same' );
+
+$budget_period_id_cloned = C4::Budgets::CloneBudgetPeriod(
+    {
+        budget_period_id        => $budget_period_id,
+        budget_period_startdate => '2014-01-01',
+        budget_period_enddate   => '2014-12-31',
+        reset_all_funds         => 1,
+    }
+);
+
+my $report = C4::Budgets::MoveOrders(
+    {
+        from_budget_period_id => $budget_period_id,
+        to_budget_period_id   => $budget_period_id_cloned,
+    }
+);
+is( scalar( @$report ), 6 , "MoveOrders has processed 6 funds" );
+
+my $number_of_orders_moved = 0;
+$number_of_orders_moved += scalar( @{ $_->{orders_moved} } ) for @$report;
+is( $number_of_orders_moved, $number_of_orders_to_move, "MoveOrders has moved $number_of_orders_to_move orders" );
+
 sub _get_dependencies {
     my ($budget_hierarchy) = @_;
     my $graph;
@@ -458,3 +507,8 @@ sub _get_budgetname_by_id {
       @$budgets;
     return $budget_name;
 }
+
+# C4::Context->userenv
+sub Mock_userenv {
+    return $userenv;
+}