From 9d36aa2cf40994e4a48db6840a778367f8f91ad7 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 1 Apr 2020 16:47:08 +0000 Subject: [PATCH] Bug 25037: Replace occurences to issues.onsite_checkout This patch replaces all occurences to issues.onsite_checkout so that on-site checkouts work normally. It does not replace any on-site checkout logic. To test: 1. Investigate Koha source files via command line. We must make sure there are no longer references to issues.onsite_checkout column. You can do this by searching all occurrences of onsite_checkout. grep -rn 'onsite_checkout' | grep -v '\.git' 2. Run all these commands in your command line interface: grep -rl --color=never 'onsite_checkout' | grep '^.*\.t$' | xargs prove prove t/db_dependent/Koha/Account/Line.t prove t/db_dependent/api/v1/checkouts.t 3. 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 ..." 4. Verify koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc - Repeat step 3 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" 5. Verify koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt - After step 4, go to INTRANET /cgi-bin/koha/members/readingrec.pl?borrowernumber=51 - You will see circulation history table. - Observe your two checkouts from step 3 - Click "Checkouts" tab - Observe your normal checkout in the list - Click "On-site checkouts"" tab - Observe your on-site checkout in the list 6. Verify koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt - Repeat step 5 but this time go to OPAC /cgi-bin/koha/opac-readingrecord.pl Sponsored-by: The National Library of Finland --- C4/Circulation.pm | 22 ++++++++++--------- C4/Items.pm | 6 ++++- api/v1/swagger/definitions/checkout.json | 6 ++--- .../prog/en/modules/members/readingrec.tt | 3 ++- koha-tmpl/intranet-tmpl/prog/js/checkouts.js | 6 ++--- .../bootstrap/en/includes/item-status.inc | 5 +++-- .../en/modules/opac-readingrecord.tt | 3 ++- svc/checkouts | 5 +++-- t/db_dependent/Circulation.t | 2 -- .../Circulation/SwitchOnSiteCheckouts.t | 2 +- t/db_dependent/Items.t | 4 +++- t/db_dependent/Koha/Account/Line.t | 2 -- 12 files changed, 37 insertions(+), 29 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 8db887db2e..0510f7fc59 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -411,8 +411,8 @@ sub TooMany { # rule 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 + my $count_query = qq| + SELECT COUNT(*) AS total, COALESCE(SUM(CASE WHEN checkout_type = '$Koha::Checkouts::type->{onsite_checkout}' THEN 1 ELSE 0 END), 0) AS onsite_checkouts FROM issues JOIN items USING (itemnumber) |; @@ -508,8 +508,8 @@ sub TooMany { my $branch_borrower_circ_rule = GetBranchBorrowerCircRule($branch, $cat_borrower); 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 + my $branch_count_query = qq| + SELECT COUNT(*) AS total, COALESCE(SUM(CASE WHEN checkout_type = '$Koha::Checkouts::type->{onsite_checkout}' 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 ( $issue->is_onsite_checkout and C4::Context->preference('SwitchOnSiteCheckouts') ) { $messages{ONSITE_CHECKOUT_WILL_BE_SWITCHED} = 1; } else { @@ -902,7 +902,7 @@ sub CanBookBeIssued { my $switch_onsite_checkout = ( C4::Context->preference('SwitchOnSiteCheckouts') and $issue - and $issue->onsite_checkout + and $issue->is_onsite_checkout 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 +1419,9 @@ 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 ? + $Koha::Checkouts::type->{onsite_checkout} + : $Koha::Checkouts::type->{checkout}, auto_renew => $auto_renew ? 1 : 0, }; @@ -2721,7 +2723,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 $issue->is_onsite_checkout; return ( 0, 'item_denied_renewal') if _item_denied_renewal({ item => $item }); my $patron = $issue->patron or return; @@ -4106,7 +4108,7 @@ sub GetAgeRestriction { sub GetPendingOnSiteCheckouts { my $dbh = C4::Context->dbh; - return $dbh->selectall_arrayref(q| + return $dbh->selectall_arrayref(qq| SELECT items.barcode, items.biblionumber, @@ -4127,7 +4129,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 = '$Koha::Checkouts::type->{onsite_checkout}' |, { Slice => {} } ); } diff --git a/C4/Items.pm b/C4/Items.pm index a162ac5dec..607687028a 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -66,6 +66,7 @@ use Koha::AuthorisedValues; use Koha::DateUtils qw(dt_from_string); use Koha::Database; +use Koha::Checkouts; use Koha::Biblioitems; use Koha::Items; use Koha::ItemTypes; @@ -672,7 +673,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, @@ -728,6 +729,9 @@ sub GetItemsInfo { $data->{notforloanvalue} = $descriptions->{lib} // ''; $data->{notforloanvalueopac} = $descriptions->{opac_description} // ''; + # is onsite checkout + $data->{onsite_checkout} = defined $data->{checkout_type} && $data->{checkout_type} eq $Koha::Checkouts::type->{onsite_checkout} ? 1 : 0; + # get restricted status and description if applicable $descriptions = Koha::AuthorisedValues->get_description_by_koha_field({frameworkcode => $data->{frameworkcode}, kohafield => 'items.restricted', authorised_value => $data->{restricted} }); $data->{restrictedvalue} = $descriptions->{lib} // ''; 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/members/readingrec.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt index 6d7ae6b829..1feba22a69 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt @@ -4,6 +4,7 @@ [% USE Koha %] [% USE AuthorisedValues %] [% USE Branches %] +[% USE Checkouts %] [% USE ColumnsSettings %] [% SET footerjs = 1 %] [% INCLUDE 'doc-head-open.inc' %] @@ -66,7 +67,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 == Checkouts.checkout_type.onsite_checkout ) %][% 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 1b9b4a213d..c5d1d2318d 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.onsite_checkout ) { 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.onsite_checkout ) && ( 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.onsite_checkout ) { 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..c362997ea3 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc @@ -1,4 +1,5 @@ [% USE AuthorisedValues %] +[% USE Checkouts %] [% SET itemavailable = 1 %] [%#- This include takes two parameters: an item structure -%] @@ -18,10 +19,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 == Checkouts.checkout_type.onsite_checkout ) ? 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 == Checkouts.checkout_type.onsite_checkout ) ? 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 3b95b586ed..828dc869bd 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt @@ -1,6 +1,7 @@ [% USE raw %] [% USE Koha %] [% USE KohaDates %] +[% USE Checkouts %] [% INCLUDE 'doc-head-open.inc' %] [% IF ( LibraryNameTitle ) %][% LibraryNameTitle | html %][% ELSE %]Koha online[% END %] catalog › Your checkout history [% INCLUDE 'doc-head-close.inc' %] @@ -90,7 +91,7 @@ [% FOREACH issue IN READING_RECORD %] - [% IF issue.onsite_checkout %][% issuetype = 'onsite_checkout' %] + [% IF ( issue.checkout_type && issue.checkout_type == Checkouts.checkout_type.onsite_checkout ) %][% issuetype = 'onsite_checkout' %] [% ELSE %][% issuetype = 'standard_checkout' %] [% END %] [% issuetype | html %] diff --git a/svc/checkouts b/svc/checkouts index fa470b3a2a..bba7f2f084 100755 --- a/svc/checkouts +++ b/svc/checkouts @@ -28,6 +28,7 @@ use C4::Overdues qw(GetFine); use C4::Context; use Koha::AuthorisedValues; +use Koha::Checkouts; use Koha::DateUtils; use Koha::ItemTypes; @@ -69,7 +70,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 +221,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 $Koha::Checkouts::type->{onsite_checkout} ? 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 7ae3b9f694..3b384d0f02 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -3136,7 +3136,6 @@ subtest 'ItemsDeniedRenewal preference' => sub { auto_renew => 0, borrowernumber => $idr_borrower->borrowernumber, itemnumber => $deny_book->itemnumber, - onsite_checkout => 0, date_due => $future, } }); @@ -3146,7 +3145,6 @@ subtest 'ItemsDeniedRenewal preference' => sub { auto_renew => 0, borrowernumber => $idr_borrower->borrowernumber, itemnumber => $allow_book->itemnumber, - onsite_checkout => 0, date_due => $future, } }); diff --git a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t index 779c19d84e..41e4eb8433 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, $Koha::Checkouts::type->{checkout}, '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 61a691b458..e6ebc564fe 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -315,7 +315,7 @@ subtest 'GetHiddenItemnumbers tests' => sub { subtest 'GetItemsInfo tests' => sub { - plan tests => 9; + plan tests => 10; $schema->storage->txn_begin; @@ -370,6 +370,8 @@ subtest 'GetItemsInfo tests' => sub { '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..102c9ff2e8 100644 --- a/t/db_dependent/Koha/Account/Line.t +++ b/t/db_dependent/Koha/Account/Line.t @@ -360,7 +360,6 @@ subtest 'Renewal related tests' => sub { class => 'Koha::Checkouts', value => { itemnumber => $item->itemnumber, - onsite_checkout => 0, renewals => 99, auto_renew => 0 } @@ -406,7 +405,6 @@ subtest 'Renewal related tests' => sub { class => 'Koha::Checkouts', value => { itemnumber => $item->itemnumber, - onsite_checkout => 0, renewals => 0, auto_renew => 0 } -- 2.17.1