From 3079438d6fa519a158ab3c6c51cb109860ae3e2a Mon Sep 17 00:00:00 2001 From: Andrew Fuerste Henry 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 --- 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 . - 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