Bugzilla – Attachment 102658 Details for
Bug 25037
Add checkout_type to checkouts
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25037: Replace occurences to issues.onsite_checkout
Bug-25037-Replace-occurences-to-issuesonsitechecko.patch (text/plain), 18.58 KB, created by
Lari Taskula
on 2020-04-09 19:48:08 UTC
(
hide
)
Description:
Bug 25037: Replace occurences to issues.onsite_checkout
Filename:
MIME Type:
Creator:
Lari Taskula
Created:
2020-04-09 19:48:08 UTC
Size:
18.58 KB
patch
obsolete
>From d3cbe27ec99f43d1c6f26843b71471b83f746252 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 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 747d660ea6..cbeadc0a1e 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, > }; > >@@ -2713,7 +2715,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; >@@ -4096,7 +4098,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, >@@ -4117,7 +4119,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 94808084ac..7a182e08e1 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; >@@ -660,7 +661,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, >@@ -716,6 +717,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..878a215563 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 %]<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 == Checkouts.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 e18c2a24c9..bb35e6eb31 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 += " <span class='onsite_checkout'>(" + INHOUSE_USE + ")</span>"; > } > >@@ -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 += " <span class='onsite_checkout'>(" + INHOUSE_USE + ")</span>"; > } > >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..e250a81966 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.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.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 91da402058..c1c29c1c0c 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' %] > <title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle | html %][% ELSE %]Koha online[% END %] catalog › Your checkout history</title> > [% INCLUDE 'doc-head-close.inc' %] >@@ -90,7 +91,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 == Checkouts.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 d0c97ec0b0..946611222d 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -3134,7 +3134,6 @@ subtest 'ItemsDeniedRenewal preference' => sub { > auto_renew => 0, > borrowernumber => $idr_borrower->borrowernumber, > itemnumber => $deny_book->itemnumber, >- onsite_checkout => 0, > date_due => $future, > } > }); >@@ -3144,7 +3143,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 c07c66703e..fcf2d89904 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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=102658">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
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