From 9415560a9995ac241047247eea6ae4b40de61dd9 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.pl creates the 2 columns and removes 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 You have 2 elements to test: - the behavior of Koha after the patch is applied - the changes made by updatedatabase.pl Plan 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 installer/data/mysql/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 | 56 +++++------- installer/data/mysql/kohastructure.sql | 3 +- installer/data/mysql/updatedatabase.pl | 11 +++ .../prog/en/modules/acqui/basketgroup.tt | 62 ++++++++------ 6 files changed, 142 insertions(+), 103 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 33aa8e3..08c4c98 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); @@ -828,22 +828,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 @@ -852,21 +854,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'}}) { @@ -885,22 +895,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 @@ -910,7 +922,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}); @@ -918,6 +930,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 6b2529d..a4dae61 100755 --- a/acqui/basketgroup.pl +++ b/acqui/basketgroup.pl @@ -246,8 +246,8 @@ $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 +# if param('basketgroupid') is not defined, adds a new basketgroup +# otherwise, edits (if it is open) or displays (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 # my $bookseller = &GetBookSellerFromId($booksellerid); @@ -270,13 +270,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 ) ); @@ -347,7 +346,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 = {}; @@ -359,41 +358,30 @@ 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, - }; + # Otherwise 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{ # no param : display the list of all basketgroups for a given vendor my $basketgroups = &GetBasketgroups($booksellerid); diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 3cf9c65..f5342ba 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -2761,12 +2761,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 062ecc1..998e606 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -8083,6 +8083,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 11438 - Add creation date and closing date to basketgroups)\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