From cc68bbb8e1a81cf64253b70b0222a93cefe9cc4c Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 1 Apr 2020 16:47:08 +0000 Subject: [PATCH] Bug 25037: Replace onsite_checkout with checkout_type To test: 1. Run all these tests: prove t/db_dependent/Circulation.t prove t/db_dependent/Circulation/TooMany.t prove t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t prove t/db_dependent/Circulation/SwitchOnSiteCheckouts.t prove t/db_dependent/Items.t prove t/db_dependent/Koha/Account/Line.t 2. Verify koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt - Create a record and add two items to it - Checkout both of them, one as on-site checkout and one as normal - Go to INTRANET /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX where XXX is your record's biblionumber - Observe Holdings table column Status => your on-site checked out item should say "Currently in local use by ..." => your normal checked out item should say "Checked out to ..." 2. Verify koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc - Repeat step 1 but this time go to OPAC /cgi-bin/koha/opac-detail.pl?biblionumber=XXX where XXX is your record's biblionumber - Observe Holdings table column Status => your on-site checked out item should say "Currently in local use" => your normal checked out item should say "Checked out" 3. Verify koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt - After step 2, go to INTRANET /cgi-bin/koha/members/readingrec.pl?borrowernumber=51 - You will see circulation history table. - Observe your two checkouts from step 2 - Click "Checkouts" tab - Observe your normal checkout in the list - Click "On-site checkouts"" tab - Observe your on-site checkout in the list 4. Verify koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt - Repeat step 3 but this time go to OPAC /cgi-bin/koha/opac-readingrecord.pl --- C4/Circulation.pm | 15 ++++++++------- C4/Items.pm | 2 +- api/v1/swagger/definitions/checkout.json | 6 +++--- .../prog/en/modules/catalogue/detail.tt | 4 ++-- .../prog/en/modules/members/readingrec.tt | 2 +- koha-tmpl/intranet-tmpl/prog/js/checkouts.js | 6 +++--- .../bootstrap/en/includes/item-status.inc | 4 ++-- .../bootstrap/en/modules/opac-readingrecord.tt | 2 +- svc/checkouts | 4 ++-- t/db_dependent/Circulation.t | 4 ++-- .../Circulation/SwitchOnSiteCheckouts.t | 2 +- t/db_dependent/Items.t | 4 ++-- t/db_dependent/Koha/Account/Line.t | 4 ++-- 13 files changed, 30 insertions(+), 29 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 3374ebef79..e89efaf691 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -412,7 +412,7 @@ sub TooMany { if (defined($maxissueqty_rule) and $maxissueqty_rule->rule_value ne '') { my @bind_params; my $count_query = q| - SELECT COUNT(*) AS total, COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts + SELECT COUNT(*) AS total, COALESCE(SUM(CASE WHEN checkout_type = 'ONSITE' THEN 1 ELSE 0 END), 0) AS onsite_checkouts FROM issues JOIN items USING (itemnumber) |; @@ -509,7 +509,7 @@ sub TooMany { if (defined($branch_borrower_circ_rule->{patron_maxissueqty}) and $branch_borrower_circ_rule->{patron_maxissueqty} ne '') { my @bind_params = (); my $branch_count_query = q| - SELECT COUNT(*) AS total, COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts + SELECT COUNT(*) AS total, COALESCE(SUM(CASE WHEN checkout_type = 'ONSITE' THEN 1 ELSE 0 END), 0) AS onsite_checkouts FROM issues JOIN items USING (itemnumber) WHERE borrowernumber = ? @@ -852,7 +852,7 @@ sub CanBookBeIssued { # If it is an on-site checkout if it can be switched to a normal checkout # or ask whether the loan should be renewed - if ( $issue->onsite_checkout + if ( defined $issue->checkout_type && $issue->checkout_type eq 'ONSITE' and C4::Context->preference('SwitchOnSiteCheckouts') ) { $messages{ONSITE_CHECKOUT_WILL_BE_SWITCHED} = 1; } else { @@ -902,7 +902,8 @@ sub CanBookBeIssued { my $switch_onsite_checkout = ( C4::Context->preference('SwitchOnSiteCheckouts') and $issue - and $issue->onsite_checkout + and defined $issue->checkout_type + and $issue->checkout_type eq 'ONSITE' and $issue->borrowernumber == $patron->borrowernumber ? 1 : 0 ); my $toomany = TooMany( $patron_unblessed, $item_object, { onsite_checkout => $onsite_checkout, switch_onsite_checkout => $switch_onsite_checkout, } ); # if TooMany max_allowed returns 0 the user doesn't have permission to check out this book @@ -1419,7 +1420,7 @@ sub AddIssue { issuedate => $issuedate->strftime('%Y-%m-%d %H:%M:%S'), date_due => $datedue->strftime('%Y-%m-%d %H:%M:%S'), branchcode => C4::Context->userenv->{'branch'}, - onsite_checkout => $onsite_checkout, + checkout_type => $onsite_checkout ? 'ONSITE' : undef, auto_renew => $auto_renew ? 1 : 0, }; @@ -2713,7 +2714,7 @@ sub CanBookBeRenewed { my $item = Koha::Items->find($itemnumber) or return ( 0, 'no_item' ); my $issue = $item->checkout or return ( 0, 'no_checkout' ); - return ( 0, 'onsite_checkout' ) if $issue->onsite_checkout; + return ( 0, 'onsite_checkout' ) if defined $issue->checkout_type && $issue->checkout_type eq 'ONSITE'; return ( 0, 'item_denied_renewal') if _item_denied_renewal({ item => $item }); my $patron = $issue->patron or return; @@ -4119,7 +4120,7 @@ sub GetPendingOnSiteCheckouts { LEFT JOIN issues ON items.itemnumber = issues.itemnumber LEFT JOIN biblio ON items.biblionumber = biblio.biblionumber LEFT JOIN borrowers ON issues.borrowernumber = borrowers.borrowernumber - WHERE issues.onsite_checkout = 1 + WHERE issues.checkout_type = 'ONSITE' |, { Slice => {} } ); } diff --git a/C4/Items.pm b/C4/Items.pm index 94808084ac..6bebc6da0f 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -660,7 +660,7 @@ sub GetItemsInfo { items.notforloan as itemnotforloan, issues.borrowernumber, issues.date_due as datedue, - issues.onsite_checkout, + issues.checkout_type, borrowers.cardnumber, borrowers.surname, borrowers.firstname, diff --git a/api/v1/swagger/definitions/checkout.json b/api/v1/swagger/definitions/checkout.json index 42b9598843..c925c452ac 100644 --- a/api/v1/swagger/definitions/checkout.json +++ b/api/v1/swagger/definitions/checkout.json @@ -52,9 +52,9 @@ "format": "date-time", "description": "Date the item was issued" }, - "onsite_checkout": { - "type": "boolean", - "description": "On site checkout" + "checkout_type": { + "type": ["string", "null"], + "description": "Checkout type, an authorised value under CHECKOUT_TYPE category" }, "note": { "type": ["string", "null"], diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt index 6ef518489b..557eacb663 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ -315,13 +315,13 @@ Note that permanent location is a code, and location may be an authval. [% IF item.CheckedOutFor %] - [% IF item.onsite_checkout %] + [% IF ( item.checkout_type && item.checkout_type == 'ONSITE' )%] Currently in local use [% ELSE %] Checked out [% END %] [% UNLESS ( item.NOTSAMEBRANCH ) %] - [% IF item.onsite_checkout %] + [% IF ( item.checkout_type && item.checkout_type == 'ONSITE' ) %] by [% ELSE %] to diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt index 6d7ae6b829..bdb26b9bf8 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt @@ -66,7 +66,7 @@ [% FOREACH issue IN loop_reading %] [% IF issue.returndate %][% ELSE %][% END %] - [% IF issue.onsite_checkout %][% issuetype = 'onsite_checkout' | html %] + [% IF ( issue.checkout_type && issue.checkout_type == 'ONSITE' ) %][% issuetype = 'onsite_checkout' | html %] [% ELSE %][% issuetype = 'standard_checkout' | html %] [% END %] [% issuetype | html %] diff --git a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js index e18c2a24c9..a88d39578c 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js +++ b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js @@ -327,7 +327,7 @@ $(document).ready(function() { } var onsite_checkout = ''; - if ( oObj.onsite_checkout == 1 ) { + if ( oObj.checkout_type === "ONSITE" ) { onsite_checkout += " (" + INHOUSE_USE + ")"; } @@ -507,7 +507,7 @@ $(document).ready(function() { span_class = "renewals-allowed"; } - var can_force_renew = ( oObj.onsite_checkout == 0 ) && + var can_force_renew = ( oObj.checkout_type !== 'ONSITE' ) && ( oObj.can_renew_error != "on_reserve" || (oObj.can_renew_error == "on_reserve" && AllowRenewalOnHoldOverride)) ? true : false; var can_renew = ( oObj.renewals_remaining > 0 && !oObj.can_renew_error ); @@ -701,7 +701,7 @@ $(document).ready(function() { } var onsite_checkout = ''; - if ( oObj.onsite_checkout == 1 ) { + if ( oObj.checkout_type === 'ONSITE' ) { onsite_checkout += " (" + INHOUSE_USE + ")"; } diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc index c15566a7be..4ef049ffaa 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc @@ -18,10 +18,10 @@ [% IF item.isa('Koha::Item') %] [% SET datedue = issue.date_due %] - [% SET onsite_checkout = issue.onsite_checkout %] + [% SET onsite_checkout = ( issue.checkout_type && issue.checkout_type == 'ONSITE' ) ? 1 : 0 %] [% ELSE %] [% SET datedue = item.datedue || issue.date_due %] - [% SET onsite_checkout = item.onsite_checkout %] + [% SET onsite_checkout = ( item.checkout_type && item.checkout_type == 'ONSITE' ) ? 1 : 0 %] [% END %] [% IF datedue %] [% SET itemavailable = 0 %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt index 238b2e534c..cb48cbc418 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt @@ -90,7 +90,7 @@ [% FOREACH issue IN READING_RECORD %] - [% IF issue.onsite_checkout %][% issuetype = 'onsite_checkout' %] + [% IF ( issue.checkout_type && issue.checkout_type == 'ONSITE' ) %][% issuetype = 'onsite_checkout' %] [% ELSE %][% issuetype = 'standard_checkout' %] [% END %] [% issuetype | html %] diff --git a/svc/checkouts b/svc/checkouts index fa470b3a2a..e0b58e9188 100755 --- a/svc/checkouts +++ b/svc/checkouts @@ -69,7 +69,7 @@ my $sql = ' issues.date_due < now() as date_due_overdue, issues.timestamp, - issues.onsite_checkout, + issues.checkout_type, biblio.biblionumber, biblio.title, @@ -220,7 +220,7 @@ while ( my $c = $sth->fetchrow_hashref() ) { date_due => $c->{date_due}, date_due_overdue => $c->{date_due_overdue} ? JSON::true : JSON::false, timestamp => $c->{timestamp}, - onsite_checkout => $c->{onsite_checkout}, + onsite_checkout => $c->{checkout_type} ? ( $c->{checkout_type} eq 'ONSITE' ? 1 : 0 ) : 0, enumchron => $c->{enumchron}, renewals_count => $renewals_count, renewals_allowed => $renewals_allowed, diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 104fc8fc85..5e52c61068 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -3133,7 +3133,7 @@ subtest 'ItemsDeniedRenewal preference' => sub { auto_renew => 0, borrowernumber => $idr_borrower->borrowernumber, itemnumber => $deny_book->itemnumber, - onsite_checkout => 0, + checkout_type => undef, date_due => $future, } }); @@ -3143,7 +3143,7 @@ subtest 'ItemsDeniedRenewal preference' => sub { auto_renew => 0, borrowernumber => $idr_borrower->borrowernumber, itemnumber => $allow_book->itemnumber, - onsite_checkout => 0, + checkout_type => undef, date_due => $future, } }); diff --git a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t index 779c19d84e..acbaa370b3 100644 --- a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t +++ b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t @@ -116,7 +116,7 @@ is( $messages->{ONSITE_CHECKOUT_WILL_BE_SWITCHED}, 1, 'If SwitchOnSiteCheckouts, is( exists $impossible->{TOO_MANY}, '', 'If SwitchOnSiteCheckouts, switch the on-site checkout' ); C4::Circulation::AddIssue( $patron_unblessed, $item->{barcode}, undef, undef, undef, undef, { switch_onsite_checkout => 1 } ); my $issue = Koha::Checkouts->find( { itemnumber => $item->{itemnumber} } ); -is( $issue->onsite_checkout, 0, 'The issue should have been switched to a regular checkout' ); +is( $issue->checkout_type, undef, 'The issue should have been switched to a regular checkout' ); my $five_days_after = dt_from_string->add( days => 5 )->set( hour => 23, minute => 59, second => 0 ); is( dt_from_string($issue->date_due, 'sql'), $five_days_after, 'The date_due should have been set depending on the circ rules when the on-site checkout has been switched' ); diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t index c07c66703e..21c0e0c92b 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -368,8 +368,8 @@ subtest 'GetItemsInfo tests' => sub { 'GetItemsInfo returns the correct home branch OPAC info notice' ); is( $results[0]->{ holding_branch_opac_info }, "holdingbranch OPAC info", 'GetItemsInfo returns the correct holding branch OPAC info notice' ); - is( exists( $results[0]->{ onsite_checkout } ), 1, - 'GetItemsInfo returns a onsite_checkout key' ); + is( exists( $results[0]->{ checkout_type } ), 1, + 'GetItemsInfo returns a checkout_type key' ); is( $results[0]->{ restricted }, 1, 'GetItemsInfo returns a restricted value code' ); is( $results[0]->{ restrictedvalue }, "Restricted Access", diff --git a/t/db_dependent/Koha/Account/Line.t b/t/db_dependent/Koha/Account/Line.t index 0b35a590c6..98d902d9ab 100644 --- a/t/db_dependent/Koha/Account/Line.t +++ b/t/db_dependent/Koha/Account/Line.t @@ -360,7 +360,7 @@ subtest 'Renewal related tests' => sub { class => 'Koha::Checkouts', value => { itemnumber => $item->itemnumber, - onsite_checkout => 0, + checkout_type => undef, renewals => 99, auto_renew => 0 } @@ -406,7 +406,7 @@ subtest 'Renewal related tests' => sub { class => 'Koha::Checkouts', value => { itemnumber => $item->itemnumber, - onsite_checkout => 0, + checkout_type => undef, renewals => 0, auto_renew => 0 } -- 2.17.1