Attachment 28705 Details for Bug 12164 – Bug 12164: Close a budget period (budget)

[patch] Bug 12164: Close a budget period (budget)

0004-Bug-12164-Close-a-budget-period-budget.patch (text/plain), 19.69 KB, created by Paola Rossi on 2014-06-06 14:26:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Paola Rossi

Created: 2014-06-06 14:26:52 UTC

Size: 19.69 KB

patch

obsolete

>From 5df3b2098cca2f4600be79f0d2f4687e64c716fa Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@biblibre.com>
>Date: Mon, 2 Jun 2014 10:18:04 +0200
>Subject: [PATCH 4/7] Bug 12164: Close a budget period (budget)
>
>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>
>---
> C4/Acquisition.pm                                  |    6 ++
> C4/Budgets.pm                                      |   78 +++++++++++++++++
> admin/aqbudgetperiods.pl                           |   50 ++++++++++-
> .../prog/en/modules/admin/aqbudgetperiods.tt       |   91 +++++++++++++++++++-
> t/db_dependent/Budgets.t                           |   56 +++++++++++-
> 5 files changed, 276 insertions(+), 5 deletions(-)
>
>diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
>index 38646cb..4b84cba 100644
>--- a/C4/Acquisition.pm
>+++ b/C4/Acquisition.pm
>@@ -1723,6 +1723,7 @@ sub SearchOrders {
>     my $owner = $params->{owner};
>     my $pending = $params->{pending};
>     my $ordered = $params->{ordered};
>+    my $budget_id = $params->{budget_id};
> 
>     my $dbh = C4::Context->dbh;
>     my @args = ();
>@@ -1812,6 +1813,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);
>diff --git a/C4/Budgets.pm b/C4/Budgets.pm
>index 89b943e..9e8245d 100644
>--- a/C4/Budgets.pm
>+++ b/C4/Budgets.pm
>@@ -1021,6 +1021,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 );
>@@ -1100,6 +1102,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)
> 
>diff --git a/admin/aqbudgetperiods.pl b/admin/aqbudgetperiods.pl
>index 7c11a34..e342b8c 100755
>--- a/admin/aqbudgetperiods.pl
>+++ b/admin/aqbudgetperiods.pl
>@@ -45,8 +45,7 @@ script to administer the budget periods table
> =cut
> 
> ## modules
>-use strict;
>-#use warnings; FIXME - Bug 2505
>+use Modern::Perl;
> use Number::Format qw(format_price);
> use CGI;
> use List::Util qw/min/;
>@@ -202,6 +201,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( { 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
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt
>index 6dc9a4d..8904659 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt
>@@ -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">
> // #################################################################################
>@@ -84,8 +88,22 @@
>                 { "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>
> 
>@@ -106,6 +124,9 @@
>     [% IF ( delete_confirmed ) %]&rsaquo;
>         Data deleted
>     [% END %]
>+    [% IF close_form %]&rsaquo;
>+      Close budget [% budget_period_description %]
>+    [% END %]
> </title>
> 
> 
>@@ -150,6 +171,12 @@
>     [% IF ( duplicate_form ) %]
>         <a href="/cgi-bin/koha/admin/aqbudgetperiods.pl">Budgets</a> &rsaquo; Duplicate budget
>     [% END %]
>+
>+    
>+    [% 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 %]
>     
>     
>     [% IF ( else ) %]
>@@ -162,7 +189,9 @@
> <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>
>@@ -309,6 +338,62 @@
> 
>     </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 %]
>+
> 
> [% IF ( else ) %]
>   <h2>Budgets administration</h2>
>@@ -350,6 +435,7 @@
>                   <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>
>@@ -390,6 +476,7 @@
>                       <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>
>diff --git a/t/db_dependent/Budgets.t b/t/db_dependent/Budgets.t
>index b0da03e..669b4ce 100755
>--- a/t/db_dependent/Budgets.t
>+++ b/t/db_dependent/Budgets.t
>@@ -1,5 +1,5 @@
> use Modern::Perl;
>-use Test::More tests => 33;
>+use Test::More tests => 39;
> 
> BEGIN {use_ok('C4::Budgets') }
> use C4::Context;
>@@ -16,6 +16,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 :
> #
>@@ -263,6 +269,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(
>@@ -284,6 +291,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(
>@@ -402,6 +410,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;
>@@ -426,3 +475,8 @@ sub _get_budgetname_by_id {
>       @$budgets;
>     return $budget_name;
> }
>+
>+# C4::Context->userenv
>+sub Mock_userenv {
>+    return $userenv;
>+}
>-- 
>1.7.10.4
>

Actions: View | Diff | Splinter Review

Attachments on bug 12164: 27905 | 27906 | 27907 | 27908 | 27909 | 27910 | 27911 | 28588 | 28589 | 28590 | 28591 | 28592 | 28593 | 28695 | 28702 | 28703 | 28704 | 28705 | 28706 | 28707 | 28708 | 29354 | 29355 | 29356 | 29422 | 29423 | 29424 | 29425 | 29426 | 29427 | 29428 | 29429 | 29430 | 29733 | 29734 | 29735 | 29736 | 29737 | 29738 | 29739 | 29740 | 29741 | 30043 | 30044