Attachment 102264 Details for Bug 25037 – Bug 25037: Replace onsite_checkout with checkout_type

[patch] Bug 25037: Replace onsite_checkout with checkout_type

Bug-25037-Replace-onsitecheckout-with-checkouttype.patch (text/plain), 17.29 KB, created by Lari Taskula on 2020-04-02 05:33:54 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Lari Taskula

Created: 2020-04-02 05:33:54 UTC

Size: 17.29 KB

patch

obsolete

>From b24e3b028ba145530c810b09ecd944e531f20b06 Mon Sep 17 00:00:00 2001
>From: Lari Taskula <lari.taskula@hypernova.fi>
>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 ..."
>
>3. Verify koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc
> - Repeat step 2 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"
>
>4. Verify koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt
> - After step 3, 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
>
>5. Verify koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt
> - Repeat step 4 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.
> <td class="status">
> 
> [% 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 %]<tr>[% ELSE %]<tr class="onissue">[% END %]
> <td style="display:none;">
>- [% 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 %]
> <tr>
> <td style="display:none;">
>- [% 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

Actions: View | Diff | Splinter Review

Attachments on bug 25037: 102259 | 102260 | 102261 | 102262 | 102263 | 102264 | 102265 | 102266 | 102267 | 102268 | 102269 | 102272 | 102413 | 102414 | 102415 | 102416 | 102417 | 102418 | 102419 | 102420 | 102421 | 102422 | 102423 | 102424 | 102586 | 102587 | 102588 | 102589 | 102590 | 102591 | 102592 | 102593 | 102594 | 102595 | 102596 | 102597 | 102598 | 102599 | 102655 | 102656 | 102657 | 102658 | 102659 | 102660 | 102661 | 102662 | 102663 | 102664 | 102665 | 102666 | 102675 | 102676 | 104267 | 104268 | 105638 | 105780 | 105781 | 105782 | 105783 | 105784