Bugzilla – Attachment 194765 Details for
Bug 26355
Allow patrons to self-renew through the OPAC
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 26355: (QA follow-up) perl tidy to address qa tool errors
Bug-26355-QA-follow-up-perl-tidy-to-address-qa-too.patch (text/plain), 63.10 KB, created by
Andrew Fuerste-Henry
on 2026-03-06 16:03:26 UTC
(
hide
)
Description:
Bug 26355: (QA follow-up) perl tidy to address qa tool errors
Filename:
MIME Type:
Creator:
Andrew Fuerste-Henry
Created:
2026-03-06 16:03:26 UTC
Size:
63.10 KB
patch
obsolete
>From 3079438d6fa519a158ab3c6c51cb109860ae3e2a Mon Sep 17 00:00:00 2001 >From: Andrew Fuerste Henry <andrew@bywatersolutions.com> >Date: Fri, 6 Mar 2026 15:56:53 +0000 >Subject: [PATCH] Bug 26355: (QA follow-up) perl tidy to address qa tool errors > >Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> >--- > opac/opac-user.pl | 710 +++++++++---------- > t/db_dependent/api/v1/patron_categories.t | 23 +- > t/db_dependent/api/v1/patrons.t | 174 +++-- > t/db_dependent/api/v1/patrons_self_renewal.t | 12 +- > 4 files changed, 495 insertions(+), 424 deletions(-) > >diff --git a/opac/opac-user.pl b/opac/opac-user.pl >index e59321ef8cf..9bf01a3c1ad 100755 >--- a/opac/opac-user.pl >+++ b/opac/opac-user.pl >@@ -16,427 +16,427 @@ > # You should have received a copy of the GNU General Public License > # along with Koha; if not, see <https://www.gnu.org/licenses>. > >- use Modern::Perl; >+use Modern::Perl; > >- use CGI qw ( -utf8 ); >- use URI; >+use CGI qw ( -utf8 ); >+use URI; > >- use C4::Auth qw( get_template_and_user ); >- use C4::Koha qw( >- getitemtypeimagelocation >- GetNormalizedISBN >- GetNormalizedUPC >- GetNormalizedOCLCNumber >- ); >- use C4::Circulation qw( CanBookBeRenewed GetRenewCount GetIssuingCharges ); >- use C4::External::BakerTaylor qw( image_url link_url ); >- use C4::Reserves qw( GetReserveStatus ); >- use C4::Members; >- use C4::Output qw( output_html_with_http_headers ); >- use Koha::Account::Lines; >- use Koha::Biblios; >- use Koha::Libraries; >- use Koha::DateUtils qw( output_pref ); >- use Koha::Holds; >- use Koha::Database; >- use Koha::ItemTypes; >- use Koha::Patron::Attribute::Types; >- use Koha::Patrons; >- use Koha::Patron::Messages; >- use Koha::Patron::Discharge; >- use Koha::Patrons; >- use Koha::Ratings; >- use Koha::Recalls; >- >- use constant ATTRIBUTE_SHOW_BARCODE => 'SHOW_BCODE'; >- >- use Scalar::Util qw( looks_like_number ); >- use Date::Calc qw( Date_to_Days Today ); >- >- my $query = CGI->new; >- >- my $op = $query->param('op') || q{}; >+use C4::Auth qw( get_template_and_user ); >+use C4::Koha qw( >+ getitemtypeimagelocation >+ GetNormalizedISBN >+ GetNormalizedUPC >+ GetNormalizedOCLCNumber >+); >+use C4::Circulation qw( CanBookBeRenewed GetRenewCount GetIssuingCharges ); >+use C4::External::BakerTaylor qw( image_url link_url ); >+use C4::Reserves qw( GetReserveStatus ); >+use C4::Members; >+use C4::Output qw( output_html_with_http_headers ); >+use Koha::Account::Lines; >+use Koha::Biblios; >+use Koha::Libraries; >+use Koha::DateUtils qw( output_pref ); >+use Koha::Holds; >+use Koha::Database; >+use Koha::ItemTypes; >+use Koha::Patron::Attribute::Types; >+use Koha::Patrons; >+use Koha::Patron::Messages; >+use Koha::Patron::Discharge; >+use Koha::Patrons; >+use Koha::Ratings; >+use Koha::Recalls; >+ >+use constant ATTRIBUTE_SHOW_BARCODE => 'SHOW_BCODE'; >+ >+use Scalar::Util qw( looks_like_number ); >+use Date::Calc qw( Date_to_Days Today ); >+ >+my $query = CGI->new; >+ >+my $op = $query->param('op') || q{}; > > # CAS single logout handling > # Will print header and exit >- if ( C4::Context->preference('casAuthentication') ) { >- require C4::Auth_with_cas; >- C4::Auth_with_cas::logout_if_required($query); >- } >+if ( C4::Context->preference('casAuthentication') ) { >+ require C4::Auth_with_cas; >+ C4::Auth_with_cas::logout_if_required($query); >+} > >- my ( $template, $borrowernumber, $cookie ) = get_template_and_user( >- { >- template_name => "opac-user.tt", >- query => $query, >- type => "opac", >- } >- ); >+my ( $template, $borrowernumber, $cookie ) = get_template_and_user( >+ { >+ template_name => "opac-user.tt", >+ query => $query, >+ type => "opac", >+ } >+); > >- my %renewed = map { $_ => 1 } split( ':', $query->param('renewed') || '' ); >+my %renewed = map { $_ => 1 } split( ':', $query->param('renewed') || '' ); > >- my $show_priority; >- for ( C4::Context->preference("OPACShowHoldQueueDetails") ) { >- m/priority/ and $show_priority = 1; >- } >+my $show_priority; >+for ( C4::Context->preference("OPACShowHoldQueueDetails") ) { >+ m/priority/ and $show_priority = 1; >+} > >- my $patronupdate = $query->param('patronupdate'); >- my $canrenew = 1; >+my $patronupdate = $query->param('patronupdate'); >+my $canrenew = 1; > >- $template->param( shibbolethAuthentication => C4::Context->config('useshibboleth') ); >+$template->param( shibbolethAuthentication => C4::Context->config('useshibboleth') ); > > # get borrower information .... >- my $patron = Koha::Patrons->find($borrowernumber); >- >- if ( $op eq 'cud-update_arc' && C4::Context->preference("AllowPatronToControlAutorenewal") ) { >- my $autorenew_checkouts = $query->param('borrower_autorenew_checkouts'); >- $patron->autorenew_checkouts($autorenew_checkouts)->store() if defined $autorenew_checkouts; >- } >+my $patron = Koha::Patrons->find($borrowernumber); > >- my $borr = $patron->unblessed; >+if ( $op eq 'cud-update_arc' && C4::Context->preference("AllowPatronToControlAutorenewal") ) { >+ my $autorenew_checkouts = $query->param('borrower_autorenew_checkouts'); >+ $patron->autorenew_checkouts($autorenew_checkouts)->store() if defined $autorenew_checkouts; >+} > >- my ( $today_year, $today_month, $today_day ) = Today(); >- my ( $warning_year, $warning_month, $warning_day ) = split /-/, $borr->{'dateexpiry'}; >+my $borr = $patron->unblessed; > >- my $debar = Koha::Patrons->find($borrowernumber)->is_debarred; >- my $userdebarred; >+my ( $today_year, $today_month, $today_day ) = Today(); >+my ( $warning_year, $warning_month, $warning_day ) = split /-/, $borr->{'dateexpiry'}; > >- if ($debar) { >- $userdebarred = 1; >- $template->param( 'userdebarred' => $userdebarred ); >- if ( $debar ne "9999-12-31" ) { >- $borr->{'userdebarreddate'} = $debar; >- } >+my $debar = Koha::Patrons->find($borrowernumber)->is_debarred; >+my $userdebarred; > >- # FIXME looks like $available is not needed >- # If a user is discharged they have a validated discharge available >- my $available = Koha::Patron::Discharge::count( >- { >- borrowernumber => $borrowernumber, >- validated => 1, >- } >- ); >- $template->param( 'discharge_available' => $available >- && Koha::Patron::Discharge::is_discharged( { borrowernumber => $borrowernumber } ) ); >+if ($debar) { >+ $userdebarred = 1; >+ $template->param( 'userdebarred' => $userdebarred ); >+ if ( $debar ne "9999-12-31" ) { >+ $borr->{'userdebarreddate'} = $debar; > } > >- if ( $userdebarred || $borr->{'gonenoaddress'} || $borr->{'lost'} ) { >- $borr->{'flagged'} = 1; >- $canrenew = 0; >- } >+ # FIXME looks like $available is not needed >+ # If a user is discharged they have a validated discharge available >+ my $available = Koha::Patron::Discharge::count( >+ { >+ borrowernumber => $borrowernumber, >+ validated => 1, >+ } >+ ); >+ $template->param( 'discharge_available' => $available >+ && Koha::Patron::Discharge::is_discharged( { borrowernumber => $borrowernumber } ) ); >+} > >- my $amountoutstanding = $patron->account->balance; >- my $no_renewal_amt = C4::Context->preference('OPACFineNoRenewals'); >- $no_renewal_amt = undef unless looks_like_number($no_renewal_amt); >- my $amountoutstandingfornewal = >- C4::Context->preference("OPACFineNoRenewalsIncludeCredits") >- ? $amountoutstanding >- : $patron->account->outstanding_debits->total_outstanding; >- >- if ( C4::Context->preference('OpacRenewalAllowed') >- && defined($no_renewal_amt) >- && $amountoutstandingfornewal > $no_renewal_amt ) >- { >- $borr->{'flagged'} = 1; >- $canrenew = 0; >- $template->param( >- renewal_blocked_fines => $no_renewal_amt, >- renewal_blocked_fines_amountoutstanding => $amountoutstandingfornewal, >- ); >- } >+if ( $userdebarred || $borr->{'gonenoaddress'} || $borr->{'lost'} ) { >+ $borr->{'flagged'} = 1; >+ $canrenew = 0; >+} > >- my $maxoutstanding = C4::Context->preference('maxoutstanding'); >- if ( $amountoutstanding && ( $amountoutstanding > $maxoutstanding ) ) { >- $borr->{blockedonfines} = 1; >- } >+my $amountoutstanding = $patron->account->balance; >+my $no_renewal_amt = C4::Context->preference('OPACFineNoRenewals'); >+$no_renewal_amt = undef unless looks_like_number($no_renewal_amt); >+my $amountoutstandingfornewal = >+ C4::Context->preference("OPACFineNoRenewalsIncludeCredits") >+ ? $amountoutstanding >+ : $patron->account->outstanding_debits->total_outstanding; >+ >+if ( C4::Context->preference('OpacRenewalAllowed') >+ && defined($no_renewal_amt) >+ && $amountoutstandingfornewal > $no_renewal_amt ) >+{ >+ $borr->{'flagged'} = 1; >+ $canrenew = 0; >+ $template->param( >+ renewal_blocked_fines => $no_renewal_amt, >+ renewal_blocked_fines_amountoutstanding => $amountoutstandingfornewal, >+ ); >+} >+ >+my $maxoutstanding = C4::Context->preference('maxoutstanding'); >+if ( $amountoutstanding && ( $amountoutstanding > $maxoutstanding ) ) { >+ $borr->{blockedonfines} = 1; >+} > > # Warningdate is the date that the warning starts appearing >- if ( $borr->{'dateexpiry'} && C4::Context->preference('NotifyBorrowerDeparture') ) { >- my $days_to_expiry = Date_to_Days( $warning_year, $warning_month, $warning_day ) - >- Date_to_Days( $today_year, $today_month, $today_day ); >- if ( $days_to_expiry < 0 ) { >- >- #borrower card has expired, warn the borrower >- $borr->{'warnexpired'} = $borr->{'dateexpiry'}; >- } elsif ( $days_to_expiry < C4::Context->preference('NotifyBorrowerDeparture') ) { >- >- # borrower card soon to expire, warn the borrower >- $borr->{'warndeparture'} = $borr->{dateexpiry}; >- if ( C4::Context->preference('ReturnBeforeExpiry') ) { >- $borr->{'returnbeforeexpiry'} = 1; >- } >+if ( $borr->{'dateexpiry'} && C4::Context->preference('NotifyBorrowerDeparture') ) { >+ my $days_to_expiry = Date_to_Days( $warning_year, $warning_month, $warning_day ) - >+ Date_to_Days( $today_year, $today_month, $today_day ); >+ if ( $days_to_expiry < 0 ) { >+ >+ #borrower card has expired, warn the borrower >+ $borr->{'warnexpired'} = $borr->{'dateexpiry'}; >+ } elsif ( $days_to_expiry < C4::Context->preference('NotifyBorrowerDeparture') ) { >+ >+ # borrower card soon to expire, warn the borrower >+ $borr->{'warndeparture'} = $borr->{dateexpiry}; >+ if ( C4::Context->preference('ReturnBeforeExpiry') ) { >+ $borr->{'returnbeforeexpiry'} = 1; > } > } >+} > >- my $saving_display = C4::Context->preference('OPACShowSavings'); >- if ( $saving_display =~ /user/ ) { >- $template->param( savings => $patron->get_savings ); >- } >+my $saving_display = C4::Context->preference('OPACShowSavings'); >+if ( $saving_display =~ /user/ ) { >+ $template->param( savings => $patron->get_savings ); >+} > > # pass on any renew errors to the template for displaying >- my $renew_error = $query->param('renew_error'); >- >- $template->param( >- amountoutstanding => $amountoutstanding, >- borrowernumber => $borrowernumber, >- patron_flagged => $borr->{flagged}, >- surname => $borr->{surname}, >- RENEW_ERROR => $renew_error, >- borrower => $borr, >- ); >+my $renew_error = $query->param('renew_error'); >+ >+$template->param( >+ amountoutstanding => $amountoutstanding, >+ borrowernumber => $borrowernumber, >+ patron_flagged => $borr->{flagged}, >+ surname => $borr->{surname}, >+ RENEW_ERROR => $renew_error, >+ borrower => $borr, >+); > > #get issued items .... > >- my $count = 0; >- my $overdues_count = 0; >- my @overdues; >- my @issuedat; >- my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search_with_localization->unblessed } }; >- my $pending_checkouts = $patron->pending_checkouts->search( >- {}, >- { >- order_by => [ { -desc => 'date_due' }, { -asc => 'issue_id' } ], >- prefetch => 'item' >+my $count = 0; >+my $overdues_count = 0; >+my @overdues; >+my @issuedat; >+my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search_with_localization->unblessed } }; >+my $pending_checkouts = $patron->pending_checkouts->search( >+ {}, >+ { >+ order_by => [ { -desc => 'date_due' }, { -asc => 'issue_id' } ], >+ prefetch => 'item' >+ } >+); >+my $are_renewable_items = 0; >+if ( $pending_checkouts->count ) { # Useless test >+ while ( my $c = $pending_checkouts->next ) { >+ my $issue = $c->unblessed_all_relateds; >+ >+ # check for reserves >+ my $restype = GetReserveStatus( $issue->{'itemnumber'} ); >+ if ($restype) { >+ $issue->{'reserved'} = 1; > } >- ); >- my $are_renewable_items = 0; >- if ( $pending_checkouts->count ) { # Useless test >- while ( my $c = $pending_checkouts->next ) { >- my $issue = $c->unblessed_all_relateds; >- >- # check for reserves >- my $restype = GetReserveStatus( $issue->{'itemnumber'} ); >- if ($restype) { >- $issue->{'reserved'} = 1; >- } > >- # Must be moved in a module if reused >- my $charges = Koha::Account::Lines->search( >- { >- borrowernumber => $patron->borrowernumber, >- amountoutstanding => { '>' => 0 }, >- debit_type_code => [ 'OVERDUE', 'LOST' ], >- itemnumber => $issue->{itemnumber} >- }, >- ); >- $issue->{charges} = $charges->total_outstanding; >+ # Must be moved in a module if reused >+ my $charges = Koha::Account::Lines->search( >+ { >+ borrowernumber => $patron->borrowernumber, >+ amountoutstanding => { '>' => 0 }, >+ debit_type_code => [ 'OVERDUE', 'LOST' ], >+ itemnumber => $issue->{itemnumber} >+ }, >+ ); >+ $issue->{charges} = $charges->total_outstanding; > >- my $rental_fines = Koha::Account::Lines->search( >- { >- borrowernumber => $patron->borrowernumber, >- amountoutstanding => { '>' => 0 }, >- debit_type_code => { 'LIKE' => 'RENT_%' }, >- itemnumber => $issue->{itemnumber} >- } >- ); >- $issue->{rentalfines} = $rental_fines->total_outstanding; >- >- # check if item is renewable >- my ( $status, $renewerror, $info ) = CanBookBeRenewed( $patron, $c ); >- ( >- $issue->{'renewcount'}, >- $issue->{'renewsallowed'}, >- $issue->{'renewsleft'}, >- $issue->{'unseencount'}, >- $issue->{'unseenallowed'}, >- $issue->{'unseenleft'} >- ) = GetRenewCount( $patron, $c->item ); >- ( $issue->{'renewalfee'}, $issue->{'renewalitemtype'} ) = >- GetIssuingCharges( $issue->{'itemnumber'}, $borrowernumber ); >- $issue->{itemtype_object} = Koha::ItemTypes->find( $c->item->effective_itemtype ); >- if ( $status && C4::Context->preference("OpacRenewalAllowed") ) { >- $are_renewable_items = 1; >- $issue->{'status'} = $status; >+ my $rental_fines = Koha::Account::Lines->search( >+ { >+ borrowernumber => $patron->borrowernumber, >+ amountoutstanding => { '>' => 0 }, >+ debit_type_code => { 'LIKE' => 'RENT_%' }, >+ itemnumber => $issue->{itemnumber} > } >+ ); >+ $issue->{rentalfines} = $rental_fines->total_outstanding; >+ >+ # check if item is renewable >+ my ( $status, $renewerror, $info ) = CanBookBeRenewed( $patron, $c ); >+ ( >+ $issue->{'renewcount'}, >+ $issue->{'renewsallowed'}, >+ $issue->{'renewsleft'}, >+ $issue->{'unseencount'}, >+ $issue->{'unseenallowed'}, >+ $issue->{'unseenleft'} >+ ) = GetRenewCount( $patron, $c->item ); >+ ( $issue->{'renewalfee'}, $issue->{'renewalitemtype'} ) = >+ GetIssuingCharges( $issue->{'itemnumber'}, $borrowernumber ); >+ $issue->{itemtype_object} = Koha::ItemTypes->find( $c->item->effective_itemtype ); >+ if ( $status && C4::Context->preference("OpacRenewalAllowed") ) { >+ $are_renewable_items = 1; >+ $issue->{'status'} = $status; >+ } > >- $issue->{'renewed'} = $renewed{ $issue->{'itemnumber'} }; >- >- if ($renewerror) { >- $issue->{'too_many'} = 1 if $renewerror eq 'too_many'; >- $issue->{'too_unseen'} = 1 if $renewerror eq 'too_unseen'; >- $issue->{'on_reserve'} = 1 if $renewerror eq 'on_reserve'; >- $issue->{'norenew_overdue'} = 1 if $renewerror eq 'overdue'; >- $issue->{'auto_renew'} = 1 if $renewerror eq 'auto_renew'; >- $issue->{'auto_too_soon'} = 1 if $renewerror eq 'auto_too_soon'; >- $issue->{'auto_too_late'} = 1 if $renewerror eq 'auto_too_late'; >- $issue->{'auto_too_much_oweing'} = 1 if $renewerror eq 'auto_too_much_oweing'; >- $issue->{'item_denied_renewal'} = 1 if $renewerror eq 'item_denied_renewal'; >- $issue->{'item_issued_to_other_patron'} = 1 if $renewerror eq 'item_issued_to_other_patron'; >- >- if ( $renewerror eq 'too_soon' ) { >- $issue->{'too_soon'} = 1; >- $issue->{'soonestrenewdate'} = $info->{soonest_renew_date}; >- } >+ $issue->{'renewed'} = $renewed{ $issue->{'itemnumber'} }; >+ >+ if ($renewerror) { >+ $issue->{'too_many'} = 1 if $renewerror eq 'too_many'; >+ $issue->{'too_unseen'} = 1 if $renewerror eq 'too_unseen'; >+ $issue->{'on_reserve'} = 1 if $renewerror eq 'on_reserve'; >+ $issue->{'norenew_overdue'} = 1 if $renewerror eq 'overdue'; >+ $issue->{'auto_renew'} = 1 if $renewerror eq 'auto_renew'; >+ $issue->{'auto_too_soon'} = 1 if $renewerror eq 'auto_too_soon'; >+ $issue->{'auto_too_late'} = 1 if $renewerror eq 'auto_too_late'; >+ $issue->{'auto_too_much_oweing'} = 1 if $renewerror eq 'auto_too_much_oweing'; >+ $issue->{'item_denied_renewal'} = 1 if $renewerror eq 'item_denied_renewal'; >+ $issue->{'item_issued_to_other_patron'} = 1 if $renewerror eq 'item_issued_to_other_patron'; >+ >+ if ( $renewerror eq 'too_soon' ) { >+ $issue->{'too_soon'} = 1; >+ $issue->{'soonestrenewdate'} = $info->{soonest_renew_date}; > } >+ } > >- if ( $c->is_overdue ) { >- push @overdues, $issue; >- $overdues_count++; >- $issue->{'overdue'} = 1; >- } else { >- $issue->{'issued'} = 1; >- } >+ if ( $c->is_overdue ) { >+ push @overdues, $issue; >+ $overdues_count++; >+ $issue->{'overdue'} = 1; >+ } else { >+ $issue->{'issued'} = 1; >+ } > >- # imageurl: >- my $itemtype = $issue->{'itemtype'}; >- if ($itemtype) { >- $issue->{'imageurl'} = getitemtypeimagelocation( 'opac', $itemtypes->{$itemtype}->{'imageurl'} ); >- $issue->{'description'} = $itemtypes->{$itemtype}->{'description'}; >- } >+ # imageurl: >+ my $itemtype = $issue->{'itemtype'}; >+ if ($itemtype) { >+ $issue->{'imageurl'} = getitemtypeimagelocation( 'opac', $itemtypes->{$itemtype}->{'imageurl'} ); >+ $issue->{'description'} = $itemtypes->{$itemtype}->{'description'}; >+ } > >- if ( C4::Context->preference('OpacStarRatings') eq 'all' ) { >- my $ratings = Koha::Ratings->search( { biblionumber => $issue->{biblionumber} } ); >- $issue->{ratings} = $ratings; >- $issue->{my_rating} = >- $borrowernumber ? $ratings->search( { borrowernumber => $borrowernumber } )->next : undef; >- } >+ if ( C4::Context->preference('OpacStarRatings') eq 'all' ) { >+ my $ratings = Koha::Ratings->search( { biblionumber => $issue->{biblionumber} } ); >+ $issue->{ratings} = $ratings; >+ $issue->{my_rating} = >+ $borrowernumber ? $ratings->search( { borrowernumber => $borrowernumber } )->next : undef; >+ } > >- my $biblio_object = Koha::Biblios->find( $issue->{biblionumber} ); >- $issue->{biblio_object} = $biblio_object; >- push @issuedat, $issue; >- $count++; >+ my $biblio_object = Koha::Biblios->find( $issue->{biblionumber} ); >+ $issue->{biblio_object} = $biblio_object; >+ push @issuedat, $issue; >+ $count++; > >- my $isbn = GetNormalizedISBN( $issue->{'isbn'} ); >- $issue->{normalized_isbn} = $isbn; >+ my $isbn = GetNormalizedISBN( $issue->{'isbn'} ); >+ $issue->{normalized_isbn} = $isbn; > >- if ( C4::Context->preference('BakerTaylorEnabled') >- || C4::Context->preference('SyndeticsEnabled') >- || C4::Context->preference('SyndeticsCoverImages') ) >- { >- my $marcrecord = $biblio_object->metadata_record( >- { >- embed_items => 1, >- interface => 'opac', >- patron => $patron >- } >- ); >- $issue->{normalized_upc} = GetNormalizedUPC( $marcrecord, C4::Context->preference('marcflavour') ); >- $issue->{normalized_oclc} = GetNormalizedOCLCNumber( $marcrecord, C4::Context->preference('marcflavour') ); >- } >+ if ( C4::Context->preference('BakerTaylorEnabled') >+ || C4::Context->preference('SyndeticsEnabled') >+ || C4::Context->preference('SyndeticsCoverImages') ) >+ { >+ my $marcrecord = $biblio_object->metadata_record( >+ { >+ embed_items => 1, >+ interface => 'opac', >+ patron => $patron >+ } >+ ); >+ $issue->{normalized_upc} = GetNormalizedUPC( $marcrecord, C4::Context->preference('marcflavour') ); >+ $issue->{normalized_oclc} = GetNormalizedOCLCNumber( $marcrecord, C4::Context->preference('marcflavour') ); >+ } > >- if ( C4::Context->preference('UseRecalls') ) { >- my $maybe_recalls = Koha::Recalls->search( >- { biblio_id => $issue->{biblionumber}, item_id => [ undef, $issue->{itemnumber} ], completed => 0 } ); >- while ( my $recall = $maybe_recalls->next ) { >- if ( $recall->checkout and $recall->checkout->issue_id == $issue->{issue_id} ) { >- $issue->{recall} = 1; >- last; >- } >+ if ( C4::Context->preference('UseRecalls') ) { >+ my $maybe_recalls = Koha::Recalls->search( >+ { biblio_id => $issue->{biblionumber}, item_id => [ undef, $issue->{itemnumber} ], completed => 0 } ); >+ while ( my $recall = $maybe_recalls->next ) { >+ if ( $recall->checkout and $recall->checkout->issue_id == $issue->{issue_id} ) { >+ $issue->{recall} = 1; >+ last; > } > } > } > } >- my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing'); >- $canrenew = 0 if ( $overduesblockrenewing ne 'allow' and $overdues_count == $count ) || !$are_renewable_items; >- >- $template->param( ISSUES => \@issuedat ); >- $template->param( issues_count => $count ); >- $template->param( canrenew => $canrenew ); >- $template->param( OVERDUES => \@overdues ); >- $template->param( overdues_count => $overdues_count ); >- >- my $show_barcode = Koha::Patron::Attribute::Types->search( # FIXME we should not need this search >- { code => ATTRIBUTE_SHOW_BARCODE } >- )->count; >- if ($show_barcode) { >- my $patron_show_barcode = $patron->get_extended_attribute(ATTRIBUTE_SHOW_BARCODE); >- undef $show_barcode if $patron_show_barcode and not $patron_show_barcode->attribute; >- } >- $template->param( show_barcode => 1 ) if $show_barcode; >+} >+my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing'); >+$canrenew = 0 if ( $overduesblockrenewing ne 'allow' and $overdues_count == $count ) || !$are_renewable_items; >+ >+$template->param( ISSUES => \@issuedat ); >+$template->param( issues_count => $count ); >+$template->param( canrenew => $canrenew ); >+$template->param( OVERDUES => \@overdues ); >+$template->param( overdues_count => $overdues_count ); >+ >+my $show_barcode = Koha::Patron::Attribute::Types->search( # FIXME we should not need this search >+ { code => ATTRIBUTE_SHOW_BARCODE } >+)->count; >+if ($show_barcode) { >+ my $patron_show_barcode = $patron->get_extended_attribute(ATTRIBUTE_SHOW_BARCODE); >+ undef $show_barcode if $patron_show_barcode and not $patron_show_barcode->attribute; >+} >+$template->param( show_barcode => 1 ) if $show_barcode; > > # now the reserved items.... >- my $reserves = $patron->holds->filter_out_has_cancellation_requests; >- >- $template->param( >- RESERVES => $reserves, >- reserves_count => $reserves->count_holds, >- showpriority => $show_priority, >- ); >- >- if ( C4::Context->preference('UseRecalls') ) { >- my $recalls = Koha::Recalls->search( { patron_id => $borrowernumber, completed => 0 } ); >- $template->param( RECALLS => $recalls ); >- } >+my $reserves = $patron->holds->filter_out_has_cancellation_requests; > >- if ( C4::Context->preference('BakerTaylorEnabled') ) { >- $template->param( >- BakerTaylorEnabled => 1, >- BakerTaylorImageURL => &image_url(), >- BakerTaylorLinkURL => &link_url(), >- BakerTaylorBookstoreURL => C4::Context->preference('BakerTaylorBookstoreURL'), >- ); >- } >+$template->param( >+ RESERVES => $reserves, >+ reserves_count => $reserves->count_holds, >+ showpriority => $show_priority, >+); > >- if ( C4::Context->preference("OPACAmazonCoverImages") >- or C4::Context->preference("GoogleJackets") >- or C4::Context->preference("BakerTaylorEnabled") >- or C4::Context->preference("SyndeticsCoverImages") >- or ( C4::Context->preference('OPACCustomCoverImages') and C4::Context->preference('CustomCoverImagesURL') ) ) >- { >- $template->param( JacketImages => 1 ); >- } >+if ( C4::Context->preference('UseRecalls') ) { >+ my $recalls = Koha::Recalls->search( { patron_id => $borrowernumber, completed => 0 } ); >+ $template->param( RECALLS => $recalls ); >+} > >+if ( C4::Context->preference('BakerTaylorEnabled') ) { > $template->param( >- overdrive_error => scalar $query->param('overdrive_error') || undef, >- overdrive_tab => scalar $query->param('overdrive_tab') || 0, >+ BakerTaylorEnabled => 1, >+ BakerTaylorImageURL => &image_url(), >+ BakerTaylorLinkURL => &link_url(), >+ BakerTaylorBookstoreURL => C4::Context->preference('BakerTaylorBookstoreURL'), > ); >+} > >- my $patron_messages = Koha::Patron::Messages->search( >- { >- borrowernumber => $borrowernumber, >- message_type => 'B', >- } >- ); >+if ( C4::Context->preference("OPACAmazonCoverImages") >+ or C4::Context->preference("GoogleJackets") >+ or C4::Context->preference("BakerTaylorEnabled") >+ or C4::Context->preference("SyndeticsCoverImages") >+ or ( C4::Context->preference('OPACCustomCoverImages') and C4::Context->preference('CustomCoverImagesURL') ) ) >+{ >+ $template->param( JacketImages => 1 ); >+} > >- if ( C4::Context->preference('AllowPatronToSetCheckoutsVisibilityForGuarantor') >- || C4::Context->preference('AllowStaffToSetCheckoutsVisibilityForGuarantor') ) >- { >- my @relatives; >+$template->param( >+ overdrive_error => scalar $query->param('overdrive_error') || undef, >+ overdrive_tab => scalar $query->param('overdrive_tab') || 0, >+); > >- # Filter out guarantees that don't want guarantor to see checkouts >- foreach my $gr ( $patron->guarantee_relationships->as_list ) { >- my $g = $gr->guarantee; >- push( @relatives, $g ) if $g->privacy_guarantor_checkouts; >- } >- $template->param( relatives => \@relatives ); >+my $patron_messages = Koha::Patron::Messages->search( >+ { >+ borrowernumber => $borrowernumber, >+ message_type => 'B', > } >+); > >- if ( C4::Context->preference('AllowPatronToSetFinesVisibilityForGuarantor') >- || C4::Context->preference('AllowStaffToSetFinesVisibilityForGuarantor') ) >- { >- my @relatives_with_fines; >+if ( C4::Context->preference('AllowPatronToSetCheckoutsVisibilityForGuarantor') >+ || C4::Context->preference('AllowStaffToSetCheckoutsVisibilityForGuarantor') ) >+{ >+ my @relatives; > >- # Filter out guarantees that don't want guarantor to see checkouts >- foreach my $gr ( $patron->guarantee_relationships->as_list ) { >- my $g = $gr->guarantee; >- push( @relatives_with_fines, $g ) if $g->privacy_guarantor_fines; >- } >- $template->param( relatives_with_fines => \@relatives_with_fines ); >+ # Filter out guarantees that don't want guarantor to see checkouts >+ foreach my $gr ( $patron->guarantee_relationships->as_list ) { >+ my $g = $gr->guarantee; >+ push( @relatives, $g ) if $g->privacy_guarantor_checkouts; > } >+ $template->param( relatives => \@relatives ); >+} > >- if ( C4::Context->preference("ArticleRequests") ) { >- $template->param( >- current_article_requests => [ $patron->article_requests->filter_by_current->as_list ], >- ); >- } >+if ( C4::Context->preference('AllowPatronToSetFinesVisibilityForGuarantor') >+ || C4::Context->preference('AllowStaffToSetFinesVisibilityForGuarantor') ) >+{ >+ my @relatives_with_fines; > >- my $self_renewal_available = $patron->is_eligible_for_self_renewal(); >- my $self_renewal_alert; >- my $self_renewal_success = $query->param('self_renewal_success'); >- $self_renewal_alert = $patron->category->self_renewal_failure_message >- if defined $self_renewal_success && !$self_renewal_success; >- my $self_renewal_confirmation_sent = $query->param('confirmation_sent'); >+ # Filter out guarantees that don't want guarantor to see checkouts >+ foreach my $gr ( $patron->guarantee_relationships->as_list ) { >+ my $g = $gr->guarantee; >+ push( @relatives_with_fines, $g ) if $g->privacy_guarantor_fines; >+ } >+ $template->param( relatives_with_fines => \@relatives_with_fines ); >+} > >+if ( C4::Context->preference("ArticleRequests") ) { > $template->param( >- patron_messages => $patron_messages, >- opacnote => $borr->{opacnote}, >- patronupdate => $patronupdate, >- OpacRenewalAllowed => C4::Context->preference("OpacRenewalAllowed"), >- userview => 1, >- tab => scalar $query->param('tab') || q{}, >- SuspendHoldsOpac => C4::Context->preference('SuspendHoldsOpac'), >- AutoResumeSuspendedHolds => C4::Context->preference('AutoResumeSuspendedHolds'), >- OpacHoldNotes => C4::Context->preference('OpacHoldNotes'), >- failed_holds => scalar $query->param('failed_holds'), >+ current_article_requests => [ $patron->article_requests->filter_by_current->as_list ], >+ ); >+} >+ >+my $self_renewal_available = $patron->is_eligible_for_self_renewal(); >+my $self_renewal_alert; >+my $self_renewal_success = $query->param('self_renewal_success'); >+$self_renewal_alert = $patron->category->self_renewal_failure_message >+ if defined $self_renewal_success && !$self_renewal_success; >+my $self_renewal_confirmation_sent = $query->param('confirmation_sent'); >+ >+$template->param( >+ patron_messages => $patron_messages, >+ opacnote => $borr->{opacnote}, >+ patronupdate => $patronupdate, >+ OpacRenewalAllowed => C4::Context->preference("OpacRenewalAllowed"), >+ userview => 1, >+ tab => scalar $query->param('tab') || q{}, >+ SuspendHoldsOpac => C4::Context->preference('SuspendHoldsOpac'), >+ AutoResumeSuspendedHolds => C4::Context->preference('AutoResumeSuspendedHolds'), >+ OpacHoldNotes => C4::Context->preference('OpacHoldNotes'), >+ failed_holds => scalar $query->param('failed_holds'), > self_renewal_available => $self_renewal_available, > self_renewal_success => $self_renewal_success, > self_renewal_alert => $self_renewal_alert, >diff --git a/t/db_dependent/api/v1/patron_categories.t b/t/db_dependent/api/v1/patron_categories.t >index 4b8a7607852..0ce2a6e8d02 100755 >--- a/t/db_dependent/api/v1/patron_categories.t >+++ b/t/db_dependent/api/v1/patron_categories.t >@@ -66,8 +66,11 @@ subtest 'list() tests' => sub { > > $t->get_ok("//$userid:$password@/api/v1/patron_categories")->status_is(200); > >- $t->get_ok("//$userid:$password@/api/v1/patron_categories?q={\"me.categorycode\":\"TEST\"}")->status_is(200) >- ->json_has('/0/name')->json_is( '/0/name' => 'Test' )->json_hasnt('/1'); >+ $t->get_ok("//$userid:$password@/api/v1/patron_categories?q={\"me.categorycode\":\"TEST\"}") >+ ->status_is(200) >+ ->json_has('/0/name') >+ ->json_is( '/0/name' => 'Test' ) >+ ->json_hasnt('/1'); > > # Off limits search > >@@ -92,14 +95,18 @@ subtest 'list() tests' => sub { > my $off_limits_userid = $off_limits_librarian->userid; > > $t->get_ok("//$off_limits_userid:$off_limits_password@/api/v1/patron_categories?q={\"me.categorycode\":\"TEST\"}") >- ->status_is(200)->json_hasnt('/0'); >+ ->status_is(200) >+ ->json_hasnt('/0'); > > # Off limits librarian category has changed to one within limits > > $off_limits_librarian->branchcode( $library->branchcode )->store; > > $t->get_ok("//$off_limits_userid:$off_limits_password@/api/v1/patron_categories?q={\"me.categorycode\":\"TEST\"}") >- ->status_is(200)->json_has('/0/name')->json_is( [ $category->to_api ] )->json_hasnt('/1'); >+ ->status_is(200) >+ ->json_has('/0/name') >+ ->json_is( [ $category->to_api ] ) >+ ->json_hasnt('/1'); > > $schema->storage->txn_rollback; > >@@ -149,7 +156,8 @@ subtest 'add() tests' => sub { > }; > > $t->post_ok( "//$userid:$password@/api/v1/patron_categories" => json => $category_with_invalid_field ) >- ->status_is(400)->json_is( >+ ->status_is(400) >+ ->json_is( > "/errors" => [ > { > message => "Properties not allowed: blah.", >@@ -161,10 +169,11 @@ subtest 'add() tests' => sub { > # Authorized attempt to write > my $patron_category_id = > $t->post_ok( "//$userid:$password@/api/v1/patron_categories" => json => $category ) >- ->status_is( 201, 'REST3.2.1' )->header_like( >+ ->status_is( 201, 'REST3.2.1' ) >+ ->header_like( > Location => qr|^\/api\/v1\/patron_categories/\d*|, > 'REST3.4.1' >- )->json_is( '/name' => $category->{name} )->tx->res->json->{patron_category_id}; >+ )->json_is( '/name' => $category->{name} )->tx->res->json->{patron_category_id}; > > $schema->storage->txn_rollback; > }; >diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t >index 516faa66dc8..68806ea519e 100755 >--- a/t/db_dependent/api/v1/patrons.t >+++ b/t/db_dependent/api/v1/patrons.t >@@ -125,13 +125,16 @@ subtest 'list() tests' => sub { > > $t->get_ok("//$userid:$password@/api/v1/patrons")->status_is( 200, 'list_borrowers makes /patrons accessible' ); > >- $t->get_ok( "//$userid:$password@/api/v1/patrons?cardnumber=" . $librarian->cardnumber )->status_is(200) >+ $t->get_ok( "//$userid:$password@/api/v1/patrons?cardnumber=" . $librarian->cardnumber ) >+ ->status_is(200) > ->json_is( '/0/cardnumber' => $librarian->cardnumber ); > > $t->get_ok( "//$userid:$password@/api/v1/patrons?q={\"cardnumber\":\"" . $librarian->cardnumber . "\"}" ) >- ->status_is(200)->json_is( '/0/cardnumber' => $librarian->cardnumber ); >+ ->status_is(200) >+ ->json_is( '/0/cardnumber' => $librarian->cardnumber ); > >- $t->get_ok( "//$userid:$password@/api/v1/patrons?address2=" . $librarian->address2 )->status_is(200) >+ $t->get_ok( "//$userid:$password@/api/v1/patrons?address2=" . $librarian->address2 ) >+ ->status_is(200) > ->json_is( '/0/address2' => $librarian->address2 ); > > subtest 'restricted & expired' => sub { >@@ -145,13 +148,19 @@ subtest 'list() tests' => sub { > $t->get_ok( "//$userid:$password@/api/v1/patrons?restricted=" > . Mojo::JSON->true > . "&cardnumber=" >- . $patron->cardnumber )->status_is(200)->json_has('/0/restricted') >- ->json_is( '/0/restricted' => Mojo::JSON->true )->json_has('/0/expired') >- ->json_is( '/0/expired' => Mojo::JSON->false )->json_hasnt('/1'); >+ . $patron->cardnumber ) >+ ->status_is(200) >+ ->json_has('/0/restricted') >+ ->json_is( '/0/restricted' => Mojo::JSON->true ) >+ ->json_has('/0/expired') >+ ->json_is( '/0/expired' => Mojo::JSON->false ) >+ ->json_hasnt('/1'); > > $patron->dateexpiry( dt_from_string->subtract( days => 2 ) )->store; >- $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron->borrowernumber )->status_is(200) >- ->json_has('/expired')->json_is( '/expired' => Mojo::JSON->true ); >+ $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron->borrowernumber ) >+ ->status_is(200) >+ ->json_has('/expired') >+ ->json_is( '/expired' => Mojo::JSON->true ); > > $t->get_ok( "//$userid:$password@/api/v1/patrons?" > . 'q={"extended_attributes.type":"CODE"}' => { 'x-koha-embed' => 'extended_attributes' } ) >@@ -180,13 +189,15 @@ subtest 'list() tests' => sub { > $t->get_ok( "//$userid:$password@/api/v1/patrons?date_of_birth=" > . $date_of_birth > . "&cardnumber=" >- . $patron->cardnumber )->status_is(200) >+ . $patron->cardnumber ) >+ ->status_is(200) > ->json_is( '/0/patron_id' => $patron->id, 'Filtering by date works' ); > > $t->get_ok( "//$userid:$password@/api/v1/patrons?last_seen=" > . $last_seen_rfc3339 > . "&cardnumber=" >- . $patron->cardnumber )->status_is(200) >+ . $patron->cardnumber ) >+ ->status_is(200) > ->json_is( '/0/patron_id' => $patron->id, 'Filtering by date-time works' ); > > my $q = encode_json( >@@ -196,7 +207,8 @@ subtest 'list() tests' => sub { > } > ); > >- $t->get_ok("//$userid:$password@/api/v1/patrons?q=$q")->status_is(200) >+ $t->get_ok("//$userid:$password@/api/v1/patrons?q=$q") >+ ->status_is(200) > ->json_is( '/0/patron_id' => $patron->id, 'Filtering by date works' ); > > $q = encode_json( >@@ -206,7 +218,8 @@ subtest 'list() tests' => sub { > } > ); > >- $t->get_ok("//$userid:$password@/api/v1/patrons?q=$q")->status_is(200) >+ $t->get_ok("//$userid:$password@/api/v1/patrons?q=$q") >+ ->status_is(200) > ->json_is( '/0/patron_id' => $patron->id, 'Filtering by date-time works' ); > }; > >@@ -250,16 +263,20 @@ subtest 'list() tests' => sub { > my $userid = $librarian->userid; > > $t->get_ok( "//$userid:$password@/api/v1/patrons?_order_by=patron_id&q=" >- . encode_json( { library_id => [ $library_1->id, $library_2->id ] } ) )->status_is(200) >- ->json_is( '/0/patron_id' => $patron_1->id )->json_is( '/1/patron_id' => $patron_2->id ) >+ . encode_json( { library_id => [ $library_1->id, $library_2->id ] } ) ) >+ ->status_is(200) >+ ->json_is( '/0/patron_id' => $patron_1->id ) >+ ->json_is( '/1/patron_id' => $patron_2->id ) > ->json_is( '/2/patron_id' => $patron_3->id ); > > @libraries_where_can_see_patrons = ( $library_2->id ); > > my $res = > $t->get_ok( "//$userid:$password@/api/v1/patrons?_order_by=patron_id&q=" >- . encode_json( { library_id => [ $library_1->id, $library_2->id ] } ) )->status_is(200) >- ->json_is( '/0/patron_id' => $patron_3->id, 'Returns the only allowed patron' )->tx->res->json; >+ . encode_json( { library_id => [ $library_1->id, $library_2->id ] } ) ) >+ ->status_is(200) >+ ->json_is( '/0/patron_id' => $patron_3->id, 'Returns the only allowed patron' ) >+ ->tx->res->json; > > is( scalar @{$res}, 1, 'Only one patron returned' ); > >@@ -322,8 +339,10 @@ subtest 'get() tests' => sub { > > $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron->id ) > ->status_is( 200, 'list_borrowers permission makes patron visible' ) >- ->json_is( '/patron_id' => $patron->id )->json_is( '/category_id' => $patron->categorycode ) >- ->json_is( '/surname' => $patron->surname )->json_is( '/patron_card_lost' => Mojo::JSON->false ); >+ ->json_is( '/patron_id' => $patron->id ) >+ ->json_is( '/category_id' => $patron->categorycode ) >+ ->json_is( '/surname' => $patron->surname ) >+ ->json_is( '/patron_card_lost' => Mojo::JSON->false ); > > $schema->storage->txn_rollback; > }; >@@ -365,18 +384,22 @@ subtest 'get() tests' => sub { > $librarian->set_password( { password => $password, skip_validation => 1 } ); > my $userid = $librarian->userid; > >- $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron_1->id )->status_is(200) >+ $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron_1->id ) >+ ->status_is(200) > ->json_is( '/patron_id' => $patron_1->id ); > >- $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->id )->status_is(200) >+ $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->id ) >+ ->status_is(200) > ->json_is( '/patron_id' => $patron_2->id ); > > @libraries_where_can_see_patrons = ( $library_1->id ); > >- $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron_1->id )->status_is(200) >+ $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron_1->id ) >+ ->status_is(200) > ->json_is( '/patron_id' => $patron_1->id ); > >- $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->id )->status_is(404) >+ $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->id ) >+ ->status_is(404) > ->json_is( '/error' => 'Patron not found' ); > > $schema->storage->txn_rollback; >@@ -465,7 +488,8 @@ subtest 'add() tests' => sub { > my $userid = $librarian->userid; > t::lib::Mocks::mock_preference( 'PatronDuplicateMatchingAddFields', 'firstname|surname|dateofbirth' ); > >- $t->post_ok( "//$userid:$password@/api/v1/patrons" => json => $newpatron )->status_is(409) >+ $t->post_ok( "//$userid:$password@/api/v1/patrons" => json => $newpatron ) >+ ->status_is(409) > ->json_is( '/error' => "A patron record matching these details already exists" ); > > # Create a library just to make sure its ID doesn't exist on the DB >@@ -479,7 +503,8 @@ subtest 'add() tests' => sub { > > # Test duplicate userid constraint > $t->post_ok( "//$userid:$password@/api/v1/patrons" => { 'x-confirm-not-duplicate' => 1 } => json => $newpatron ) >- ->status_is(400)->json_is( '/error' => "Problem with " . $newpatron->{userid} ); >+ ->status_is(400) >+ ->json_is( '/error' => "Problem with " . $newpatron->{userid} ); > > $newpatron->{library_id} = $patron->branchcode; > >@@ -488,7 +513,8 @@ subtest 'add() tests' => sub { > warning_like { > $t->post_ok( > "//$userid:$password@/api/v1/patrons" => { 'x-confirm-not-duplicate' => 1 } => json => $newpatron ) >- ->status_is(409)->json_has( '/error', 'Fails when trying to POST duplicate cardnumber' ) >+ ->status_is(409) >+ ->json_has( '/error', 'Fails when trying to POST duplicate cardnumber' ) > ->json_like( '/conflict' => qr/(borrowers\.)?cardnumber/ ); > } > qr/DBD::mysql::st execute failed: Duplicate entry '(.*?)' for key '(borrowers\.)?cardnumber'/; >@@ -503,7 +529,8 @@ subtest 'add() tests' => sub { > $newpatron->{category_id} = $deleted_category_id; # Test invalid patron category > > $t->post_ok( "//$userid:$password@/api/v1/patrons" => { 'x-confirm-not-duplicate' => 1 } => json => $newpatron ) >- ->status_is(400)->json_is( '/error' => "Given category_id does not exist" ); >+ ->status_is(400) >+ ->json_is( '/error' => "Given category_id does not exist" ); > $newpatron->{category_id} = $patron->categorycode; > > $newpatron->{falseproperty} = "Non existent property"; >@@ -534,11 +561,15 @@ subtest 'add() tests' => sub { > $letter_enqueued = 0; > $t->post_ok( > "//$userid:$password@/api/v1/patrons" => { 'x-koha-override' => 'welcome_yes' } => json => $newpatron ) >- ->status_is( 201, 'Patron created successfully' )->header_like( >+ ->status_is( 201, 'Patron created successfully' ) >+ ->header_like( > Location => qr|^\/api\/v1\/patrons/\d*|, > 'REST3.4.1' >- )->json_has( '/patron_id', 'got a patron_id' )->json_is( '/cardnumber' => $newpatron->{cardnumber} ) >- ->json_is( '/surname' => $newpatron->{surname} )->json_is( '/firstname' => $newpatron->{firstname} ) >+ ) >+ ->json_has( '/patron_id', 'got a patron_id' ) >+ ->json_is( '/cardnumber' => $newpatron->{cardnumber} ) >+ ->json_is( '/surname' => $newpatron->{surname} ) >+ ->json_is( '/firstname' => $newpatron->{firstname} ) > ->json_is( '/date_of_birth' => $newpatron->{date_of_birth}, 'Date field set (Bug 28585)' ) > ->json_is( '/last_seen' => $newpatron->{last_seen}, 'Date-time field set (Bug 28585)' ); > >@@ -665,7 +696,8 @@ subtest 'add() tests' => sub { > "city" => "Konstanz", > "library_id" => "MPL" > } >- )->status_is(400) >+ ) >+ ->status_is(400) > ->json_is( '/error' => "Tried to add more than one non-repeatable attributes. type=$code value=$attr" ); > > is( Koha::Patrons->search->count, $patrons_count, 'No patron added' ); >@@ -680,7 +712,8 @@ subtest 'add() tests' => sub { > "city" => "Konstanz", > "library_id" => "MPL" > } >- )->status_is(400) >+ ) >+ ->status_is(400) > ->json_is( > '/error' => "Your action breaks a unique constraint on the attribute. type=$code value=$attr" ); > >@@ -841,7 +874,8 @@ subtest 'add() tests' => sub { > "city" => "Bigtown", > "library_id" => "MPL", > } >- )->status_is( 201, 'Patron added with EnhancedMessagingPreferences disabled' ) >+ ) >+ ->status_is( 201, 'Patron added with EnhancedMessagingPreferences disabled' ) > ->tx->res->json->{patron_id}; > > # No messaging preferences should be set >@@ -901,7 +935,8 @@ subtest 'update() tests' => sub { > delete $newpatron->{anonymized}; > delete $newpatron->{self_renewal_available}; > >- $t->put_ok( "//$userid:$password@/api/v1/patrons/-1" => json => $newpatron )->status_is(404) >+ $t->put_ok( "//$userid:$password@/api/v1/patrons/-1" => json => $newpatron ) >+ ->status_is(404) > ->json_has( '/error', 'Fails when trying to PUT nonexistent patron' ); > > # Create a library just to make sure its ID doesn't exist on the DB >@@ -915,7 +950,8 @@ subtest 'update() tests' => sub { > $newpatron->{category_id} = $deleted_category_id; > > $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->borrowernumber => json => $newpatron ) >- ->status_is(400)->json_is( '/error' => "Given category_id does not exist" ); >+ ->status_is(400) >+ ->json_is( '/error' => "Given category_id does not exist" ); > > # Restore the valid category > $newpatron->{category_id} = $patron_2->categorycode; >@@ -932,7 +968,8 @@ subtest 'update() tests' => sub { > > warning_like { > $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->borrowernumber => json => $newpatron ) >- ->status_is(400)->json_is( '/error' => "Given library_id does not exist" ); >+ ->status_is(400) >+ ->json_is( '/error' => "Given library_id does not exist" ); > } > qr/DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails/; > >@@ -943,7 +980,8 @@ subtest 'update() tests' => sub { > $newpatron->{falseproperty} = "Non existent property"; > > $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->borrowernumber => json => $newpatron ) >- ->status_is(400)->json_is( '/errors/0/message' => 'Properties not allowed: falseproperty.' ); >+ ->status_is(400) >+ ->json_is( '/errors/0/message' => 'Properties not allowed: falseproperty.' ); > > # Get rid of the invalid attribute > delete $newpatron->{falseproperty}; >@@ -953,7 +991,8 @@ subtest 'update() tests' => sub { > $newpatron->{userid} = $patron_1->userid; > > $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->borrowernumber => json => $newpatron ) >- ->status_is(400)->json_has( '/error', "Problem with userid " . $patron_1->userid ); >+ ->status_is(400) >+ ->json_has( '/error', "Problem with userid " . $patron_1->userid ); > > $newpatron->{cardnumber} = $patron_1->id . $patron_2->id; > $newpatron->{userid} = "user" . $patron_1->id . $patron_2->id; >@@ -1040,7 +1079,7 @@ subtest 'update() tests' => sub { > ->status_is( > 403, > "Non-superlibrarian user change of superlibrarian secondary_email to undefined forbidden" >- )->json_is( { error => "Not enough privileges to change a superlibrarian's email" } ); >+ )->json_is( { error => "Not enough privileges to change a superlibrarian's email" } ); > > $newpatron->{secondary_email} = $superlibrarian->emailpro; > $newpatron->{altaddress_email} = 'nonsense@no.no'; >@@ -1056,7 +1095,7 @@ subtest 'update() tests' => sub { > ->status_is( > 403, > "Non-superlibrarian user change of superlibrarian altaddress_email to undefined forbidden" >- )->json_is( { error => "Not enough privileges to change a superlibrarian's email" } ); >+ )->json_is( { error => "Not enough privileges to change a superlibrarian's email" } ); > > # update patron without sending email > delete $newpatron->{email}; >@@ -1127,13 +1166,15 @@ subtest 'update() tests' => sub { > "city" => "Konstanz", > "library_id" => "MPL" > } >- )->status_is(400) >+ ) >+ ->status_is(400) > ->json_is( > '/error' => "Missing mandatory extended attribute (type=" . $attr_type_mandatory->code . ")" ); > > $t->put_ok( "//$userid:$password@/api/v1/patrons/" > . $superlibrarian->borrowernumber => { 'x-koha-embed' => 'extended_attributes' } => json => >- $newpatron )->status_is(400) >+ $newpatron ) >+ ->status_is(400) > ->json_is( '/error' => 'Tried to use an invalid attribute type. type=' . $deleted_attr_code ) > ->json_is( '/error_code' => 'invalid_attribute_type' ); > >@@ -1150,11 +1191,13 @@ subtest 'update() tests' => sub { > > $t->put_ok( "//$userid:$password@/api/v1/patrons/" > . $superlibrarian->borrowernumber => { 'x-koha-embed' => 'extended_attributes' } => json => >- $newpatron )->status_is(400) >+ $newpatron ) >+ ->status_is(400) > ->json_is( '/error' => 'Your action breaks a unique constraint on the attribute. type=' > . $attr_type_unique->code > . ' value=' >- . $unique_attr->attribute )->json_is( '/error_code' => 'attribute_not_unique' ); >+ . $unique_attr->attribute ) >+ ->json_is( '/error_code' => 'attribute_not_unique' ); > > $newpatron->{extended_attributes} = [ > { type => $attr_type_repeatable->code, value => 'a' }, >@@ -1165,11 +1208,13 @@ subtest 'update() tests' => sub { > > $t->put_ok( "//$userid:$password@/api/v1/patrons/" > . $superlibrarian->borrowernumber => { 'x-koha-embed' => 'extended_attributes' } => json => >- $newpatron )->status_is(400) >+ $newpatron ) >+ ->status_is(400) > ->json_is( '/error' => 'Your action breaks a unique constraint on the attribute. type=' > . $attr_type_unique->code > . ' value=' >- . $unique_attr->attribute )->json_is( '/error_code' => 'attribute_not_unique' ); >+ . $unique_attr->attribute ) >+ ->json_is( '/error_code' => 'attribute_not_unique' ); > > $newpatron->{extended_attributes} = [ > { type => $attr_type_repeatable->code, value => 'a' }, >@@ -1179,10 +1224,12 @@ subtest 'update() tests' => sub { > > $t->put_ok( "//$userid:$password@/api/v1/patrons/" > . $superlibrarian->borrowernumber => { 'x-koha-embed' => 'extended_attributes' } => json => >- $newpatron )->status_is(400) >+ $newpatron ) >+ ->status_is(400) > ->json_is( '/error' => 'Tried to add more than one non-repeatable attributes. type=' > . $attr_type_mandatory->code >- . ' value=pong' )->json_is( '/error_code' => 'non_repeatable_attribute' ); >+ . ' value=pong' ) >+ ->json_is( '/error_code' => 'non_repeatable_attribute' ); > > my $unique_value = $unique_attr->attribute; > $unique_attr->delete; >@@ -1241,7 +1288,8 @@ subtest 'delete() tests' => sub { > > t::lib::Mocks::mock_preference( 'AnonymousPatron', $patron->borrowernumber ); > $t->delete_ok( "//$userid:$password@/api/v1/patrons/" . $patron->borrowernumber ) >- ->status_is( 409, 'Anonymous patron cannot be deleted' )->json_is( >+ ->status_is( 409, 'Anonymous patron cannot be deleted' ) >+ ->json_is( > { > error => 'Anonymous patron cannot be deleted', > error_code => 'is_anonymous_patron' >@@ -1263,7 +1311,8 @@ subtest 'delete() tests' => sub { > $guarantee->add_guarantor( { guarantor_id => $patron->id, relationship => 'parent' } ); > > $t->delete_ok( "//$userid:$password@/api/v1/patrons/" . $patron->borrowernumber ) >- ->status_is( 409, 'Patron with checkouts cannot be deleted' )->json_is( >+ ->status_is( 409, 'Patron with checkouts cannot be deleted' ) >+ ->json_is( > { > error => 'Pending checkouts prevent deletion', > error_code => 'has_checkouts' >@@ -1274,7 +1323,8 @@ subtest 'delete() tests' => sub { > $checkout->delete; > > $t->delete_ok( "//$userid:$password@/api/v1/patrons/" . $patron->borrowernumber ) >- ->status_is( 409, 'Patron with debt cannot be deleted' )->json_is( >+ ->status_is( 409, 'Patron with debt cannot be deleted' ) >+ ->json_is( > { > error => 'Pending debts prevent deletion', > error_code => 'has_debt' >@@ -1285,7 +1335,8 @@ subtest 'delete() tests' => sub { > $patron->account->pay( { amount => 10, debits => [$debit] } ); > > $t->delete_ok( "//$userid:$password@/api/v1/patrons/" . $patron->borrowernumber ) >- ->status_is( 409, 'Patron with guarantees cannot be deleted' )->json_is( >+ ->status_is( 409, 'Patron with guarantees cannot be deleted' ) >+ ->json_is( > { > error => 'Patron is a guarantor and it prevents deletion', > error_code => 'has_guarantees' >@@ -1298,7 +1349,8 @@ subtest 'delete() tests' => sub { > $patron->protected(1)->store(); > > $t->delete_ok( "//$userid:$password@/api/v1/patrons/" . $patron->borrowernumber ) >- ->status_is( 409, 'Protected patron cannot be deleted' )->json_is( >+ ->status_is( 409, 'Protected patron cannot be deleted' ) >+ ->json_is( > { > error => 'Protected patrons cannot be deleted', > error_code => 'is_protected' >@@ -1308,7 +1360,8 @@ subtest 'delete() tests' => sub { > $patron->protected(0)->store(); > > $t->delete_ok( "//$userid:$password@/api/v1/patrons/" . $patron->borrowernumber ) >- ->status_is( 204, 'REST3.2.4' )->content_is( '', 'REST3.3.4' ); >+ ->status_is( 204, 'REST3.2.4' ) >+ ->content_is( '', 'REST3.3.4' ); > > my $deleted_patrons = Koha::Old::Patrons->search( { borrowernumber => $patron->borrowernumber } ); > is( $deleted_patrons->count, 1, 'The patron has been moved to the vault' ); >@@ -1340,11 +1393,13 @@ subtest 'guarantors_can_see_charges() tests' => sub { > )->status_is(401)->json_is( { error => "Authentication failure." } ); > > $t->put_ok( "//$userid:$password@/api/v1/public/patrons/$other_patron_id/guarantors/can_see_charges" => json => >- { allowed => Mojo::JSON->true } )->status_is(403) >+ { allowed => Mojo::JSON->true } ) >+ ->status_is(403) > ->json_is( { error => "Unprivileged user cannot access another user's resources" } ); > > $t->put_ok( "//$userid:$password@/api/v1/public/patrons/$patron_id/guarantors/can_see_charges" => json => >- { allowed => Mojo::JSON->true } )->status_is(403) >+ { allowed => Mojo::JSON->true } ) >+ ->status_is(403) > ->json_is( '/error', 'The current configuration doesn\'t allow the requested action.' ); > > t::lib::Mocks::mock_preference( 'AllowPatronToSetFinesVisibilityForGuarantor', 1 ); >@@ -1382,13 +1437,15 @@ subtest 'guarantors_can_see_checkouts() tests' => sub { > { allowed => Mojo::JSON->true } )->status_is(401)->json_is( { error => "Authentication failure." } ); > > $t->put_ok( "//$userid:$password@/api/v1/public/patrons/$other_patron_id/guarantors/can_see_checkouts" => json => >- { allowed => Mojo::JSON->true } )->status_is(403) >+ { allowed => Mojo::JSON->true } ) >+ ->status_is(403) > ->json_is( { error => "Unprivileged user cannot access another user's resources" } ); > > t::lib::Mocks::mock_preference( 'AllowPatronToSetCheckoutsVisibilityForGuarantor', 0 ); > > $t->put_ok( "//$userid:$password@/api/v1/public/patrons/$patron_id/guarantors/can_see_checkouts" => json => >- { allowed => Mojo::JSON->true } )->status_is(403) >+ { allowed => Mojo::JSON->true } ) >+ ->status_is(403) > ->json_is( '/error', 'The current configuration doesn\'t allow the requested action.' ); > > t::lib::Mocks::mock_preference( 'AllowPatronToSetCheckoutsVisibilityForGuarantor', 1 ); >@@ -1431,7 +1488,8 @@ sub unauthorized_access_tests { > $unauthorized_patron->set_password( { password => $password, skip_validation => 1 } ); > my $unauth_userid = $unauthorized_patron->userid; > >- $t->$verb_ok( "//$unauth_userid:$password\@$endpoint" => json => $json )->status_is(403) >+ $t->$verb_ok( "//$unauth_userid:$password\@$endpoint" => json => $json ) >+ ->status_is(403) > ->json_has('/required_permissions'); > }; > } >diff --git a/t/db_dependent/api/v1/patrons_self_renewal.t b/t/db_dependent/api/v1/patrons_self_renewal.t >index 88662caff5f..d7bb966b69c 100755 >--- a/t/db_dependent/api/v1/patrons_self_renewal.t >+++ b/t/db_dependent/api/v1/patrons_self_renewal.t >@@ -64,14 +64,16 @@ subtest 'start()' => sub { > my $borrowernumber = $patron->borrowernumber; > > $t->get_ok("//$userid:$password@/api/v1/public/patrons/$borrowernumber/self_renewal") >- ->status_is( 403, 'REST3.2.2' )->json_is( { error => "You are not eligible for self-renewal" } ); >+ ->status_is( 403, 'REST3.2.2' ) >+ ->json_is( { error => "You are not eligible for self-renewal" } ); > > $category->self_renewal_enabled(1)->store(); > > t::lib::Mocks::mock_preference( 'OPACPatronDetails', 1 ); > > $t->get_ok("//$userid:$password@/api/v1/public/patrons/$borrowernumber/self_renewal") >- ->status_is( 200, 'REST3.2.2' )->json_is( >+ ->status_is( 200, 'REST3.2.2' ) >+ ->json_is( > { > self_renewal_settings => { > self_renewal_failure_message => $category->self_renewal_failure_message, >@@ -92,7 +94,8 @@ subtest 'start()' => sub { > } > ); > $t->get_ok("//$userid:$password@/api/v1/public/patrons/$borrowernumber/self_renewal") >- ->status_is( 200, 'REST3.2.2' )->json_is( >+ ->status_is( 200, 'REST3.2.2' ) >+ ->json_is( > { > self_renewal_settings => { > self_renewal_failure_message => $category->self_renewal_failure_message, >@@ -139,7 +142,8 @@ subtest 'submit()' => sub { > my $borrowernumber = $patron->borrowernumber; > > $t->post_ok( "//$userid:$password@/api/v1/public/patrons/$borrowernumber/self_renewal" => json => {} ) >- ->status_is( 403, 'REST3.2.2' )->json_is( { error => "You are not eligible for self-renewal" } ); >+ ->status_is( 403, 'REST3.2.2' ) >+ ->json_is( { error => "You are not eligible for self-renewal" } ); > > t::lib::Mocks::mock_preference( 'OPACPatronDetails', 1 ); > t::lib::Mocks::mock_preference( 'AutoApprovePatronProfileSettings', 0 ); >-- >2.39.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 26355
:
191781
|
191782
|
191783
|
191784
|
191785
|
191786
|
191787
|
191788
|
191789
|
192440
|
192441
|
192442
|
192443
|
192444
|
192445
|
192446
|
192447
|
192448
|
193065
|
193066
|
193067
|
193068
|
193069
|
193070
|
193071
|
193072
|
193073
|
193074
|
193249
|
193250
|
193251
|
193252
|
193253
|
193521
|
193522
|
193523
|
193524
|
193525
|
193526
|
193527
|
193528
|
193529
|
193581
|
193635
|
193654
|
193699
|
193756
|
193757
|
193758
|
193759
|
193760
|
193761
|
193762
|
193763
|
193764
|
193765
|
193921
|
194435
|
194464
|
194465
|
194466
|
194467
|
194468
|
194469
|
194470
|
194471
|
194472
|
194473
|
194474
|
194753
|
194754
|
194755
|
194756
|
194757
|
194758
|
194759
|
194760
|
194761
|
194763
|
194764
| 194765 |
194780