From 501bb50e6be40477491a11dad45d626b9459bd90 Mon Sep 17 00:00:00 2001 From: Mathieu Saby Date: Thu, 19 Dec 2013 00:10:44 +0100 Subject: [PATCH] Bug 11438 - Add creation date and closing date to basketgroups Content-Type: text/plain; charset="utf-8" This patch change the way Koha is dealing with closing basketgroups, taking as a model the way it deals with closing baskets. This improvement will serve as a base for further enh: - displaying the closing date in PDF instead of current date (this can be syspref-dependant) - usign the closing date for calculation of late orders, instead of closing date of the basket (this can be syspref-dependant) The patch provides 3 changes in in aqbasketgroups table structure: - The closing date is stored in bg_closedate column - The creation date is stored in bg_creationdate column - The closed column is removed When a basketgroup is open, or reopened, the closing date is set to NULL. I add a "bg_" prefix in column names to avoid all possible confusion between them and closedate/creationdate column in aqbaskets. Some changes are made to subs dealing with basketgroups in C4/Acquisition.pm. The 2 dates are displayed in basketgroup template - in lists of opened/closed basketgroups - in an individual basketgroup. Updatedatabase create the 2 columns and remove closed column, but also populates the 2 new columns: - bg_creationdate with the most recent closing date of the baskets included in each basketgroup - bg_closingdate with NULL is the bg is open, or with the most recent closing date of the baskets included in each basketgroup To test: (bg = basketgroup) 1. Before applying the patch, prepare some bgs : at least 1 open with 1 basket, 1 open with 2 baskets closed on different dates, 1 closed with 1 basket, 1 closed with 2 baskets closed on different dates 2. Apply the patch and run updatedatabase.pl 3. Show the list of open and closed bg and check the creation dates are correct (it should be the most recent closing date of the baskets included in each basketgroup) 4. Check the open bg have no closing date 5. Check the closed bg have a closing date, which must be the same as the creation date 6. Click on "View" to display an individual closed bg : check you the same dates as in the list of bgs 7. Reopen the closed bg. Check it is actually reopened and displayed in "open" tab, with no value in "closing date" column 8. Click on "Edit" to edit an individual open bg : check you the same creation date as in the list of bgs, and no closing date 9. Make some changes to this bg, and Save it without closing it 10. Check the changes were actually saved, and that the bg is still in "Open" tab, with no value in "closing date column" 11. Edit again the bg 12. Make some changes, but this time, check "Close basket group" box at the bottom of the form before Saving it 13. Check the changes were actually saved, and that the bg is now in "Closed" tab, with the today date in "closing date" column 14. Click on "New basketgroup" to create a new bg 15. Check no date at all is displayed in the creation form. 16. Give it a name, and Save it without closing it 17. Check the bg has been created, and is displayed in "Open" tab, with today date in "creation date" column, and no value in "closing date" 18. Click on "New basketgroup" to create an other new bg 19. Give it a name, but this time, check "Close basket group" box at the bottom of the form before Saving it 20. Check the bg has been created, and is displayed in "Open" tab, with today date in "creation date" column, and no value in "closing date" 23. In basket pages, Close an open basket, and ask to create a new bg with the same name 24. Check in bg page that a new closed bg with the same name as the closed basket has been created 25. In basket pages, Close basket without creating a bg, then put this basket in an open bg, using the "change group" dropdown menu 26. Check in bg page that the basket is now included in the bg --- C4/Acquisition.pm | 90 ++++++++++++-------- Koha/Schema/Result/Aqbasketgroup.pm | 23 +++-- acqui/basketgroup.pl | 50 +++++------ installer/data/mysql/kohastructure.sql | 3 +- installer/data/mysql/updatedatabase.pl | 11 +++ .../prog/en/modules/acqui/basketgroup.tt | 62 ++++++++------ 6 files changed, 139 insertions(+), 100 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 1bf41cb..b634824 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -424,7 +424,7 @@ sub CloseBasketgroup { my $dbh = C4::Context->dbh; my $sth = $dbh->prepare(" UPDATE aqbasketgroups - SET closed=1 + SET bg_closedate=now() WHERE id=? "); $sth->execute($basketgroupno); @@ -445,7 +445,7 @@ sub ReOpenBasketgroup { my $dbh = C4::Context->dbh; my $sth = $dbh->prepare(" UPDATE aqbasketgroups - SET closed=0 + SET bg_closedate=NULL WHERE id=? "); $sth->execute($basketgroupno); @@ -834,22 +834,24 @@ sub GetBasketsByBasketgroup { $basketgroupid = NewBasketgroup(\%hashref); Adds a basketgroup to the aqbasketgroups table, and add the initial baskets to it. +Keys of \%hashref can be one or several fields of aqbooksellers table: +- booksellerid +- name +- basketlist +- billingplace +- deliveryplace +- freedeliveryplace +- deliverycomment +or the special key "closed", which force the closure of the basketgroup if its value is "1". +If "closed" key does not exist or is not set to "1", the new basketgroup will be open. -$hashref->{'booksellerid'} is the 'id' field of the bookseller in the aqbooksellers table, +The "booksellerid" key is mandatory. -$hashref->{'name'} is the 'name' field of the basketgroup in the aqbasketgroups table, +bg_creationdate field of aqbasketgroups table is filled with the value of now () +bg_closedate field of aqbasketgroups table is filled with the value of now () if the "closed" key has "1" value -$hashref->{'basketlist'} is a list reference of the 'id's of the baskets that belong to this group, - -$hashref->{'billingplace'} is the 'billingplace' field of the basketgroup in the aqbasketgroups table, - -$hashref->{'deliveryplace'} is the 'deliveryplace' field of the basketgroup in the aqbasketgroups table, - -$hashref->{'freedeliveryplace'} is the 'freedeliveryplace' field of the basketgroup in the aqbasketgroups table, - -$hashref->{'deliverycomment'} is the 'deliverycomment' field of the basketgroup in the aqbasketgroups table, - -$hashref->{'closed'} is the 'closed' field of the aqbasketgroups table, it is false if 0, true otherwise. +If "booksellerid" key is not provided, the function dies with a message. +If a key is not allowed, the function ignores it. =cut @@ -858,21 +860,29 @@ sub NewBasketgroup { die "booksellerid is required to create a basketgroup" unless $basketgroupinfo->{'booksellerid'}; my $query = "INSERT INTO aqbasketgroups ("; my @params; - foreach my $field (qw(name billingplace deliveryplace freedeliveryplace deliverycomment closed)) { + foreach my $field (qw(name billingplace deliveryplace freedeliveryplace deliverycomment booksellerid)) { if ( defined $basketgroupinfo->{$field} ) { $query .= "$field, "; push(@params, $basketgroupinfo->{$field}); } } - $query .= "booksellerid) VALUES ("; + if (defined $basketgroupinfo->{"closed"} and $basketgroupinfo->{"closed"} eq "1") { + $query .= "bg_closedate, "; + } + $query .= "bg_creationdate) VALUES ("; foreach (@params) { $query .= "?, "; } - $query .= "?)"; - push(@params, $basketgroupinfo->{'booksellerid'}); + chop($query); + chop($query); + if (defined $basketgroupinfo->{"closed"} and $basketgroupinfo->{"closed"} eq "1") { + $query .= ", CAST(NOW() AS DATE)"; + } + $query .= ", CAST(NOW() AS DATE))"; my $dbh = C4::Context->dbh; my $sth = $dbh->prepare($query); $sth->execute(@params); + my $basketgroupid = $dbh->{'mysql_insertid'}; if( $basketgroupinfo->{'basketlist'} ) { foreach my $basketno (@{$basketgroupinfo->{'basketlist'}}) { @@ -891,22 +901,24 @@ sub NewBasketgroup { ModBasketgroup(\%hashref); Modifies a basketgroup in the aqbasketgroups table, and add the baskets to it. - -$hashref->{'id'} is the 'id' field of the basketgroup in the aqbasketgroup table, this parameter is mandatory, - -$hashref->{'name'} is the 'name' field of the basketgroup in the aqbasketgroups table, - -$hashref->{'basketlist'} is a list reference of the 'id's of the baskets that belong to this group, - -$hashref->{'billingplace'} is the 'billingplace' field of the basketgroup in the aqbasketgroups table, - -$hashref->{'deliveryplace'} is the 'deliveryplace' field of the basketgroup in the aqbasketgroups table, - -$hashref->{'freedeliveryplace'} is the 'freedeliveryplace' field of the basketgroup in the aqbasketgroups table, - -$hashref->{'deliverycomment'} is the 'deliverycomment' field of the basketgroup in the aqbasketgroups table, - -$hashref->{'closed'} is the 'closed' field of the aqbasketgroups table, it is false if 0, true otherwise. +Keys of \%hashref can be one or several fields of aqbooksellers table: +- booksellerid +- name +- basketlist +- billingplace +- deliveryplace +- freedeliveryplace +- deliverycomment +or the special key "closed", which force the closure of the basketgroup if its value is "1", and its opening if its value is "0" +The "booksellerid" key is mandatory. + +bg_closedate field of aqbasketgroups table is +- filled with the value of now() if the "closed" key has "1" value +or +- set to NULL if the "closed" key has "0" value + +If "booksellerid" key is not provided, the function dies with a message. +If a key is not allowed, the function ignores it. =cut @@ -916,7 +928,7 @@ sub ModBasketgroup { my $dbh = C4::Context->dbh; my $query = "UPDATE aqbasketgroups SET "; my @params; - foreach my $field (qw(name billingplace deliveryplace freedeliveryplace deliverycomment closed)) { + foreach my $field (qw(name billingplace deliveryplace freedeliveryplace deliverycomment)) { if ( defined $basketgroupinfo->{$field} ) { $query .= "$field=?, "; push(@params, $basketgroupinfo->{$field}); @@ -924,6 +936,12 @@ sub ModBasketgroup { } chop($query); chop($query); + if (defined $basketgroupinfo->{"closed"} and $basketgroupinfo->{"closed"} eq "1") { + $query .= ", bg_closedate=CAST(NOW() AS DATE)"; + } + if (defined $basketgroupinfo->{"closed"} and $basketgroupinfo->{"closed"} eq "0") { + $query .= ", bg_closedate=NULL"; + } $query .= " WHERE id=?"; push(@params, $basketgroupinfo->{'id'}); my $sth = $dbh->prepare($query); diff --git a/Koha/Schema/Result/Aqbasketgroup.pm b/Koha/Schema/Result/Aqbasketgroup.pm index 6b3348e..4700950 100644 --- a/Koha/Schema/Result/Aqbasketgroup.pm +++ b/Koha/Schema/Result/Aqbasketgroup.pm @@ -35,11 +35,6 @@ __PACKAGE__->table("aqbasketgroups"); is_nullable: 1 size: 50 -=head2 closed - - data_type: 'tinyint' - is_nullable: 1 - =head2 booksellerid data_type: 'integer' @@ -69,6 +64,18 @@ __PACKAGE__->table("aqbasketgroups"); is_nullable: 1 size: 10 +=head2 bg_creationdate + + data_type: 'date' + is_nullable: 1 + datetime_undef_if_invalid: 1 + +=head2 bg_closedate + + data_type: 'varchar' + is_nullable: 1 + datetime_undef_if_invalid: 1 + =cut __PACKAGE__->add_columns( @@ -76,8 +83,6 @@ __PACKAGE__->add_columns( { data_type => "integer", is_auto_increment => 1, is_nullable => 0 }, "name", { data_type => "varchar", is_nullable => 1, size => 50 }, - "closed", - { data_type => "tinyint", is_nullable => 1 }, "booksellerid", { data_type => "integer", is_foreign_key => 1, is_nullable => 0 }, "deliveryplace", @@ -88,6 +93,10 @@ __PACKAGE__->add_columns( { data_type => "varchar", is_nullable => 1, size => 255 }, "billingplace", { data_type => "varchar", is_nullable => 1, size => 10 }, + "bg_creationdate", + { data_type => "date", datetime_undef_if_invalid => 1, is_nullable => 1}, + "bg_closedate", + { data_type => "date", datetime_undef_if_invalid => 1, is_nullable => 1}, ); =head1 PRIMARY KEY diff --git a/acqui/basketgroup.pl b/acqui/basketgroup.pl index 36c4462..82f299e 100755 --- a/acqui/basketgroup.pl +++ b/acqui/basketgroup.pl @@ -311,7 +311,7 @@ $template->param(booksellerid => $booksellerid); if ( $op eq "add" ) { # # if no param('basketgroupid') is not defined, adds a new basketgroup -# else, edit (if it is open) or display (if it is close) the basketgroup basketgroupid +# else, edit (if it is open) or display (if it is closed) the basketgroup basketgroupid # the template will know if basketgroup must be displayed or edited, depending on the value of closed key # if(! $booksellerid){ @@ -353,13 +353,12 @@ if ( $op eq "add" ) { name => $basketgroup->{name}, deliverycomment => $basketgroup->{deliverycomment}, freedeliveryplace => $basketgroup->{freedeliveryplace}, + bg_creationdate => $basketgroup->{bg_creationdate}, + bg_closedate => $basketgroup->{bg_closedate} ); $billingplace = $basketgroup->{billingplace}; $deliveryplace = $basketgroup->{deliveryplace}; $freedeliveryplace = $basketgroup->{freedeliveryplace}; - $template->param( closedbg => ($basketgroup ->{'closed'}) ? 1 : 0); - } else { - $template->param( closedbg => 0); } # determine default billing and delivery places depending on librarian homebranch and existing basketgroup data my $borrower = GetMember( ( 'borrowernumber' => $loggedinuser ) ); @@ -472,7 +471,7 @@ if ( $op eq "add" ) { print $input->redirect($redirectpath); } elsif ( $op eq 'attachbasket') { # -# save a modified basketgroup, or creates a new basketgroup when a basket is closed. called from basket page +# save a modified basketgroup, or put an existing basket in a basketgroup (called from basket page) # # Getting parameters my $basketgroup = {}; @@ -484,39 +483,28 @@ if ( $op eq "add" ) { my $deliveryplace = $input->param('deliveryplace'); my $freedeliveryplace = $input->param('freedeliveryplace'); my $deliverycomment = $input->param('deliverycomment'); - my $closedbg = $input->param('closedbg') ? 1 : 0; + # if "close" box is checked in form, $close is set to 1, and ModBasketgroup/NewBasketgroup closes the basketgroup by setting bg_closedate field to now() + my $close = $input->param('close'); + $basketgroup = { + name => $basketgroupname, + booksellerid => $booksellerid, + basketlist => \@baskets, + billingplace => $billingplace, + deliveryplace => $deliveryplace, + freedeliveryplace => $freedeliveryplace, + deliverycomment => $deliverycomment, + closed => $close, + }; if ($basketgroupid) { # If we have a basketgroupid we edit the basketgroup - $basketgroup = { - name => $basketgroupname, - id => $basketgroupid, - basketlist => \@baskets, - billingplace => $billingplace, - deliveryplace => $deliveryplace, - freedeliveryplace => $freedeliveryplace, - deliverycomment => $deliverycomment, - closed => $closedbg, - }; + $basketgroup->{id} = $basketgroupid; ModBasketgroup($basketgroup); - if($closedbg){ -# FIXME - } }else{ - # we create a new basketgroup (whith a closed basket) - $basketgroup = { - name => $basketgroupname, - booksellerid => $booksellerid, - basketlist => \@baskets, - billingplace => $billingplace, - deliveryplace => $deliveryplace, - freedeliveryplace => $freedeliveryplace, - deliverycomment => $deliverycomment, - closed => $closedbg, - }; + # Else we create a new basketgroup $basketgroupid = NewBasketgroup($basketgroup); } my $redirectpath = ((defined $input->param('mode')) && ($input->param('mode') eq 'singlebg')) ?'/cgi-bin/koha/acqui/basketgroup.pl?op=add&basketgroupid='.$basketgroupid.'&booksellerid='.$booksellerid : '/cgi-bin/koha/acqui/basketgroup.pl?booksellerid=' . $booksellerid; - $redirectpath .= "&listclosed=1" if $closedbg ; + $redirectpath .= "&listclosed=1" if $close ; print $input->redirect($redirectpath ); }else{ diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 136104f..2a8ca40 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -2767,12 +2767,13 @@ DROP TABLE IF EXISTS `aqbasketgroups`; CREATE TABLE `aqbasketgroups` ( `id` int(11) NOT NULL auto_increment, `name` varchar(50) default NULL, - `closed` tinyint(1) default NULL, `booksellerid` int(11) NOT NULL, `deliveryplace` varchar(10) default NULL, `freedeliveryplace` text default NULL, `deliverycomment` varchar(255) default NULL, `billingplace` varchar(10) default NULL, + bg_creationdate date default NULL, -- the date the basketgroup was created + bg_closedate date default NULL, -- the date the basketgroup was closed PRIMARY KEY (`id`), KEY `booksellerid` (`booksellerid`), CONSTRAINT `aqbasketgroups_ibfk_1` FOREIGN KEY (`booksellerid`) REFERENCES `aqbooksellers` (`id`) ON UPDATE CASCADE ON DELETE CASCADE diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 0220b42..e55773d 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -7878,6 +7878,17 @@ if(CheckVersion($DBversion)) { SetVersion($DBversion); } +$DBversion = "3.15.00.XXX"; +if(CheckVersion($DBversion)) { + $dbh->do("ALTER TABLE aqbasketgroups ADD bg_creationdate DATE DEFAULT NULL AFTER billingplace;"); + $dbh->do("ALTER TABLE aqbasketgroups ADD bg_closedate DATE DEFAULT NULL AFTER bg_creationdate;"); + $dbh->do("UPDATE aqbasketgroups SET bg_creationdate=(SELECT MAX(creationdate) FROM aqbasket WHERE aqbasket.basketgroupid=aqbasketgroups.id)"); + $dbh->do("UPDATE aqbasketgroups SET bg_closedate=(SELECT MAX(creationdate) FROM aqbasket WHERE aqbasket.basketgroupid=aqbasketgroups.id) WHERE aqbasketgroups.closed = 1"); + $dbh->do("ALTER TABLE aqbasketgroups DROP COLUMN closed;"); + print "Upgrade to $DBversion done (Bug 10919)\n"; + SetVersion($DBversion); +} + =head1 FUNCTIONS =head2 TableExists($table) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketgroup.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketgroup.tt index 6aff7da..fe9aec8 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketgroup.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketgroup.tt @@ -1,4 +1,6 @@ [% USE Branches %] +[% USE KohaDates %] + [% INCLUDE 'doc-head-open.inc' %] Koha › Basket grouping for [% booksellername |html %] @@ -96,16 +98,6 @@ var MSG_SAVE_BEFORE_PRINTING = _("You need to save the page before printing"); var MSG_REOPEN_BASKETGROUP = _("reopen basketgroup"); var MSG_FILE_DOWNLOAD_ERROR = _("Error downloading the file"); -function submitForm(form) { - if (form.close.checked == true) { - var input = document.createElement("input"); - input.setAttribute("type", "hidden"); - input.setAttribute("name", "closed"); - input.setAttribute("value", "1"); - form.appendChild(input); - } -} - $(document).ready(function() { $("#basket_groups").tabs(); @@ -139,7 +131,7 @@ function submitForm(form) {
[% IF ( grouping ) %] - [% IF (closedbg) %] + [% IF (bg_closedate) %]
@@ -148,7 +140,7 @@ function submitForm(form) { [% ELSE %] [% END %] - [% IF (name && closedbg) %] + [% IF (name && bg_closedate) %]

Basket group [% name %] ([% basketgroupid %]) for [% booksellername |html %]

[% ELSIF (name) %]

Edit basket group [% name %] ([% basketgroupid %]) for [% booksellername |html %]

@@ -156,7 +148,7 @@ function submitForm(form) {

Add basket group for [% booksellername |html %]

[% END %]
- [% UNLESS (closedbg) %] + [% UNLESS (bg_closedate) %]
@@ -190,7 +182,21 @@ function submitForm(form) {
    - [% UNLESS (closedbg) %] + [% IF (bg_creationdate) %] +
  1. + + [% bg_creationdate | $KohaDates %] +
  2. + + [% END %] + [% IF (bg_closedate ) %] +
  3. + + [% bg_closedate | $KohaDates %] +
  4. + + [% END %] + [% UNLESS (bg_closedate) %]
  5. @@ -199,7 +205,7 @@ function submitForm(form) { [% END %]
  6. - [% UNLESS (closedbg) %] + [% UNLESS (bg_closedate) %] @@ -250,7 +256,7 @@ function submitForm(form) {
  7. [% END %]
  8. - [% UNLESS (closedbg) %] + [% UNLESS (bg_closedate) %] [% ELSE %] @@ -260,7 +266,7 @@ function submitForm(form) {
  9. Baskets in this group: - [% UNLESS (closedbg) %] + [% UNLESS (bg_closedate) %]
      [% ELSE %]
        @@ -280,14 +286,12 @@ function submitForm(form) { [% END %]
      - [% UNLESS (closedbg) %] -
    • - [% ELSE %] - + [% UNLESS (bg_closedate) %] +
    • [% END %]
- [% UNLESS (closedbg) %] + [% UNLESS (bg_closedate) %]
[% IF ( basketgroupid ) %] @@ -320,12 +324,14 @@ function submitForm(form) { Billing place Delivery place Number of baskets + Created on + Closed on Action [% FOREACH basketgroup IN basketgroups %] - [% UNLESS ( basketgroup.closed ) %] + [% UNLESS ( basketgroup.bg_closedate ) %] [% IF ( basketgroup.name ) %] [% basketgroup.name %] @@ -337,6 +343,8 @@ function submitForm(form) { [% Branches.GetName( basketgroup.billingplace ) %] [% IF (basketgroup.freedeliveryplace) %]Free delivery place[% ELSE %][% Branches.GetName( basketgroup.deliveryplace ) %][% END %] [% basketgroup.basketsqty %] + [% basketgroup.bg_creationdate %] + [% basketgroup.bg_closedate %] @@ -357,12 +365,14 @@ function submitForm(form) { Billing place Delivery place Number of baskets + Created on + Closed on Action [% FOREACH basketgroup IN basketgroups %] - [% IF ( basketgroup.closed ) %] + [% IF ( basketgroup.bg_closedate ) %] [% IF ( basketgroup.name ) %] @@ -375,6 +385,8 @@ function submitForm(form) { [% Branches.GetName( basketgroup.billingplace ) %] [% IF (basketgroup.freedeliveryplace) %]Free delivery place[% ELSE %][% Branches.GetName( basketgroup.deliveryplace ) %][% END %] [% basketgroup.basketsqty %] + [% basketgroup.bg_creationdate %] + [% basketgroup.bg_closedate %]
-- 1.7.9.5