From 1fe3514c3cca5ae73b370411150112bf2647c63d Mon Sep 17 00:00:00 2001 From: Paul Poulain Date: Wed, 11 May 2011 16:54:56 +0200 Subject: [PATCH] Bug 6328 fine in days does not work Some code coming from BibLibre has been lost in the process of inclusion in 3.4. The result is that fine in days does not work at all (you can setup rules, but it does nothing) Step to reproduce: - Koha > Admin > circ rules > set 1 day fine every day of overdue for default rule - Issue a book return date last week - check-in the book => no debarment is set The following patch will fix all of those problems by : * updating borrowers.debarred to a date field (instead of tinyint). It contains the limit of the debarment * changing API of DebarMember and UpdateBorrowerDebarred to pass a date * display debarrdate where applicable. Note that a debarrdate of 31/12/9999 is considered as unlimited and not displayed * added a debarrcomment, usefull to explain why a patron is debarred (this is independant from debarrdate changes and can be used when placing an unlimited debarment too) [2011-05-12] F. Demians. It works as described. And I can confirm this functionality is impatiently awaited by French libraries since one year. Thanks BibLibre for the good work and for contributing this code. Bug 6328 Followup--update DB structure Thanks Katrin. Bug 6328: make comment a textbox / fix debar by notice trigger Debarring by notice triggers was broken, because the new function expects a date as second parameter. The comment field in patron account details was a very long text field. Patch changes it to be a textbox instead. Bug 6328: Lift debarment leaves patron account 'Lift debarment' redirects to an empty circulation page. BZ6328 follow-up 3 Fixes comment 23 from Fernando L. Canizo : when the patron was debarred and debar removed he still could not check-out. The changes in the IsMemberBlocked (that were on biblibre/master) were lost somewhere The sub was still checking for old_issues instead of calling CheckBorrowerDebarred to get a debardate if applicable Note : this bug was appearing only is you had issuing rules defined for itemtype/categorycode/branch. Seemed to work if you had only default rules. That's probably why it hadn't been spotted before BZ6328 follow-up 4 Comments fron Zeno Tajoli: The patch is OK and I sign-off it. Two little changes done on installer/data/mysql/kohastructure.sql and installer/data/mysql/updatedatabase.pl Signed-off-by: koha --- C4/Circulation.pm | 60 +++++++++++++++++++ C4/Members.pm | 48 ++++----------- C4/Overdues.pm | 23 +++---- circ/circulation.pl | 11 ++++ circ/returns.pl | 7 ++- installer/data/mysql/kohastructure.sql | 6 +- installer/data/mysql/updatedatabase.pl | 15 +++++ .../prog/en/modules/circ/circulation.tt | 11 ++++ .../prog/en/modules/circ/returns.tt | 3 + .../prog/en/modules/members/memberentrygen.tt | 35 ++++++++++- .../prog/en/modules/members/moremember.tt | 7 +-- .../opac-tmpl/prog/en/modules/opac-user.tt | 2 +- members/memberentry.pl | 19 +++++- members/moremember.pl | 12 +++- members/setstatus.pl | 4 +- misc/cronjobs/overdue_notices.pl | 2 +- opac/opac-user.pl | 1 + 17 files changed, 203 insertions(+), 63 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index d2c5f5926b..debef01db5 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -42,6 +42,7 @@ use Date::Calc qw( Date_to_Days Day_of_Week Add_Delta_Days + check_date ); use POSIX qw(strftime); use C4::Branch; # GetBranches @@ -1631,6 +1632,10 @@ sub AddReturn { if ($borrowernumber) { my $fix = _FixOverduesOnReturn($borrowernumber, $item->{itemnumber}, $exemptfine, $dropbox); defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $item->{itemnumber}...) failed!"; # zero is OK, check defined + + # fix fine days + my $debardate = _FixFineDaysOnReturn( $borrower, $item, $issue->{date_due} ); + $messages->{'Debarred'} = $debardate if ($debardate); } # find reserves..... @@ -1754,6 +1759,61 @@ sub MarkIssueReturned { $sth_del->execute($borrowernumber, $itemnumber); } +=head2 _FixFineDaysOnReturn + + &_FixFineDaysOnReturn($borrower, $item, $datedue); + +C<$borrower> borrower hashref + +C<$item> item hashref + +C<$datedue> date due + +Internal function, called only by AddReturn that calculate and update the user fine days, and debars him + +=cut + +sub _FixFineDaysOnReturn { + my ( $borrower, $item, $datedue ) = @_; + + if ($datedue) { + $datedue = C4::Dates->new( $datedue, "iso" ); + } else { + return; + } + + my $branchcode = _GetCircControlBranch( $item, $borrower ); + my $calendar = C4::Calendar->new( branchcode => $branchcode ); + my $today = C4::Dates->new(); + + my $deltadays = $calendar->daysBetween( $datedue, C4::Dates->new() ); + + my $circcontrol = C4::Context::preference('CircControl'); + my $issuingrule = GetIssuingRule( $borrower->{categorycode}, $item->{itype}, $branchcode ); + my $finedays = $issuingrule->{finedays}; + + # exit if no finedays defined + return unless $finedays; + my $grace = $issuingrule->{firstremind}; + + if ( $deltadays - $grace > 0 ) { + my @newdate = Add_Delta_Days( Today(), $deltadays * $finedays ); + my $isonewdate = join( '-', @newdate ); + my ( $deby, $debm, $debd ) = split( /-/, $borrower->{debarred} ); + if ( check_date( $deby, $debm, $debd ) ) { + my @olddate = split( /-/, $borrower->{debarred} ); + + if ( Delta_Days( @olddate, @newdate ) > 0 ) { + C4::Members::DebarMember( $borrower->{borrowernumber}, $isonewdate ); + return $isonewdate; + } + } else { + C4::Members::DebarMember( $borrower->{borrowernumber}, $isonewdate ); + return $isonewdate; + } + } +} + =head2 _FixOverduesOnReturn &_FixOverduesOnReturn($brn,$itm, $exemptfine, $dropboxmode); diff --git a/C4/Members.pm b/C4/Members.pm index 4f5a299589..27b6b0ecc0 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -623,39 +623,12 @@ sub IsMemberBlocked { my $borrowernumber = shift; my $dbh = C4::Context->dbh; - # does patron have current fine days? - my $strsth=qq{ - SELECT - ADDDATE(returndate, finedays * DATEDIFF(returndate,date_due) ) AS blockingdate, - DATEDIFF(ADDDATE(returndate, finedays * DATEDIFF(returndate,date_due)),NOW()) AS blockedcount - FROM old_issues - }; - if(C4::Context->preference("item-level_itypes")){ - $strsth.= - qq{ LEFT JOIN items ON (items.itemnumber=old_issues.itemnumber) - LEFT JOIN issuingrules ON (issuingrules.itemtype=items.itype)} - }else{ - $strsth .= - qq{ LEFT JOIN items ON (items.itemnumber=old_issues.itemnumber) - LEFT JOIN biblioitems ON (biblioitems.biblioitemnumber=items.biblioitemnumber) - LEFT JOIN issuingrules ON (issuingrules.itemtype=biblioitems.itemtype) }; - } - $strsth.= - qq{ WHERE finedays IS NOT NULL - AND date_due < returndate - AND borrowernumber = ? - ORDER BY blockingdate DESC, blockedcount DESC - LIMIT 1}; - my $sth=$dbh->prepare($strsth); - $sth->execute($borrowernumber); - my $row = $sth->fetchrow_hashref; - my $blockeddate = $row->{'blockeddate'}; - my $blockedcount = $row->{'blockedcount'}; + my $blockeddate = CheckBorrowerDebarred($borrowernumber); - return (1, $blockedcount) if $blockedcount > 0; + return ( 1, $blockeddate ) if $blockeddate; # if he have late issues - $sth = $dbh->prepare( + my $sth = $dbh->prepare( "SELECT COUNT(*) as latedocs FROM issues WHERE borrowernumber = ? @@ -664,9 +637,9 @@ sub IsMemberBlocked { $sth->execute($borrowernumber); my $latedocs = $sth->fetchrow_hashref->{'latedocs'}; - return (-1, $latedocs) if $latedocs > 0; + return ( -1, $latedocs ) if $latedocs > 0; - return (0, 0); + return ( 0, 0 ); } =head2 GetMemberIssuesAndFines @@ -2078,7 +2051,7 @@ sub GetBorrowersNamesAndLatestIssue { =head2 DebarMember - my $success = DebarMember( $borrowernumber ); +my $success = DebarMember( $borrowernumber, $todate ); marks a Member as debarred, and therefore unable to checkout any more items. @@ -2090,13 +2063,16 @@ true on success, false on failure sub DebarMember { my $borrowernumber = shift; + my $todate = shift; return unless defined $borrowernumber; return unless $borrowernumber =~ /^\d+$/; - return ModMember( borrowernumber => $borrowernumber, - debarred => 1 ); - + return ModMember( + borrowernumber => $borrowernumber, + debarred => $todate + ); + } =head2 ModPrivacy diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 9f4b22f920..7e9b9c9c11 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -1077,16 +1077,17 @@ sub CheckBorrowerDebarred { SELECT debarred FROM borrowers WHERE borrowernumber=? + AND debarred > NOW() |; my $sth = $dbh->prepare($query); $sth->execute($borrowernumber); - my ($debarredstatus) = $sth->fetchrow; - return ( $debarredstatus eq '1' ? 1 : 0 ); + my $debarredstatus = $sth->fetchrow; + return $debarredstatus; } =head2 UpdateBorrowerDebarred - ($borrowerstatut) = &UpdateBorrowerDebarred($borrowernumber); +($borrowerstatut) = &UpdateBorrowerDebarred($borrowernumber, $todate); update status of borrowers in borrowers table (field debarred) @@ -1095,16 +1096,16 @@ C<$borrowernumber> borrower number =cut sub UpdateBorrowerDebarred{ - my($borrowernumber) = @_; - my $dbh = C4::Context->dbh; - my $query=qq|UPDATE borrowers - SET debarred='1' + my ( $borrowernumber, $todate ) = @_; + my $dbh = C4::Context->dbh; + my $query = qq|UPDATE borrowers + SET debarred=? WHERE borrowernumber=? |; - my $sth=$dbh->prepare($query); - $sth->execute($borrowernumber); - $sth->finish; - return 1; + my $sth = $dbh->prepare($query); + $sth->execute( $todate, $borrowernumber ); + $sth->finish; + return 1; } =head2 CheckExistantNotifyid diff --git a/circ/circulation.pl b/circ/circulation.pl index 378956e654..fa043dddde 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -30,6 +30,7 @@ use C4::Dates qw/format_date/; use C4::Branch; # GetBranches use C4::Koha; # GetPrinter use C4::Circulation; +use C4::Overdues qw/CheckBorrowerDebarred/; use C4::Members; use C4::Biblio; use C4::Reserves; @@ -260,6 +261,16 @@ if ($borrowernumber) { issuecount => $issue, finetotal => $fines ); + + my $debar = CheckBorrowerDebarred($borrowernumber); + if ($debar) { + $template->param( 'userdebarred' => 1 ); + $template->param( 'debarredcomment' => $borrower->{debarredcomment} ); + if ( $debar ne "9999-12-31" ) { + $template->param( 'userdebarreddate' => C4::Dates::format_date($debar) ); + } + } + } # diff --git a/circ/returns.pl b/circ/returns.pl index afe574f7a1..98d310e124 100755 --- a/circ/returns.pl +++ b/circ/returns.pl @@ -459,7 +459,12 @@ foreach my $code ( keys %$messages ) { } elsif ( $code eq 'Wrongbranch' ) { } - + elsif ( $code eq 'Debarred' ) { + $err{debarred} = format_date( $messages->{'Debarred'} ); + $err{debarcardnumber} = $borrower->{cardnumber}; + $err{debarborrowernumber} = $borrower->{borrowernumber}; + $err{debarname} = "$borrower->{firstname} $borrower->{surname}"; + } else { die "Unknown error code $code"; # note we need all the (empty) elsif's above, or we die. # This forces the issue of staying in sync w/ Circulation.pm diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index e4388a461d..452173de92 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -232,7 +232,8 @@ CREATE TABLE `borrowers` ( -- this table includes information about your patrons `dateexpiry` date default NULL, -- date the patron/borrower's card is set to expire (YYYY-MM-DD) `gonenoaddress` tinyint(1) default NULL, -- set to 1 for yes and 0 for no, flag to note that library marked this patron/borrower as having an unconfirmed address `lost` tinyint(1) default NULL, -- set to 1 for yes and 0 for no, flag to note that library marked this patron/borrower as having lost their card - `debarred` tinyint(1) default NULL, -- set to 1 for yes and 0 for no, flag to note that library marked this patron/borrower as being restricted + `debarred` date default NULL, -- until this date the patron can only check-in (no loans, no holds, etc.), is a fine based on days instead of money (YYY-MM-DD) + `debarredcomment` VARCHAR(255) DEFAULT NULL, -- comment on the stop of the patron `contactname` mediumtext, -- used for children and profesionals to include surname or last name of guarentor or organization name `contactfirstname` text, -- used for children to include first name of guarentor `contacttitle` text, -- used for children to include title (Mr., Mrs., etc) of guarentor @@ -693,7 +694,8 @@ CREATE TABLE `deletedborrowers` ( -- stores data related to the patrons/borrower `dateexpiry` date default NULL, -- date the patron/borrower's card is set to expire (YYYY-MM-DD) `gonenoaddress` tinyint(1) default NULL, -- set to 1 for yes and 0 for no, flag to note that library marked this patron/borrower as having an unconfirmed address `lost` tinyint(1) default NULL, -- set to 1 for yes and 0 for no, flag to note that library marked this patron/borrower as having lost their card - `debarred` tinyint(1) default NULL, -- set to 1 for yes and 0 for no, flag to note that library marked this patron/borrower as being restricted + `debarred` date default NULL, -- until this date the patron can only check-in (no loans, no holds, etc.), is a fine based on days instead of money (YYY-MM-DD) + `debarredcomment` VARCHAR(255) DEFAULT NULL, -- comment on the stop of patron `contactname` mediumtext, -- used for children and profesionals to include surname or last name of guarentor or organization name `contactfirstname` text, -- used for children to include first name of guarentor `contacttitle` text, -- used for children to include title (Mr., Mrs., etc) of guarentor diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 815a71d65f..5a1dc97853 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -4438,6 +4438,7 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) { SetVersion($DBversion); } + $DBversion = "3.05.00.011"; if (C4::Context->preference("Version") < TransformToNum($DBversion)) { $dbh->do("INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('OPACResultsSidebar','','Define HTML to be included on the search results page, underneath the facets sidebar','70|10','Textarea')"); @@ -4550,6 +4551,20 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) { SetVersion ($DBversion); } +$DBversion = "3.05.00.XXX"; +if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { + my $borrowers = $dbh->selectcol_arrayref( "SELECT borrowernumber from borrowers where debarred <>0;", { Columns => [1] } ); + $dbh->do("ALTER TABLE borrowers MODIFY debarred DATE DEFAULT NULL;"); + $dbh->do( "UPDATE borrowers set debarred='9999-12-31' where borrowernumber IN (" . join( ",", @$borrowers ) . ");" ) if ($borrowers and scalar(@$borrowers)>0); + $dbh->do("ALTER TABLE borrowers ADD COLUMN debarredcomment VARCHAR(255) DEFAULT NULL AFTER debarred;"); + $dbh->do("ALTER TABLE deletedborrowers MODIFY debarred DATE DEFAULT NULL;"); + $dbh->do("ALTER TABLE deletedborrowers ADD COLUMN debarredcomment VARCHAR(255) DEFAULT NULL AFTER debarred;"); + print "Upgrade done (Change borrowers.debarred into Date )\n"; + + SetVersion($DBversion); +} + + =head1 FUNCTIONS =head2 DropAllForeignKeys($table) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt index f98ff1db3a..5fadb18129 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -563,6 +563,17 @@ No patron matched [% message %]
  • Lost: Patron's card is lost
    • [% END %] + [% IF ( userdebarred ) %] +
  • + Restricted: Patron's account is restricted [% IF (userdebarreddate ) %] until [% userdebarreddate %] [% END %] [% IF (debarredcomment ) %]([% debarredcomment %])[% END %] +
    + + + + +
    +
    • [% END %] + [% IF ( dbarred ) %]
  • Restricted: Patron's account is restricted Lift restriction
    • [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt index 65df373777..70367aedda 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt @@ -321,6 +321,9 @@ function Dopop(link) { [% IF ( errmsgloo.withdrawn ) %]

    Item is withdrawn.

    [% END %] + [% IF ( errmsgloo.debarred ) %] +

    [% errmsgloo.debarname %]([% errmsgloo.debarcardnumber %]) is now debarred until [% errmsgloo.debarred %]

    + [% END %] [% END %] [% IF ( soundon ) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt index 47ea6c9a97..937b28e75a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -1127,10 +1127,43 @@ [% END %] - + [% END %] +
  • + + [% IF ( debarred ) %] + + + + + [% ELSE %] + + + + + [% END %] + +
    + + + Show Calendar + +
    + + +
    • + + [% END %] [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt index cce4cd9639..454032ca42 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ -164,12 +164,11 @@ function validate1(date) { [% IF ( flagged ) %]