Bug 41421: [24.05.x] Fix DB update

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

C4/Acquisition.pm

@@ -277,7 +277,15 @@ sub GetBasketAsCSV {
 
         my $delimiter = $csv_profile->csv_separator;
         $delimiter = "\t" if $delimiter eq "\\t";
-        my $csv = Text::CSV_XS->new({'quote_char'=>'"','escape_char'=>'"','sep_char'=>$delimiter,'binary'=>1});
+        my $csv = Text::CSV_XS->new(
+            {
+                quote_char  => '"',
+                escape_char => '"',
+                sep_char    => $delimiter,
+                binary      => 1,
+                formula     => "empty",
+            }
+        );
         my $csv_profile_content = $csv_profile->content;
         my ( @headers, @fields );
         while ( $csv_profile_content =~ /

C4/Auth.pm

@@ -82,14 +82,9 @@ BEGIN {
         get_template_and_user haspermission create_basic_session
     );
 
-    $ldap      = C4::Context->config('useldapserver') || 0;
     $cas       = C4::Context->preference('casAuthentication');
     $caslogout = C4::Context->preference('casLogout');
 
-    if ($ldap) {
-        require C4::Auth_with_ldap;
-        import C4::Auth_with_ldap qw(checkpw_ldap);
-    }
     if ($cas) {
         require C4::Auth_with_cas;    # no import
         import C4::Auth_with_cas qw(check_api_auth_cas checkpw_cas login_cas logout_cas login_cas_url logout_if_required multipleAuth getMultipleAuth);
@@ -554,9 +549,9 @@ sub get_template_and_user {
         my $opac_limit_override = $ENV{'OPAC_LIMIT_OVERRIDE'};
         my $opac_name           = '';
         if (
-            ( $opac_limit_override && $opac_search_limit && $opac_search_limit =~ /branch:([\w-]+)/ ) ||
-            ( $in->{'query'}->param('limit') && $in->{'query'}->param('limit') =~ /branch:([\w-]+)/ ) ||
-            ( $in->{'query'}->param('limit') && $in->{'query'}->param('limit') =~ /multibranchlimit:(\w+)/ )
+            ( $opac_limit_override && $opac_search_limit && $opac_search_limit =~ /^branch:([\w-]+)/ ) ||
+            ( $in->{'query'}->param('limit') && $in->{'query'}->param('limit') =~ /^branch:([\w-]+)/ ) ||
+            ( $in->{'query'}->param('limit') && $in->{'query'}->param('limit') =~ /^multibranchlimit:(\w+)/ )
           ) {
             $opac_name = $1;    # opac_search_limit is a branch, so we use it.
         } elsif ( $in->{'query'}->param('multibranchlimit') ) {
@@ -1218,10 +1213,10 @@ sub checkauth {
                         my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list };
                         if ( C4::Context->preference('StaffLoginRestrictLibraryByIP') ) {
                             # we have to check they are coming from the right ip range
-                            my $domain = $branches->{$branchcode}->{'branchip'};
+                            my $domain = $branches->{$branchcode}->{'branchip'} // q{};
                             $domain =~ s|\.\*||g;
                             $domain =~ s/\s+//g;
-                            if ( $ip !~ /^$domain/ ) {
+                            if ( $domain && $ip !~ /^$domain/ ) {
                                 $cookie = $cookie_mgr->replace_in_list( $cookie, $query->cookie(
                                     -name     => 'CGISESSID',
                                     -value    => '',
@@ -1418,6 +1413,11 @@ sub checkauth {
     my $auth_error = $query->param('auth_error');
     my $auth_template_name = ( $type eq 'opac' ) ? 'opac-auth.tt' : 'auth.tt';
     my $template = C4::Templates::gettemplate( $auth_template_name, $type, $query );
+
+    my $borrowernumber      = $patron and $patron->borrowernumber;
+    my $anonymous_patron    = C4::Context->preference('AnonymousPatron');
+    my $is_anonymous_patron = $patron && ( $patron->borrowernumber eq $anonymous_patron );
+
     $template->param(
         login                                 => 1,
         INPUTS                                => \@inputs,
@@ -1452,6 +1452,7 @@ sub checkauth {
         opac_css_override                     => $ENV{'OPAC_CSS_OVERRIDE'},
         too_many_login_attempts               => ( $patron and $patron->account_locked ),
         password_has_expired                  => ( $patron and $patron->password_expired ),
+        is_anonymous_patron                   => ( $is_anonymous_patron ),
         auth_error                            => $auth_error,
     );
 
@@ -1633,7 +1634,7 @@ sub check_api_auth {
             -secure => ( C4::Context->https_enabled() ? 1 : 0 ),
             -sameSite => 'Lax'
         );
-        return ( $return, $cookie, $session ); # return == 'ok' here
+        return ( $return, $cookie, $session->id ); # return == 'ok' here
 
     } else {
 
@@ -1981,26 +1982,24 @@ sub checkpw {
     my $shib       = C4::Context->config('useshibboleth') && shib_ok();
     my $shib_login = $shib ? get_login_shib() : undef;
 
+    my $anonymous_patron = C4::Context->preference('AnonymousPatron');
+
     my @return;
-    my $patron;
-    if ( defined $userid ) {
-        $patron = Koha::Patrons->find( { userid     => $userid } );
-        $patron = Koha::Patrons->find( { cardnumber => $userid } ) unless $patron;
-    }
     my $check_internal_as_fallback = 0;
     my $passwd_ok                  = 0;
+    my $patron;
 
     # Note: checkpw_* routines returns:
     # 1 if auth is ok
     # 0 if auth is nok
     # -1 if user bind failed (LDAP only)
-
-    if ( $patron and ( $patron->account_locked ) ) {
-
-        # Nothing to check, account is locked
-    } elsif ( $ldap && defined($password) ) {
+    $ldap = C4::Context->config('useldapserver') || 0;
+    if ( $ldap && defined($password) ) {
         my ( $retval, $retcard, $retuserid );
-        ( $retval, $retcard, $retuserid, $patron ) = checkpw_ldap(@_);    # EXTERNAL AUTH
+        require C4::Auth_with_ldap;
+        import C4::Auth_with_ldap qw(checkpw_ldap);
+
+        ( $retval, $retcard, $retuserid, $patron ) = C4::Auth_with_ldap::checkpw_ldap(@_);    # EXTERNAL AUTH
         if ( $retval == 1 ) {
             @return    = ( $retval, $retcard, $retuserid, $patron );
             $passwd_ok = 1;
@@ -2012,8 +2011,8 @@ sub checkpw {
         # In case of a CAS authentication, we use the ticket instead of the password
         my $ticket = $query->param('ticket');
         $query->delete('ticket');                                         # remove ticket to come back to original URL
-        my ( $retval, $retcard, $retuserid, $cas_ticket, $patron ) =
-            checkpw_cas( $ticket, $query, $type );                        # EXTERNAL AUTH
+        my ( $retval, $retcard, $retuserid, $cas_ticket );
+        ( $retval, $retcard, $retuserid, $cas_ticket, $patron ) = checkpw_cas( $ticket, $query, $type ); # EXTERNAL AUTH
         if ($retval) {
             @return = ( $retval, $retcard, $retuserid, $patron, $cas_ticket );
         } else {
@@ -2033,7 +2032,8 @@ sub checkpw {
 
         # Then, we check if it matches a valid koha user
         if ($shib_login) {
-            my ( $retval, $retcard, $retuserid, $patron ) =
+            my ( $retval, $retcard, $retuserid );
+            ( $retval, $retcard, $retuserid, $patron ) =
                 C4::Auth_with_shibboleth::checkpw_shib($shib_login);    # EXTERNAL AUTH
             if ($retval) {
                 @return = ( $retval, $retcard, $retuserid, $patron );
@@ -2044,20 +2044,31 @@ sub checkpw {
         $check_internal_as_fallback = 1;
     }
 
-    # INTERNAL AUTH
     if ($check_internal_as_fallback) {
-        @return = checkpw_internal( $userid, $password, $no_set_userenv );
-        push( @return, $patron );
-        $passwd_ok = 1 if $return[0] > 0;    # 1 or 2
+
+        # INTERNAL AUTH
+        @return    = checkpw_internal( $userid, $password, $no_set_userenv );
+        $passwd_ok = $return[0];
+        $patron    = $passwd_ok ? $return[3] : undef;
+    }
+
+    if ( defined $userid && !$patron ) {
+        $patron = Koha::Patrons->find( { userid     => $userid } );
+        $patron = Koha::Patrons->find( { cardnumber => $userid } ) unless $patron;
+        push @return, $patron if $check_internal_as_fallback;    # We pass back the patron if authentication fails
     }
 
     if ($patron) {
-        if ($passwd_ok) {
+        if ( $patron->account_locked ) {
+            @return = ();
+        } elsif ($passwd_ok) {
             $patron->update( { login_attempts => 0 } );
-            if ( $patron->password_expired ) {
+            if ( defined($anonymous_patron) && ($patron->borrowernumber eq $anonymous_patron) ) {
+                @return = ( -3, $patron );
+            } elsif ( $patron->password_expired ) {
                 @return = ( -2, $patron );
             }
-        } elsif ( !$patron->account_locked ) {
+        } else {
             $patron->update( { login_attempts => $patron->login_attempts + 1 } );
         }
     }
@@ -2076,41 +2087,28 @@ sub checkpw_internal {
     my ( $userid, $password, $no_set_userenv ) = @_;
 
     $password = Encode::encode( 'UTF-8', $password )
-      if Encode::is_utf8($password);
+        if Encode::is_utf8($password);
 
-    my $dbh = C4::Context->dbh;
-    my $sth =
-      $dbh->prepare(
-        "select password,cardnumber,borrowernumber,userid,firstname,surname,borrowers.branchcode,branches.branchname,flags from borrowers join branches on borrowers.branchcode=branches.branchcode where userid=?"
-      );
-    $sth->execute($userid);
-    if ( $sth->rows ) {
-        my ( $stored_hash, $cardnumber, $borrowernumber, $userid, $firstname,
-            $surname, $branchcode, $branchname, $flags )
-          = $sth->fetchrow;
-
-        if ( checkpw_hash( $password, $stored_hash ) ) {
-
-            C4::Context->set_userenv( "$borrowernumber", $userid, $cardnumber,
-                $firstname, $surname, $branchcode, $branchname, $flags ) unless $no_set_userenv;
-            return 1, $cardnumber, $userid;
+    my $patron = Koha::Patrons->find( { userid => $userid } );
+    if ($patron) {
+        if ( checkpw_hash( $password, $patron->password ) ) {
+            my $borrowernumber = $patron->borrowernumber;
+            C4::Context->set_userenv(
+                "$borrowernumber", $patron->userid, $patron->cardnumber,
+                $patron->firstname, $patron->surname, $patron->branchcode, $patron->library->branchname, $patron->flags
+            ) unless $no_set_userenv;
+            return 1, $patron->cardnumber, $patron->userid, $patron;
         }
     }
-    $sth =
-      $dbh->prepare(
-        "select password,cardnumber,borrowernumber,userid,firstname,surname,borrowers.branchcode,branches.branchname,flags from borrowers join branches on borrowers.branchcode=branches.branchcode where cardnumber=?"
-      );
-    $sth->execute($userid);
-    if ( $sth->rows ) {
-        my ( $stored_hash, $cardnumber, $borrowernumber, $userid, $firstname,
-            $surname, $branchcode, $branchname, $flags )
-          = $sth->fetchrow;
-
-        if ( checkpw_hash( $password, $stored_hash ) ) {
-
-            C4::Context->set_userenv( $borrowernumber, $userid, $cardnumber,
-                $firstname, $surname, $branchcode, $branchname, $flags ) unless $no_set_userenv;
-            return 1, $cardnumber, $userid;
+    $patron = Koha::Patrons->find( { cardnumber => $userid } );
+    if ($patron) {
+        if ( checkpw_hash( $password, $patron->password ) ) {
+            my $borrowernumber = $patron->borrowernumber;
+            C4::Context->set_userenv(
+                "$borrowernumber", $patron->userid, $patron->cardnumber,
+                $patron->firstname, $patron->surname, $patron->branchcode, $patron->library->branchname, $patron->flags
+            ) unless $no_set_userenv;
+            return 1, $patron->cardnumber, $patron->userid, $patron;
         }
     }
     return 0;

C4/Auth_with_ldap.pm

@@ -212,12 +212,13 @@ sub checkpw_ldap {
     }
 
     if ($borrowernumber) {
-        if ($config{update}) { # A1, B1
-            my $c2 = &update_local($local_userid,$password,$borrowernumber,\%borrower) || '';
-            ($cardnumber eq $c2) or warn "update_local returned cardnumber '$c2' instead of '$cardnumber'";
-        } else { # C1, D1
-            # maybe update just the password?
-		return(1, $cardnumber, $local_userid);
+        if ( $config{update} ) {    # A1, B1
+            my $c2 = &update_local( $local_userid, $password, $borrowernumber, \%borrower ) || '';
+            ( $cardnumber eq $c2 ) or warn "update_local returned cardnumber '$c2' instead of '$cardnumber'";
+        } else {                    # C1, D1
+                                    # maybe update just the password?
+            my $patron = Koha::Patrons->find($borrowernumber);
+            return ( 1, $cardnumber, $local_userid, $patron );
         }
     } elsif ($config{replicate}) { # A2, C2
         my @columns = Koha::Patrons->columns;
@@ -226,6 +227,7 @@ sub checkpw_ldap {
                 map { exists( $borrower{$_} ) ? ( $_ => $borrower{$_} ) : () } @columns
             }
         )->store;
+        $patron->discard_changes;
         die "Insert of new patron failed" unless $patron;
         $borrowernumber = $patron->borrowernumber;
         C4::Members::Messaging::SetMessagingPreferencesFromDefaults(

C4/Auth_with_shibboleth.pm

@@ -135,6 +135,8 @@ sub _autocreate {
     }
 
     my $patron = Koha::Patron->new( \%borrower )->store;
+    $patron->discard_changes;
+
     C4::Members::Messaging::SetMessagingPreferencesFromDefaults(
         {
             borrowernumber => $patron->borrowernumber,
@@ -172,7 +174,7 @@ sub _autocreate {
             C4::Letters::SendQueuedMessages( { message_id => $message_id } ) if $message_id;
         }
     }
-    return ( 1, $patron->cardnumber, $patron->userid );
+    return ( 1, $patron->cardnumber, $patron->userid, $patron );
 }
 
 sub _sync {
@@ -409,7 +411,7 @@ This routine is NOT exported
 
 =head2 _autocreate
 
-  my ( $retval, $retcard, $retuserid ) = _autocreate( $config, $match );
+  my ( $retval, $retcard, $retuserid, $patron ) = _autocreate( $config, $match, $patron );
 
 Given a shibboleth attribute reference and a userid this internal routine will add the given user to Koha and return their user credentials.
 

C4/AuthoritiesMarc.pm

@@ -323,17 +323,17 @@ sub SearchAuthorities {
             if ( C4::Context->preference('ShowHeadingUse') ) {
                 # checking valid heading use
                 my $f008 = $authrecord->field('008');
-                my $pos14to16 = substr( $f008->data, 14, 3 );
-                my $main = substr( $pos14to16, 0, 1 );
-                $newline{main} = 1 if $main eq 'a';
-                my $subject = substr( $pos14to16, 1, 1);
-                $newline{subject} = 1 if $subject eq 'a';
-                my $series = substr( $pos14to16, 2, 1 );
-                $newline{series} = 1 if $series eq 'a';
+                if ($f008) {
+                    my $pos14to16 = substr( $f008->data, 14, 3 );
+                    my $main      = substr( $pos14to16,  0,  1 );
+                    $newline{main} = 1 if $main eq 'a';
+                    my $subject = substr( $pos14to16, 1, 1 );
+                    $newline{subject} = 1 if $subject eq 'a';
+                    my $series = substr( $pos14to16, 2, 1 );
+                    $newline{series} = 1 if $series eq 'a';
+                }
             }
-
-            $newline{authtype}     = defined($thisauthtype) ?
-                                        $thisauthtype->authtypetext : '';
+            $newline{authtype}     = defined($thisauthtype) ? $thisauthtype->authtypetext : '';
             $newline{summary}      = $summary;
             $newline{even}         = $counter % 2;
             $newline{reported_tag} = $reported_tag;
@@ -1009,14 +1009,13 @@ sub BuildSummary {
         } } $record->field('7..');
 
     } else {
-# construct MARC21 summary
-# FIXME - looping over 1XX is questionable
-# since MARC21 authority should have only one 1XX
+        # construct MARC21 summary
+        # FIXME - looping over 1XX is questionable
+        # since MARC21 authority should have only one 1XX
         use C4::Heading::MARC21;
         my $handler = C4::Heading::MARC21->new();
         my $subfields_to_report;
-        my $subfields_to_subdivision = "";
-        my $delimiter                = C4::Context->preference('AuthoritySeparator');
+        my $delimiter = C4::Context->preference('AuthoritySeparator');
 
         foreach my $field ($record->field('1..')) {
             my $tag = $field->tag();
@@ -1095,7 +1094,8 @@ sub BuildSummary {
             push @notes, { note => $field->as_string(), field => $field->tag() };
         }
 
-     foreach my $field ( $record->field('7..') ) {
+    foreach my $field ( $record->field('7..') ) {
+        my $subfields_to_subdivision;
         my $tag = $field->tag();
 
         if ( $tag eq '700' ) {
@@ -1138,9 +1138,11 @@ sub BuildSummary {
 
         my $heading = $field->as_string($subfields_to_report);
 
-        my $subheading = $field->as_string( $subfields_to_subdivision, $delimiter );
-        if ( length $subheading > 0 ) {
-            $heading .= $delimiter . $subheading;
+        if ($subfields_to_subdivision) {
+            my $subheading = $field->as_string( $subfields_to_subdivision, $delimiter );
+            if ( length $subheading > 0 ) {
+                $heading .= $delimiter . $subheading;
+            }
         }
 
         if ($subfields_to_report) {

C4/Barcodes.pm

@@ -94,11 +94,12 @@ sub max {
 	return $self->db_max;
 }
 sub db_max {
-	my $self = shift;
-	my $query = "SELECT max(abs(barcode)) FROM items LIMIT 1"; # Possible problem if multiple barcode types populated
-	my $sth = C4::Context->dbh->prepare($query);
-	$sth->execute();
-	return $sth->fetchrow_array || $self->initial;
+    my $self  = shift;
+    my $query = "SELECT max(abs(CAST(barcode AS UNSIGNED))) FROM items LIMIT 1"
+        ;    # Possible problem if multiple barcode types populated
+    my $sth = C4::Context->dbh->prepare($query);
+    $sth->execute();
+    return $sth->fetchrow_array || $self->initial;
 }
 sub next_value {
 	my $self = shift;

C4/Biblio.pm

@@ -555,9 +555,13 @@ sub DelBiblio {
 
     # We delete any existing holds
     my $holds = $biblio->holds;
+    $holds->update( { deleted_biblionumber => $biblionumber }, { no_triggers => 1 } );
+    my $old_holds = $biblio->old_holds;
+    $old_holds->update( { deleted_biblionumber => $biblionumber }, { no_triggers => 1 } );
     while ( my $hold = $holds->next ) {
+
         # no need to update the holds queue on each step, we'll do it at the end
-        $hold->cancel({ skip_holds_queue => 1 });
+        $hold->cancel( { skip_holds_queue => 1 } );
     }
 
     # We update any existing orders
@@ -3100,19 +3104,21 @@ sub UpdateTotalIssues {
         $exception->rethrow unless ( $exception->isa('Koha::Exceptions::Metadata::Invalid') );
         warn $exception;
         warn "UpdateTotalIssues could not get bibliographic record for biblionumber $biblionumber";
-        return;
+        return -1;
     }
     my $biblioitem = $biblio->biblioitem;
     my ($totalissuestag, $totalissuessubfield) = GetMarcFromKohaField( 'biblioitems.totalissues' );
     unless ($totalissuestag) {
-        return 1; # There is nothing to do
+        return 0; # There is nothing to do
     }
 
+    my $current_issues = $biblioitem->totalissues // 0;
     if (defined $value) {
         $totalissues = $value;
     } else {
         $totalissues = $biblioitem->totalissues + $increase;
     }
+    return 0 if $current_issues == $totalissues;    # No need to update if no changes
 
      my $field = $record->field($totalissuestag);
      if (defined $field) {

C4/Circulation.pm

@@ -1266,9 +1266,8 @@ sub CanBookBeIssued {
 
     ## CHECK AGE RESTRICTION
     my $agerestriction = $biblioitem->agerestriction;
-    my ( $restriction_age, $daysToAgeRestriction ) =
-      GetAgeRestriction( $agerestriction, $patron->unblessed );
-    if ( $daysToAgeRestriction && $daysToAgeRestriction > 0 ) {
+    my $restriction_age = GetAgeRestriction( $agerestriction );
+    if ( $restriction_age && $patron->dateofbirth && $restriction_age > $patron->get_age() ) {
         if ( C4::Context->preference('AgeRestrictionOverride') ) {
             $needsconfirmation{AGE_RESTRICTION} = "$agerestriction";
         }
@@ -1591,7 +1590,7 @@ sub AddIssue {
                       $issuedate,    # here interpreted as the renewal date
                 }
             );
-            $issue = $item_object->checkout;
+            $issue = $item_object->checkout->get_from_storage;
         }
         else {
 
@@ -2461,7 +2460,7 @@ sub AddReturn {
         ccode          => $item->ccode,
         categorycode   => $categorycode,
         interface      => C4::Context->interface,
-    }) unless ( $skip_localuse && $stat_type eq 'localuse' );
+    }) unless ( $skip_localuse && !$issue );
 
     # Send a check-in slip. # NOTE: borrower may be undef. Do not try to send messages then.
     if ( $patron ) {
@@ -4421,21 +4420,15 @@ sub IsItemIssued {
 
 =head2 GetAgeRestriction
 
-  my ($ageRestriction, $daysToAgeRestriction) = GetAgeRestriction($record_restrictions, $borrower);
-  my ($ageRestriction, $daysToAgeRestriction) = GetAgeRestriction($record_restrictions);
-
-  if($daysToAgeRestriction <= 0) { #Borrower is allowed to access this material, as they are older or as old as the agerestriction }
-  if($daysToAgeRestriction > 0) { #Borrower is this many days from meeting the agerestriction }
+  my $ageRestriction = GetAgeRestriction($record_restrictions);
 
 @PARAM1 the koha.biblioitems.agerestriction value, like K18, PEGI 13, ...
-@PARAM2 a borrower-object with koha.borrowers.dateofbirth. (OPTIONAL)
-@RETURNS The age restriction age in years and the days to fulfill the age restriction for the given borrower.
-         Negative days mean the borrower has gone past the age restriction age.
+@RETURNS The age restriction age in years.
 
 =cut
 
 sub GetAgeRestriction {
-    my ($record_restrictions, $borrower) = @_;
+    my ($record_restrictions) = @_;
     my $markers = C4::Context->preference('AgeRestrictionMarker');
 
     return unless $record_restrictions;
@@ -4470,26 +4463,7 @@ sub GetAgeRestriction {
         last if ( $restriction_year > 0 );
     }
 
-    #Check if the borrower is age restricted for this material and for how long.
-    if ($restriction_year && $borrower) {
-        if ( $borrower->{'dateofbirth'} ) {
-            my @alloweddate = split /-/, $borrower->{'dateofbirth'};
-            $alloweddate[0] += $restriction_year;
-
-            #Prevent runime eror on leap year (invalid date)
-            if ( ( $alloweddate[1] == 2 ) && ( $alloweddate[2] == 29 ) ) {
-                $alloweddate[2] = 28;
-            }
-
-            #Get how many days the borrower has to reach the age restriction
-            my @Today = split /-/, dt_from_string()->ymd();
-            my $daysToAgeRestriction = Date_to_Days(@alloweddate) - Date_to_Days(@Today);
-            #Negative days means the borrower went past the age restriction age
-            return ($restriction_year, $daysToAgeRestriction);
-        }
-    }
-
-    return ($restriction_year);
+    return $restriction_year;
 }
 
 

C4/Context.pm

@@ -364,9 +364,11 @@ sub multivalue_preference {
     my ( $self, $preference ) = @_;
 
     my $syspref = $self->preference($preference) // q{};
-    my $values  = [ split qr{\|}, $syspref ];
 
-    return $values;
+    return [ split qr{\|}, $syspref ]
+        if $syspref =~ qr{\|};
+
+    return [ split qr{,}, $syspref ]
 }
 
 =head2 enable_syspref_cache

C4/Creators/Lib.pm

@@ -345,8 +345,7 @@ sub get_label_summary {
         $record->{'title'} =~ s/\W*$//;  # strip off ugly trailing chars
         # FIXME contructing staff interface URLs should be done *much* higher up the stack - for the most part, C4 module code
         # should not know that it's part of a web app
-        $record->{'title'} = '<a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=' . $record->{'biblionumber'} . '"> ' . $record->{'title'} . '</a>';
-        $label_summary->{'_summary'} = $record->{'title'} . " | " . ($record->{'author'} ? $record->{'author'} : 'N/A');
+        $label_summary->{'_summary'} = { title => $record->{title}, author => $record->{author}, biblionumber => $record->{biblionumber} };
         $label_summary->{'_item_type'} = C4::Context->preference("item-level_itypes") ? $record->{'itype'} : $record->{'itemtype'};
         $label_summary->{'_barcode'} = $record->{'barcode'};
         $label_summary->{'_item_number'} = $item->{'item_number'};

C4/Heading.pm

@@ -151,22 +151,6 @@ sub authorities {
     return $results;
 }
 
-=head2 preferred_authorities
-
-  my $preferred_authorities = $heading->preferred_authorities;
-
-Return a list of authority records for headings
-that are a preferred form of the heading.
-
-=cut
-
-sub preferred_authorities {
-    my $self = shift;
-    my $skipmetadata = shift || undef;
-    my ( $results, $total ) = _search( 'see-from', $skipmetadata );
-    return $results;
-}
-
 =head2 valid_heading_subfield
 
     if (C4::Heading::valid_heading_subfield('100', 'e', '')) ...

C4/HoldsQueue.pm

@@ -421,7 +421,7 @@ sub _allocateWithTransportCostMatrix {
     my @remaining_items = grep { !exists $allocated_items->{ $_->{itemnumber} } && $_->{holdallowed} ne 'not_allowed'; }
         @$available_items;
 
-    my @requests  = grep { !defined $_->{itemnumber} } @$hold_requests;
+    my @requests  = grep { !defined $_->{itemnumber} && !defined $_->{allocated} } @$hold_requests;
     my @remaining = ();
 
     my $num_agents = scalar(@remaining_items);

C4/ILSDI/Services.pm

@@ -218,7 +218,7 @@ sub GetRecords {
 
         my $biblioitem = $biblio->biblioitem->unblessed;
 
-        my $record = $biblio->metadata->record({ embed_items => 1 });
+        my $record = $biblio->metadata->record({ embed_items => 1, opac => 1, });
         if ($record) {
             $biblioitem->{marcxml} = $record->as_xml_record( C4::Context->preference('marcflavour') );
         }
@@ -236,7 +236,7 @@ sub GetRecords {
         foreach my $checkout (@$checkouts) {
             delete $checkout->{'borrowernumber'};
         }
-        my @items            = $biblio->items->as_list;
+        my @items            = $biblio->items->filter_by_visible_in_opac->as_list;
 
         $biblioitem->{items}->{item} = [];
 
@@ -968,7 +968,7 @@ sub _availability {
     my $location = $library ? $library->branchname : '';
     my $itemcallnumber = $item->itemcallnumber;
 
-    if ( $item->is_notforloan ) {
+    if ( $item->effective_not_for_loan_status ) {
         return ( $biblionumber, __('not available'), __('Not for loan'), $location, $itemcallnumber );
     } elsif ( $item->onloan ) {
         return ( $biblionumber, __('not available'), __('Checked out'), $location, $itemcallnumber );

C4/ImportExportFramework.pm

@@ -291,6 +291,7 @@ sub _export_table_csv
             for my $field (@fields) {
                 my $value = $hashRef->{$field} // q||;
                 $value =~ s/[\r\n]//g;
+                $value =~ s/"/""/g;
                 $$strCSV .= '"' . $value . '",';
             }
             chop $$strCSV;
@@ -965,7 +966,7 @@ sub _import_table_csv
     shift @fieldsPK;
     my $ok = 0;
     my $pos = 0;
-    my $csv = Text::CSV_XS->new ({ binary => 1 });
+    my $csv = Text::CSV_XS->new( { binary => 1, formula => "empty" } );
     while ( my $row = $csv->getline($dom) ) {
         my @fields = @$row;
         @arrData = @fields;

C4/Items.pm

@@ -45,6 +45,7 @@ BEGIN {
 }
 
 use Carp qw( croak );
+use C4::Circulation qw( barcodedecode );
 use C4::Context;
 use C4::Koha;
 use C4::Biblio qw( GetMarcStructure TransformMarcToKoha );
@@ -459,11 +460,15 @@ sub CheckItemPreSave {
     my %errors = ();
 
     # check for duplicate barcode
-    if (exists $item_ref->{'barcode'} and defined $item_ref->{'barcode'}) {
-        my $existing_item= Koha::Items->find({barcode => $item_ref->{'barcode'}});
+    if ( exists $item_ref->{'barcode'} and defined $item_ref->{'barcode'} ) {
+        my $barcode       = C4::Circulation::barcodedecode( $item_ref->{'barcode'} );
+        my $existing_item = Koha::Items->find( { barcode => $barcode } );
         if ($existing_item) {
-            if (!exists $item_ref->{'itemnumber'}                       # new item
-                or $item_ref->{'itemnumber'} != $existing_item->itemnumber) { # existing item
+            if (
+                !exists $item_ref->{'itemnumber'}    # new item
+                or $item_ref->{'itemnumber'} != $existing_item->itemnumber
+                )
+            {                                        # existing item
                 $errors{'duplicate_barcode'} = $item_ref->{'barcode'};
             }
         }
@@ -1178,7 +1183,7 @@ sub SearchItems {
           LEFT JOIN biblio ON biblio.biblionumber = items.biblionumber
           LEFT JOIN biblioitems ON biblioitems.biblioitemnumber = items.biblioitemnumber
           LEFT JOIN biblio_metadata ON biblio_metadata.biblionumber = biblio.biblionumber
-          LEFT JOIN issues ON issues.itemnumber = items.itemnumber
+          LEFT JOIN issues USING (itemnumber)
           WHERE 1
     };
     if (defined $where_str and $where_str ne '') {

C4/Labels/Label.pm

@@ -115,7 +115,7 @@ sub _get_label_item {
 
 sub _get_text_fields {
     my $format_string = shift;
-    my $csv = Text::CSV_XS->new({allow_whitespace => 1});
+    my $csv = Text::CSV_XS->new( { allow_whitespace => 1, formula => "empty" } );
     my $status = $csv->parse($format_string);
     my @sorted_fields = map {{ 'code' => $_, desc => $_ }} 
                         map { $_ && $_ eq 'callnumber' ? 'itemcallnumber' : $_ } # see bug 5653

C4/Letters.pm

@@ -557,24 +557,30 @@ sub SendAlerts {
 
 =head2 GetPreparedLetter( %params )
 
-    %params hash:
-      module => letter module, mandatory
-      letter_code => letter code, mandatory
-      branchcode => for letter selection, if missing default system letter taken
-      tables => a hashref with table names as keys. Values are either:
-        - a scalar - primary key value
-        - an arrayref - primary key values
-        - a hashref - full record
-      substitute => custom substitution key/value pairs
-      repeat => records to be substituted on consecutive lines:
-        - an arrayref - tries to guess what needs substituting by
-          taking remaining << >> tokensr; not recommended
-        - a hashref token => @tables - replaces <token> << >> << >> </token>
-          subtemplate for each @tables row; table is a hashref as above
-      want_librarian => boolean,  if set to true triggers librarian details
-        substitution from the userenv
-    Return value:
-      letter fields hashref (title & content useful)
+    my $letter = GetPreparedLetter(
+        {
+            module      => "letter module",           # mandatory
+            letter_code => "letter code",             # mandatory
+            branchcode  => "for letter selection",    # if missing default system letter taken
+            objects     => {},                        # a hashref with object names as keys and objects as values
+            tables      => {},                        # a hashref with table names as keys. Values are either:
+                                                      #  - a scalar - primary key value
+                                                      #  - an arrayref - primary key values
+                                                      #  - a hashref - full record
+            substitute  => {},                        # custom substitution key/value pairs
+            repeat      => '',                        # records to be substituted on consecutive lines, accepts either:
+                                                      #  - an arrayref
+                                                      #    tries to guess what needs substituting by taking remaining
+                                                      #    << >> tokensr; not recommended
+                                                      #  - a hashref token => @tables
+                                                      #    replaces <token> << >> << >> </token> subtemplate for each
+                                                      #    @tables row; table is a hashref as above
+            want_librarian => ''                      # boolean,  if set to true triggers librarian details substitution
+                                                      # from the userenv
+        }
+    );
+
+Return value: letter fields hashref (title & content useful)
 
 =cut
 
@@ -1563,8 +1569,8 @@ sub _wrap_html {
     $css = qq{<link rel="stylesheet" type="text/css" href="$css">} if $css;
     return <<EOS;
 <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
-    "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
-<html lang="en" xml:lang="en" xmlns="http://www.w3.org/1999/xhtml">
+    "https://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<html lang="en" xml:lang="en" xmlns="https://www.w3.org/1999/xhtml">
 <head>
 <title>$title</title>
 <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
@@ -1701,8 +1707,8 @@ sub _process_tt {
     my $use_template_cache = C4::Context->config('template_cache_dir') && defined $ENV{GATEWAY_INTERFACE};
     my $template           = Template->new(
         {
-            EVAL_PERL    => 1,
-            ABSOLUTE     => 1,
+            EVAL_PERL    => 0,
+            ABSOLUTE     => 0,
             PLUGIN_BASE  => 'Koha::Template::Plugin',
             COMPILE_EXT  => $use_template_cache ? '.ttc' : '',
             COMPILE_DIR  => $use_template_cache ? C4::Context->config('template_cache_dir') : '',
@@ -1890,6 +1896,7 @@ sub _get_tt_params {
         },
     };
 
+    my $dbh = C4::Context->dbh;
     foreach my $table ( keys %$tables ) {
         next unless $config->{$table};
 
@@ -1910,12 +1917,19 @@ sub _get_tt_params {
                 my $objects = $module->search(
                     { $key => $values },
                     {
-                            # We want to retrieve the data in the same order
-                            # FIXME MySQLism
-                            # field is a MySQLism, but they are no other way to do it
-                            # To be generic we could do it in perl, but we will need to fetch
-                            # all the data then order them
-                        @$values ? ( order_by => \[ "field($key, " . join( ', ', @$values ) . ")" ] ) : ()
+                        # We want to retrieve the data in the same order
+                        # FIXME MySQLism
+                        # field is a MySQLism, but they are no other way to do it
+                        # To be generic we could do it in perl, but we will need to fetch
+                        # all the data then order them
+                        @$values
+                        ? (
+                            order_by => \[
+                                sprintf "field(%s, %s)", $key,
+                                join(',', map { $dbh->quote($_) } @$values )
+                            ]
+                            )
+                        : ()
                     }
                 );
                 $params->{ $config->{$table}->{plural} } = $objects;

C4/Linker/Default.pm

@@ -31,6 +31,7 @@ sub get_link {
     my $search_form = $heading->search_form();
     my $auth_type = $heading->auth_type();
     my $thesaurus   = $heading->{thesaurus} || 'notdefined';
+    $thesaurus = 'notconsidered' unless C4::Context->preference('LinkerConsiderThesaurus');
     my $authid;
     my $fuzzy = 0;
     my $match_count;
@@ -91,6 +92,7 @@ sub update_cache {
     my $search_form = $heading->search_form();
     my $auth_type = $heading->auth_type();
     my $thesaurus   = $heading->{thesaurus} || 'notdefined';
+    $thesaurus = 'notconsidered' unless C4::Context->preference('LinkerConsiderThesaurus');
     my $fuzzy = 0;
 
     $self->{'cache'}->{$search_form.$auth_type.$thesaurus}->{'cached'} = 1;

C4/Overdues.pm

@@ -288,10 +288,12 @@ sub CalcFine {
         && $amount > $issuing_rule->{overduefinescap};
 
     # This must be moved to Koha::Item (see also similar code in C4::Accounts::chargelostitem
-    $item->{replacementprice} ||= $itemtype->defaultreplacecost
-      if $itemtype
-      && ( ! defined $item->{replacementprice} || $item->{replacementprice} == 0 )
-      && C4::Context->preference("useDefaultReplacementCost");
+    if (   $itemtype
+        && ( !defined $item->{replacementprice} || $item->{replacementprice} + .0 == 0 )
+        && C4::Context->preference("useDefaultReplacementCost") )
+    {
+        $item->{replacementprice} = $itemtype->defaultreplacecost;
+    }
 
     $amount = $item->{replacementprice} if ( $issuing_rule->{cap_fine_to_replacement_price} && $item->{replacementprice} && $amount > $item->{replacementprice} );
 
@@ -683,7 +685,7 @@ sub GetBranchcodesWithOverdueRules {
         WHERE delay1 IS NOT NULL
         ORDER BY branchcode
     |);
-    if ( $branchcodes->[0] eq '' ) {
+    if ( defined $branchcodes->[0] && $branchcodes->[0] eq '' ) {
         # If a default rule exists, all branches should be returned
         return Koha::Libraries->search({}, { order_by => 'branchname' })->get_column('branchcode');
     }

C4/Patroncards/Patroncard.pm

@@ -286,7 +286,7 @@ sub draw_text {
 
                 #                $font_units_width = $m->string_width($line);
                 #                $string_width = ($font_units_width * $text_attribs->{'font_size'}) / $units_per_em;
-                if ( ( $string_width + $text_attribs->{'llx'} ) < $self->{'width'} ) {
+                if ( $string_width && ( ( $string_width + $text_attribs->{'llx'} ) < $self->{'width'} ) ) {
                     ( $Tx, $Tw ) = text_alignment(
                         $origin_llx, $self->{'width'}, $text_attribs->{'llx'}, $string_width, $line,
                         $text_attribs->{'text_alignment'}

C4/Record.pm

@@ -401,7 +401,7 @@ sub marc2csv {
     my ($biblios, $id, $itemnumbers) = @_;
     $itemnumbers ||= [];
     my $output;
-    my $csv = Text::CSV::Encoded->new();
+    my $csv = Text::CSV::Encoded->new( { formula => "empty" } );
 
     # Getting yaml file
     my $configfile = "../tools/csv-profiles/$id.yaml";

C4/Reports/Guided.pm

@@ -220,10 +220,13 @@ sub _get_columns {
     $tablehash{'__first__'} = $first;
 	push @columns, \%tablehash;
     while ( my $data = $sth->fetchrow_arrayref() ) {
-        my %temphash;
-        $temphash{'name'}        = "$tablename.$data->[0]";
-        $temphash{'description'} = $columns->{$tablename}->{$data->[0]};
-        push @columns, \%temphash;
+        my $forbidden   = Koha::Report->new->check_columns( undef, [$data->[0]] );
+        unless ( $forbidden ) {
+            my %temphash;
+            $temphash{'name'}        = "$tablename.$data->[0]";
+            $temphash{'description'} = $columns->{$tablename}->{$data->[0]};
+            push @columns, \%temphash;
+        }
     }
     $sth->finish();
     return (@columns);
@@ -623,6 +626,11 @@ sub execute_query {
         ->info("Report finished: $report_id") if $report_id;
 
     return ( $sth, { queryerr => $sth->errstr } ) if ( $sth->err );
+
+    # Check if table.* contained forbidden column names
+    return ( $sth, { passworderr => "Illegal column in results" } )
+        if Koha::Report->new->check_columns( undef, $sth->{NAME_lc} );
+
     return ($sth);
 }
 

C4/Reserves.pm

@@ -44,7 +44,6 @@ use Koha::Holds;
 use Koha::ItemTypes;
 use Koha::Items;
 use Koha::Libraries;
-use Koha::Old::Holds;
 use Koha::Patrons;
 use Koha::Plugins;
 use Koha::Policy::Holds;
@@ -485,10 +484,6 @@ sub CanItemBeReserved {
         }
     }
 
-    # we retrieve borrowers and items informations #
-    # item->{itype} will come for biblioitems if necessery
-    my $borrower = $patron->unblessed;
-
     # If an item is damaged and we don't allow holds on damaged items, we can stop right here
     return _cache { status =>'damaged' }
       if ( $item->damaged
@@ -497,9 +492,8 @@ sub CanItemBeReserved {
     if( GetMarcFromKohaField('biblioitems.agerestriction') ){
         my $biblio = $item->biblio;
         # Check for the age restriction
-        my ( $ageRestriction, $daysToAgeRestriction ) =
-          C4::Circulation::GetAgeRestriction( $biblio->biblioitem->agerestriction, $borrower );
-        return _cache { status => 'ageRestricted' } if $daysToAgeRestriction && $daysToAgeRestriction > 0;
+        my $ageRestriction = C4::Circulation::GetAgeRestriction( $biblio->biblioitem->agerestriction );
+        return _cache { status => 'ageRestricted' } if $ageRestriction && $patron->dateofbirth && $ageRestriction > $patron->get_age();
     }
 
     # Check that the patron doesn't have an item level hold on this item already
@@ -527,14 +521,14 @@ sub CanItemBeReserved {
     }
     elsif ( $controlbranch eq "PatronLibrary" ) {
         $branchfield = "borrowers.branchcode";
-        $reserves_control_branch  = $borrower->{branchcode};
+        $reserves_control_branch  = $patron->branchcode;
     }
 
     # we retrieve rights
     if (
         my $reservesallowed = Koha::CirculationRules->get_effective_rule({
                 itemtype     => $item->effective_itemtype,
-                categorycode => $borrower->{categorycode},
+                categorycode => $patron->categorycode,
                 branchcode   => $reserves_control_branch,
                 rule_name    => 'reservesallowed',
         })
@@ -547,7 +541,7 @@ sub CanItemBeReserved {
     }
 
     my $rights = Koha::CirculationRules->get_effective_rules({
-        categorycode => $borrower->{'categorycode'},
+        categorycode => $patron->categorycode,
         itemtype     => $item->effective_itemtype,
         branchcode   => $reserves_control_branch,
         rules        => ['holds_per_record','holds_per_day']
@@ -656,7 +650,7 @@ sub CanItemBeReserved {
     }
 
     if (   $branchitemrule->{holdallowed} eq 'from_home_library'
-        && $borrower->{branchcode} ne $item->homebranch )
+        && $patron->branchcode ne $item->homebranch )
     {
         return _cache { status => 'cannotReserveFromOtherBranches' };
     }
@@ -685,7 +679,7 @@ sub CanItemBeReserved {
         if ($branchitemrule->{hold_fulfillment_policy} eq 'holdgroup' && !$item_library->validate_hold_sibling( {branchcode => $pickup_branchcode} )) {
             return _cache { status => 'pickupNotInHoldGroup' };
         }
-        if ($branchitemrule->{hold_fulfillment_policy} eq 'patrongroup' && !Koha::Libraries->find({branchcode => $borrower->{branchcode}})->validate_hold_sibling({branchcode => $pickup_branchcode})) {
+        if ($branchitemrule->{hold_fulfillment_policy} eq 'patrongroup' && !Koha::Libraries->find({branchcode => $patron->branchcode})->validate_hold_sibling({branchcode => $pickup_branchcode})) {
             return _cache { status => 'pickupNotInHoldGroup' };
         }
     }
@@ -1602,6 +1596,7 @@ sub _FixPriority {
         $hold = Koha::Holds->find( $reserve_id );
         if (!defined $hold){
             # may have already been checked out and hold fulfilled
+            require Koha::Old::Holds;
             $hold = Koha::Old::Holds->find( $reserve_id );
         }
         return unless $hold;
@@ -2118,6 +2113,8 @@ sub RevertWaitingStatus {
         }
     )->next;
 
+    my $original = C4::Context->preference('HoldsLog') ? $hold->unblessed : undef;
+
     ## Increment the priority of all other non-waiting
     ## reserves for this bib record
     my $holds = Koha::Holds->search({ biblionumber => $hold->biblionumber, priority => { '>' => 0 } })
@@ -2134,7 +2131,7 @@ sub RevertWaitingStatus {
         }
     )->store( { hold_reverted => 1 } );
 
-    logaction( 'HOLDS', 'MODIFY', $hold->id, $hold )
+    logaction( 'HOLDS', 'MODIFY', $hold->id, $hold, undef, $original )
         if C4::Context->preference('HoldsLog');
 
     _FixPriority( { biblionumber => $hold->biblionumber } );

C4/SIP/ILS.pm

@@ -132,12 +132,36 @@ sub checkout {
     # BEGIN TRANSACTION
     $circ->patron( $patron = C4::SIP::ILS::Patron->new($patron_id) );
     $circ->item( $item     = C4::SIP::ILS::Item->new($item_id) );
+
     if ($fee_ack) {
         $circ->fee_ack($fee_ack);
     }
+
     if ( !$patron ) {
         $circ->screen_msg("Invalid Patron");
     }
+    elsif ( !$item ) {
+        $circ->screen_msg("Invalid Item");
+    }
+    elsif ( $no_block_due_date ) {
+        # A no block due date means we need check this item out to the patron
+        # regardless if fines, restrictions or any other things that would
+        # typically prevent a patron from checking out.
+        # A no block transaction is used for send offline ( store and forward )
+        # transaction to Koha. The patron already has possession of the item
+        # so it should be checked out to the patron no matter what.
+        $circ->do_checkout( $account, $no_block_due_date );
+
+        $item->{borrowernumber} = $patron_id;
+        $item->{due_date}       = $circ->{due};
+        push( @{ $patron->{items} }, { barcode => $item_id } );
+        $circ->desensitize( !$item->magnetic_media );
+
+        siplog(
+            "LOG_DEBUG", "ILS::Checkout: patron %s has checked out %s via a no block checkout",
+            $patron_id,  join( ', ', map { $_->{barcode} } @{ $patron->{items} } )
+        );
+    }
     elsif ( !$patron->charge_ok ) {
         if ($patron->debarred) {
             $circ->screen_msg("Patron debarred");
@@ -169,7 +193,9 @@ sub checkout {
         $circ->screen_msg("Item type cannot be checked out at this checkout location");
     }
     else {
-        $circ->do_checkout($account, $no_block_due_date);
+        # No block checkouts were handled earlier so there is no need
+        # to bass the no block due date here.
+        $circ->do_checkout($account);
         if ( $circ->ok ) {
 
             # If the item is already associated with this patron, then
@@ -311,7 +337,7 @@ sub end_patron_session {
     my ($self, $patron_id) = @_;
 
     # success?, screen_msg, print_line
-    return (1, 'Thank you !', '');
+    return (1, 'Thank you!', '');
 }
 
 sub pay_fee {

C4/SIP/ILS/Patron.pm

@@ -121,6 +121,9 @@ sub new {
         }
     }
 
+    # Get currency 3 chars max
+    my $currency = substr Koha::Acquisition::Currencies->get_active->currency, 0, 3;
+
     my $circ_blocked =( C4::Context->preference('OverduesBlockCirc') ne "noblock" &&  defined $flags->{ODUES}->{itemlist} ) ? 1 : 0;
     {
     no warnings;    # any of these $kp->{fields} being concat'd could be undef
@@ -164,6 +167,7 @@ sub new {
         fine_blocked    => $fine_blocked,
         fee_limit       => $fee_limit,
         userid          => $kp->{userid},
+        currency        => $currency,
     );
     }
 
@@ -205,7 +209,7 @@ sub new {
         }
     }
 
-    # FIXME: populate fine_items recall_items
+    # FIXME: populate recall_items
     $ilspatron{unavail_holds} = _get_outstanding_holds($kp->{borrowernumber});
 
     my $pending_checkouts = $patron->pending_checkouts;
@@ -390,6 +394,7 @@ sub x_items {
 
     my $item_list = [];
     if ($self->{$array_var}) {
+
         if ($start && $start > 1) {
             --$start;
         }
@@ -397,6 +402,7 @@ sub x_items {
             $start = 0;
         }
         if ( $end && $end < @{$self->{$array_var}} ) {
+          --$end;
         }
         else {
             $end = @{$self->{$array_var}};
@@ -430,29 +436,33 @@ sub charged_items {
     return $self->x_items('items', @_);
 }
 sub fine_items {
+
     require Koha::Database;
     require Template;
 
-    my $self = shift;
-    my $start = shift;
-    my $end = shift;
+    my $self   = shift;
+    my $start  = shift;
+    my $end    = shift;
     my $server = shift;
 
-    my @fees = Koha::Database->new()->schema()->resultset('Accountline')->search(
+    my @fees =
+      Koha::Database->new()->schema()->resultset('Accountline')->search(
         {
             borrowernumber    => $self->{borrowernumber},
             amountoutstanding => { '>' => '0' },
         }
-    );
+      );
 
     $start = $start ? $start - 1 : 0;
     $end   = $end   ? $end - 1   : scalar @fees - 1;
 
-    my $av_field_template = $server ? $server->{account}->{av_field_template} : undef;
-    $av_field_template ||= "[% accountline.description %] [% accountline.amountoutstanding | format('%.2f') %]";
+    my $av_field_template =
+      $server ? $server->{account}->{av_field_template} : undef;
+    $av_field_template ||=
+"[% accountline.description %] [% accountline.amountoutstanding | format('%.2f') %]";
 
     my @return_values;
-    for ( my $i = $start; $i <= $end; $i++ ) {
+    for ( my $i = $start ; $i <= $end ; $i++ ) {
         my $fee = $fees[$i];
 
         next unless $fee;
@@ -462,7 +472,9 @@ sub fine_items {
     }
 
     return \@return_values;
+
 }
+
 sub recall_items {
     my $self = shift;
     return $self->x_items('recall_items', @_);

C4/SIP/SIPServer.pm

@@ -8,7 +8,7 @@ use FindBin qw($Bin);
 use lib "$Bin";
 use Net::Server::PreFork;
 use IO::Socket::INET;
-use Socket qw(:DEFAULT :crlf);
+use Socket qw(:DEFAULT :crlf TCP_KEEPIDLE TCP_KEEPINTVL IPPROTO_TCP);
 use Scalar::Util qw(blessed);
 require UNIVERSAL::require;
 
@@ -90,6 +90,126 @@ push @parms, 'group=' . $>;
 # This is the main event.
 __PACKAGE__ ->run(@parms);
 
+#
+# Server
+#
+
+=head3 options
+
+As per Net::Server documentation, override "options" to provide your own
+custom options to the Net::Server* object. This allows us to use the Net::Server
+infrastructure for configuration rather than hacking our own configuration into the
+object.
+
+=cut
+
+sub options {
+    my $self     = shift;
+    my $prop     = $self->{'server'};
+    my $template = shift;
+
+    # setup options in the parent classes
+    $self->SUPER::options($template);
+
+    $prop->{'custom_tcp_keepalive'} ||= undef;
+    $template->{'custom_tcp_keepalive'} = \$prop->{'custom_tcp_keepalive'};
+
+    $prop->{'custom_tcp_keepalive_time'} ||= undef;
+    $template->{'custom_tcp_keepalive_time'} = \$prop->{'custom_tcp_keepalive_time'};
+
+    $prop->{'custom_tcp_keepalive_intvl'} ||= undef;
+    $template->{'custom_tcp_keepalive_intvl'} = \$prop->{'custom_tcp_keepalive_intvl'};
+}
+
+=head3 post_configure_hook
+
+As per Net::Server documentation, this method validates our custom configuration.
+
+=cut
+
+sub post_configure_hook {
+    my $self = shift;
+    my $prop = $self->{'server'};
+    if ( defined $prop->{'custom_tcp_keepalive'} && $prop->{'custom_tcp_keepalive'} ) {
+
+        #NOTE: Any true value defined is forced to 1 just for the sake of predictability
+        $prop->{'custom_tcp_keepalive'} = 1;
+    }
+
+    foreach my $key ( 'custom_tcp_keepalive_time', 'custom_tcp_keepalive_intvl' ) {
+        my $value = $prop->{$key};
+
+        #NOTE: A regex is used here to detect a positive integer, as int() returns integers but does not validate them
+        #NOTE: We do not allow zero as it can lead to problematic behaviour
+        if ( $value && $value =~ /^\d+$/ ) {
+
+            #NOTE: Strictly, you must convert into an integer as a string will cause setsockopt to fail
+            $prop->{$key} = int($value);
+        }
+    }
+}
+
+=head3 post_accept_hook
+
+This hook occurs after the client connection socket is created, which gives
+us an opportunity to enable support for TCP keepalives using the SO_KEEPALIVE
+socket option.
+
+By default, the kernel-level defaults (in seconds) are used. You can view these in the output of "sysctl -a":
+net.ipv4.tcp_keepalive_intvl = 75
+net.ipv4.tcp_keepalive_time = 7200
+
+Alternatively, you can use "custom_tcp_keepalive_time" and "custom_tcp_keepalive_intvl" to define
+your own custom values for the socket. Note that these parameters are defined at the top server-level
+and not on the listener-level.
+
+If you lower "custom_tcp_keepalive_time" below 75, you will also need to set "custom_tcp_keepalive_intvl".
+The "tcp_keepalive_time" is the initial time used for the keepalive timer, and the "tcp_keepalive_intvl"
+is the time used for subsequent keepalive timers. However, timers only send keepalive ACKs if the idle time
+elapsed is greater than "tcp_keepalive_time".
+
+Thus, if "tcp_keepalive_time = 10" and "tcp_keepalive_intvl = 5", a keepalive ACK will be sent every 10 seconds
+of idle time. If "tcp_keepalive_intvl = 10" and "tcp_keepalive_time = 5", a keepalive ACK will be sent after 5
+seconds of idle time, and the next keepalive ACK will be sent after 10 seconds of idle time. Generally speaking,
+it's best to set "tcp_keepalive_time" to be higher than "tcp_keepalive_intvl".
+
+Reminder: once these settings are set on the socket, they are handled by the operating system kernel, and not
+by the SIP server application. If you are having trouble with your settings, monitor your TCP traffic using
+a tool such as "tcpdump" to review and refine how they work.
+
+=cut
+
+sub post_accept_hook {
+    my $self   = shift;
+    my $prop   = $self->{'server'};
+    my $client = shift || $prop->{'client'};
+
+    my $tcp_keepalive       = $prop->{custom_tcp_keepalive};
+    my $tcp_keepalive_time  = $prop->{custom_tcp_keepalive_time};
+    my $tcp_keepalive_intvl = $prop->{custom_tcp_keepalive_intvl};
+
+    if ($tcp_keepalive) {
+
+        #NOTE: set the SO_KEEPALIVE option to enable TCP keepalives
+        setsockopt( $client, SOL_SOCKET, SO_KEEPALIVE, 1 )
+            or die "Unable to set SO_KEEPALIVE: $!";
+
+        if ($tcp_keepalive_time) {
+
+            #NOTE: override "net.ipv4.tcp_keepalive_time" kernel parameter for this socket
+            setsockopt( $client, IPPROTO_TCP, TCP_KEEPIDLE, $tcp_keepalive_time )
+                or die "Unable to set TCP_KEEPIDLE: $!";
+        }
+
+        if ($tcp_keepalive_intvl) {
+
+            #NOTE: override "net.ipv4.tcp_keepalive_intvl" kernel parameter for this socket
+            setsockopt( $client, IPPROTO_TCP, TCP_KEEPINTVL, $tcp_keepalive_intvl )
+                or die "Unable to set TCP_KEEPINTVL: $!";
+        }
+    }
+}
+
 #
 # Child
 #

C4/SIP/Sip.pm

@@ -1,6 +1,9 @@
-#
-# Sip.pm: General Sip utility functions
-#
+
+=head1 NAME
+
+C4::SIP::Sip - General Sip utility functions
+
+=cut
 
 package C4::SIP::Sip;
 
@@ -187,34 +190,33 @@ sub boolspace {
 #
 
 sub write_msg {
-    my ($self, $msg, $file, $terminator, $encoding) = @_;
+    my ( $self, $msg, $server ) = @_;
+    my $terminator = $server->{account}->{terminator};
+    my $encoding   = $server->{account}->{encoding};
 
     $terminator ||= q{};
     $terminator = ( $terminator eq 'CR' ) ? $CR : $CRLF;
 
-    $msg = encode($encoding, $msg) if ( $encoding );
+    my $separator = $server->{account}->{convert_nonprinting_characters};
+    $msg =~ s/[^[:print:]]/$separator/g if defined $separator;
+
+    $msg = encode( $encoding, $msg ) if ($encoding);
 
     my $cksum;
 
     # $msg = encode_utf8($msg);
     if ($error_detection) {
-        if (defined($self->{seqno})) {
+        if ( defined( $self->{seqno} ) ) {
             $msg .= 'AY' . $self->{seqno};
         }
         $msg .= 'AZ';
         $cksum = checksum($msg);
-        $msg .= sprintf('%04.4X', $cksum);
+        $msg .= sprintf( '%04.4X', $cksum );
     }
 
-
-    if ($file) {
-        $file->autoflush(1);
-        print $file $msg, $terminator;
-    } else {
-        STDOUT->autoflush(1);
-        print $msg, $terminator;
-        siplog("LOG_INFO", "OUTPUT MSG: '$msg'");
-    }
+    STDOUT->autoflush(1);
+    print $msg, $terminator;
+    siplog( "LOG_INFO", "OUTPUT MSG: '$msg'" );
 
     $last_response = $msg;
 }
@@ -231,8 +233,53 @@ sub siplog {
 
     my $message = @args ? sprintf($mask, @args) : $mask;
 
+    $message = remove_password_from_message($message);
+
     my $logger = C4::SIP::Logger::get_logger();
     $logger->$method($message) if $logger;
 }
 
+=head2 remove_password_from_message
+
+  my $safe_message = remove_password_from_message( $message );
+
+Look for the fields AC, AD and CO, and replace contents with three asterisks.
+
+We do this by looking for:
+
+  delimiter + AC/AD/CO + something + delimiter
+
+and replacing it with:
+
+  delimiter + field name + asterisks + delimiter
+
+Messages and fields that can contain passwords:
+
+  23 Patron Status Request - AC terminal password - AD patron password
+  11 Checkout - AC terminal password - AD patron password
+  09 Checkin - AC terminal password
+  01 Block Patron - AC terminal password
+  93 Login - CO login password
+  63 Patron Information - AC terminal password - AD patron password
+  35 End Patron Session - AC terminal password - AD patron password
+  37 Fee Paid - AC terminal password - AD patron password
+  17 Item Information - AC terminal password
+  19 Item Status Update - AC terminal password
+  25 Patron Enable - AC terminal password- AD patron password
+  15 Hold - AC terminal password - AD patron password
+  29 Renew - AC terminal password - AD patron password
+  65 Renew All - AC terminal password - AD patron password
+
+=cut
+
+sub remove_password_from_message {
+    my ($message) = @_;
+
+    # regex does not work with (AC|AD|CO), probably due to .*? and the g modifier
+    # NOTE: Does not work correctly if the password contains a field delimiter, but you cannot login either.
+    $message =~ s/\Q${field_delimiter}\E$_.*?\Q${field_delimiter}\E/${field_delimiter}$_***${field_delimiter}/g
+        for ( 'AC', 'AD', 'CO' );
+    return $message;
+}
+
 1;

C4/SIP/Sip/Configuration.pm

@@ -39,8 +39,8 @@ sub new {
     my %listeners;
 
     # The key to the listeners hash is the 'port' component of the
-    # configuration, which is of the form '[host]:[port]/proto', and
-    # the 'proto' component could be upper-, lower-, or mixed-cased.
+    # configuration, which is of the form '[host]:[port]/proto[/IPv[46]]'
+    # The 'proto' component could be upper-, lower-, or mixed-cased.
     # Regularize it here to lower-case, and then do the same below in
     # find_server() when building the keys to search the hash.
 
@@ -76,6 +76,10 @@ sub find_service {
         siplog( "LOG_DEBUG",
             "Configuration::find_service: Trying $portstr" );
         last if ( exists( ( $self->{listeners} )->{$portstr} ) );
+        $portstr .= '/ipv4';    # lc, see ->new
+        last if ( exists( ( $self->{listeners} )->{$portstr} ) );
+        $portstr .= '/ipv6';    # lc, see ->new
+        last if ( exists( ( $self->{listeners} )->{$portstr} ) );
     }
     return $self->{listeners}->{$portstr};
 }

C4/SIP/Sip/MsgType.pm

@@ -504,12 +504,12 @@ sub handle_patron_status {
 
     $ils->check_inst_id( $fields->{ (FID_INST_ID) }, "handle_patron_status" );
     $patron = $ils->find_patron( $fields->{ (FID_PATRON_ID) } );
-    if ( C4::Context->preference('TrackLastPatronActivityTriggers') ) {
+    if ( C4::Context->preference('TrackLastPatronActivityTriggers') && $patron ) {
         my $koha_patron = Koha::Patrons->find($patron->{borrowernumber});
         $koha_patron->update_lastseen('connection');
     }
     $resp = build_patron_status( $patron, $lang, $fields, $server );
-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $resp, $server );
     return (PATRON_STATUS_REQ);
 }
 
@@ -649,7 +649,7 @@ sub handle_checkout {
         }
     }
 
-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $resp, $server );
     return (CHECKOUT);
 }
 
@@ -748,7 +748,7 @@ sub handle_checkin {
     $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server );
     $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server );
 
-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $resp, $server );
 
     return (CHECKIN);
 }
@@ -795,7 +795,7 @@ sub handle_block_patron {
     }
 
     $resp = build_patron_status( $patron, $patron->language, $fields, $server );
-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $resp, $server );
     return (BLOCK_PATRON);
 }
 
@@ -836,7 +836,7 @@ sub handle_request_acs_resend {
 
         # We haven't sent anything yet, so respond with a
         # REQUEST_SC_RESEND msg (p. 16)
-        $self->write_msg( REQUEST_SC_RESEND, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+        $self->write_msg( REQUEST_SC_RESEND, $server );
     } elsif ( ( length($last_response) < 9 )
         || substr( $last_response, -9, 2 ) ne 'AY' ) {
 
@@ -850,7 +850,7 @@ sub handle_request_acs_resend {
         # Cut out the sequence number and checksum, since the old
         # checksum is wrong for the resent message.
         my $rebuilt = substr( $last_response, 0, -9 );
-        $self->write_msg( $rebuilt, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+        $self->write_msg( $rebuilt, $server );
     }
 
     return REQUEST_ACS_RESEND;
@@ -932,7 +932,7 @@ sub handle_login {
         $status = login_core( $server, $uid, $pwd );
     }
 
-    $self->write_msg( LOGIN_RESP . $status, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( LOGIN_RESP . $status, $server );
     return $status ? LOGIN : '';
 }
 
@@ -971,7 +971,7 @@ sub summary_info {
     my $fid      = $summary_map[$summary_type]->{fid};
     my $itemlist = &$func( $patron, $start, $end, $server );
 
-    siplog( "LOG_DEBUG", "summary_info: list = (%s)", join( ", ", map{ $_->{barcode} } @{$itemlist} ) );
+    siplog( "LOG_DEBUG", "summary_info: list = (%s)", join( ", ", map { $_->{barcode} } @{$itemlist} ) );
     foreach my $i ( @{$itemlist} ) {
         $resp .= add_field( $fid, $i->{barcode}, $server );
     }
@@ -1101,7 +1101,7 @@ sub handle_patron_info {
         $resp .= maybe_add( FID_SCREEN_MSG, INVALID_CARD, $server );
     }
 
-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $resp, $server );
     return (PATRON_INFO);
 }
 
@@ -1128,7 +1128,7 @@ sub handle_end_patron_session {
     $resp .= maybe_add( FID_SCREEN_MSG, $screen_msg, $server );
     $resp .= maybe_add( FID_PRINT_LINE, $print_line, $server );
 
-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $resp, $server );
 
     return (END_PATRON_SESSION);
 }
@@ -1210,7 +1210,7 @@ sub handle_fee_paid {
     $resp .= maybe_add( FID_SCREEN_MSG,     $status->screen_msg, $server );
     $resp .= maybe_add( FID_PRINT_LINE,     $status->print_line, $server );
 
-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $resp, $server );
 
     return (FEE_PAID);
 }
@@ -1303,7 +1303,7 @@ sub handle_item_information {
         $resp .= $item->build_custom_field_string( $server );
     }
 
-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $resp, $server );
 
     return (ITEM_INFORMATION);
 }
@@ -1353,7 +1353,7 @@ sub handle_item_status_update {
     $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server );
     $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server );
 
-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $resp, $server );
 
     return (ITEM_STATUS_UPDATE);
 }
@@ -1407,7 +1407,7 @@ sub handle_patron_enable {
 
     $resp .= add_field( FID_INST_ID, $ils->institution, $server );
 
-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $resp, $server );
 
     return (PATRON_ENABLE);
 }
@@ -1472,7 +1472,7 @@ sub handle_hold {
     $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server );
     $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server );
 
-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $resp, $server );
 
     return (HOLD);
 }
@@ -1559,7 +1559,7 @@ sub handle_renew {
     $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server );
     $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server );
 
-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $resp, $server );
 
     return (RENEW);
 }
@@ -1612,7 +1612,7 @@ sub handle_renew_all {
     $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server );
     $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server );
 
-    $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $resp, $server );
 
     return (RENEW_ALL);
 }
@@ -1718,7 +1718,7 @@ sub send_acs_status {
 
     # Do we want to tell the terminal its location?
 
-    $self->write_msg( $msg, undef, $server->{account}->{terminator}, $server->{account}->{encoding} );
+    $self->write_msg( $msg, $server );
     return 1;
 }
 

C4/SIP/Trapper.pm

@@ -27,12 +27,23 @@ sub TIEHANDLE {
 
 sub PRINT {
     my $self = shift;
-    $Log::Log4perl::caller_depth++;
+    $Log::Log4perl::caller_depth += 3;
     my $logger =
       Koha::Logger->get( { interface => 'sip', category => 'STDERR' } );
     warn @_;
     $logger->warn(@_);
-    $Log::Log4perl::caller_depth--;
+    $Log::Log4perl::caller_depth -= 3;
+}
+
+=head2 OPEN
+
+    We need OPEN in case Net::Server tries to redirect STDERR. This will
+    be tried when param log_file or setsid is passed.
+
+=cut
+
+sub OPEN {
+    return 1;
 }
 
 =head2 BINMODE

C4/Serials.pm

@@ -1875,6 +1875,8 @@ sub GetLateOrMissingIssues {
     my ( $supplierid, $serialid, $order ) = @_;
 
     return unless ( $supplierid or $serialid );
+    return if defined $supplierid && !looks_like_number($supplierid);
+    return if defined $serialid   && !looks_like_number($serialid);
 
     my $dbh = C4::Context->dbh;
 
@@ -2023,7 +2025,7 @@ sub addroutingmember {
 
     my $rank;
     my $dbh = C4::Context->dbh;
-    my $sth = $dbh->prepare( "SELECT max(ranking) rank FROM subscriptionroutinglist WHERE subscriptionid = ?" );
+    my $sth = $dbh->prepare( "SELECT max(ranking) `rank` FROM subscriptionroutinglist WHERE subscriptionid = ?" );
     $sth->execute($subscriptionid);
     while ( my $line = $sth->fetchrow_hashref ) {
         if ( defined $line->{rank} && $line->{rank} > 0 ) {

C4/TTParser.pm

@@ -73,6 +73,26 @@ sub text{
     my $line = shift;
     my $work = shift; # original text
     my $is_cdata = shift;
+    # If there is a split template toolkit tag
+    if ( $work =~ m/^(?:(?!\[%).)*?%\]/s ) {
+        my @strings = ($&);
+        my $token;
+        $work = $';
+        while ( $token = pop @tokens ) {
+            if ( $token->string =~ m/\[%.*?$/s ) {
+                push @tokens,  C4::TmplToken->new( $`, $token->type, $token->line_number, $token->pathname );
+                push @strings, $&;
+                last;
+            } else {
+                push @strings, $token->string;
+            }
+        }
+        push @tokens,
+            C4::TmplToken->new(
+            join( '', reverse @strings ),         C4::TmplTokenType::DIRECTIVE,
+            $token ? $token->line_number : $line, $self->{filename}
+            );
+    }
     while($work){
         # if there is a template_toolkit tag
         if( $work =~ m/\[%.*?%\]/ ){

C4/Templates.pm

@@ -31,6 +31,7 @@ C4::Templates - Object for manipulating templates for use with Koha
 
 use base qw(Class::Accessor);
 use Template;
+use Template::Filters;
 use C4::Languages qw( get_bidi getTranslatedLanguages regex_lang_subtags );
 
 use C4::Context;
@@ -41,6 +42,7 @@ use Koha::Exceptions;
 __PACKAGE__->mk_accessors(qw( theme activethemes preferredtheme lang filename htdocs interface vars));
 
 
+Template::Filters->use_rfc3986();
 
 sub new {
     my $class     = shift;

C4/XSLT.pm

@@ -320,9 +320,9 @@ sub buildKohaItemsNamespace {
             $status = 'other';
             $substatus = 'In transit';
         }
-        elsif ($item->damaged) {
-            $status = 'other';
-            $substatus = "Damaged";
+        elsif ( $item->damaged && !C4::Context->preference('AllowHoldsOnDamagedItems') ) {
+            $status    = 'other';
+            $substatus = 'Damaged';
         }
         elsif ($item->itemlost) {
             $status = 'other';

Koha.pm

@@ -29,7 +29,7 @@ use vars qw{ $VERSION };
 # - #4 : the developer version. The 4th number is the database subversion.
 #        used by developers when the database changes. updatedatabase take care of the changes itself
 #        and is automatically called by Auth.pm when needed.
-$VERSION = "24.05.00.000";
+$VERSION = "24.05.16.000";
 
 sub version {
     return $VERSION;

Koha/Account/Line.pm

@@ -27,7 +27,6 @@ use Koha::Account::Offsets;
 use Koha::Database;
 use Koha::DateUtils qw( dt_from_string );
 use Koha::Exceptions::Account;
-use Koha::Items;
 use Koha::Patron::Debarments;
 
 use base qw(Koha::Object Koha::Object::Mixin::AdditionalFields);
@@ -80,6 +79,7 @@ sub item {
     my ( $self ) = @_;
     my $rs = $self->_result->itemnumber;
     return unless $rs;
+    require Koha::Item;
     return Koha::Item->_new_from_dbic( $rs );
 }
 
@@ -497,6 +497,10 @@ sub reduce {
         Koha::Exceptions::Account::IsNotDebit->throw(
             error => 'Account line ' . $self->id . 'is a payout' );
     }
+    if ( $self->debit_type_code eq 'VOID' ) {
+        Koha::Exceptions::Account::IsNotDebit->throw(
+            error => 'Account line ' . $self->id . 'is void' );
+    }
 
     # Check for mandatory parameters
     my @mandatory = ( 'interface', 'reduction_type', 'amount' );

Koha/AdditionalContent.pm

@@ -134,9 +134,10 @@ Return the translated content for a given language. The default is returned if n
 
 sub translated_content {
     my ( $self, $lang ) = @_;
+    $lang ||= 'default';
     my $content = $self->translated_contents->search(
-        { lang => [ 'default', ( $lang ? $lang : () ) ] },
-        { ( $lang ? ( order_by => \[ "field(lang, '" . $lang . "', 'default')" ] ) : () ) }
+        { lang     => [ 'default', $lang ] },
+        { order_by => { -asc => [ \'lang="default"', 'id' ] } }
     )->next;
     return $content;
 }

Koha/AdditionalContents.pm

@@ -81,12 +81,13 @@ sub search_for_display {
         qq|(SELECT COUNT(*) FROM additional_contents_localizations WHERE lang='$lang' AND additional_content_id=me.additional_content_id)=0|;
 
     my $search_params;
-    $search_params->{location}       = $params->{location};
-    $search_params->{branchcode}     = $params->{library_id} ? [ $params->{library_id}, undef ] : undef;
-    $search_params->{published_on}   = { '<=' => \'CAST(NOW() AS DATE)' };
-    $search_params->{expirationdate} = [ '-or', { '>=' => \'CAST(NOW() AS DATE)' }, undef ];
+    $search_params->{'additional_content.id'} = $params->{id} if $params->{id};
+    $search_params->{location}                = $params->{location};
+    $search_params->{branchcode}              = $params->{library_id} ? [ $params->{library_id}, undef ] : undef;
+    $search_params->{published_on}   = { '<=' => \'CAST(NOW() AS DATE)' }                   unless $params->{id};
+    $search_params->{expirationdate} = [ '-or', { '>=' => \'CAST(NOW() AS DATE)' }, undef ] unless $params->{id};
     $search_params->{category}       = $params->{category} if $params->{category};
-    $search_params->{lang}           = 'default' if !$lang || $lang eq 'default';
+    $search_params->{lang}           = 'default'           if !$lang || $lang eq 'default';
     $search_params->{-or}            = [ { 'lang' => $lang }, '-and' => [ 'lang', 'default', \$subquery ] ]
         if !$search_params->{lang};
 

Koha/App/Intranet.pm

@@ -38,6 +38,8 @@ sub startup {
     # FIXME This generates routes like this: /api/api/v1/...
     $self->plugin('RESTV1');
 
+    $self->plugin('CSRF');
+
     $self->hook(before_dispatch => \&_before_dispatch);
     $self->hook(around_action => \&_around_action);
 

Koha/App/Opac.pm

@@ -38,6 +38,8 @@ sub startup {
     # FIXME This generates routes like this: /api/api/v1/...
     $self->plugin('RESTV1');
 
+    $self->plugin('CSRF');
+
     $self->hook(before_dispatch => \&_before_dispatch);
     $self->hook(around_action => \&_around_action);
 

Koha/App/Plugin/CSRF.pm

@@ -0,0 +1,124 @@
+package Koha::App::Plugin::CSRF;
+
+# This file is part of Koha.
+#
+# Koha is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# Koha is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Koha; if not, see <http://www.gnu.org/licenses>.
+
+=head1 NAME
+
+Koha::App::Plugin::CSRF
+
+=head1 SYNOPSIS
+
+    $app->plugin('CSRF');
+
+=head1 DESCRIPTION
+
+Enables CSRF protection in a Mojolicious app
+
+=cut
+
+use Modern::Perl;
+
+use Mojo::Base 'Mojolicious::Plugin';
+
+use Mojo::Message::Response;
+
+use Koha::Token;
+
+=head1 METHODS
+
+=head2 register
+
+Called by Mojolicious when the plugin is loaded.
+
+Defines an `around_action` hook that will prevent CSRF attacks.
+
+If the HTTP request method is safe (GET, HEAD, OPTIONS, TRACE) and the request
+contains an `op` parameter whose value starts with "cud-" (which means it's a
+Create, Update or Delete operation), it will immediately return a 400 response.
+
+If the HTTP request method is unsafe (POST, PUT, DELETE, PATCH, CONNECT) and
+the request contains an `op` parameter whose value does not start with "cud-",
+it will immediately return a 400 response.
+
+If the HTTP request method is unsafe (POST, PUT, DELETE, PATCH, CONNECT) and
+the CGISESSID cookie is set, the CSRF token is checked. A 403 response is
+immediately returned if the CSRF token is missing or invalid.
+If the CGISESSID cookie is missing, it means that we are not authenticated or
+we are authenticated to the API by another method (HTTP basic or OAuth2). In
+this case, no verification is done.
+
+=cut
+
+# If you need to update the logic here, please also update Koha::Middleware::CSRF
+
+sub register {
+    my ( $self, $app, $conf ) = @_;
+
+    $app->hook(
+        around_action => sub {
+            my ( $next, $c, $action, $last ) = @_;
+
+            my $method = $c->req->method;
+            if ( $method eq 'GET' || $method eq 'HEAD' || $method eq 'OPTIONS' || $method eq 'TRACE' ) {
+                my $op = $c->req->param('op');
+                if ( $op && $op =~ /^cud-/ ) {
+                    return $c->reply->exception(
+                        'Incorrect use of a safe HTTP method with an `op` parameter that starts with "cud-"')
+                        ->rendered(400);
+                }
+            } else {
+                my $op = $c->req->param('op');
+                if ( $op && $op !~ /^cud-/ ) {
+                    return $c->reply->exception(
+                        'Incorrect use of an unsafe HTTP method with an `op` parameter that does not start with "cud-"')
+                        ->rendered(400);
+                }
+
+                if ( $c->cookie('CGISESSID') && !$self->is_csrf_valid( $c->req ) ) {
+                    return $c->reply->exception('Wrong CSRF token')->rendered(403);
+                }
+            }
+
+            return $next->();
+        }
+    );
+}
+
+=head2 is_csrf_valid
+
+Checks if a CSRF token exists and is valid
+
+    $is_valid = $plugin->is_csrf_valid($req)
+
+C<$req> must be a Mojo::Message::Request object
+
+=cut
+
+sub is_csrf_valid {
+    my ( $self, $req ) = @_;
+
+    my $csrf_token = $req->param('csrf_token') || $req->headers->header('CSRF-TOKEN');
+    my $cookie     = $req->cookie('CGISESSID');
+    if ( $csrf_token && $cookie ) {
+        my $session_id = $cookie->value;
+
+        return Koha::Token->new->check_csrf( { session_id => $session_id, token => $csrf_token } );
+    }
+
+    return 0;
+}
+
+1;

Koha/ArticleRequest.pm

@@ -267,6 +267,9 @@ sub store {
         $self->created_on( dt_from_string() );
     }
 
+    Koha::Exceptions::ArticleRequest::WrongFormat->throw
+        unless grep { $_ eq $self->format } @{ C4::Context->multivalue_preference('ArticleRequestsSupportedFormats') };
+
     return $self->SUPER::store;
 }
 

Koha/Auth/Identity/Provider.pm

@@ -62,7 +62,7 @@ sub get_config {
     my ($self) = @_;
 
     return try {
-        return decode_json( $self->config );
+        return decode_json( Encode::encode_utf8( $self->config ) );
     }
     catch {
         Koha::Exceptions::Object::BadValue->throw("Error reading JSON data: $_");
@@ -127,7 +127,7 @@ sub get_mapping {
     my ($self) = @_;
 
     return try {
-        return decode_json( $self->mapping );
+        return decode_json( Encode::encode_utf8( $self->mapping ) );
     }
     catch {
         Koha::Exceptions::Object::BadValue->throw("Error reading JSON data: $_");

Koha/AuthorisedValue.pm

@@ -132,6 +132,26 @@ sub is_integer_only {
     return $self->_result->category->is_integer_only;
 }
 
+=head3 to_api
+
+    my $json = $av->to_api;
+
+Overloaded method that returns a JSON representation of the Koha::AuthorisedValue object,
+suitable for API output.
+
+=cut
+
+sub to_api {
+    my ( $self, $params ) = @_;
+
+    my $response  = $self->SUPER::to_api($params);
+    my $overrides = {};
+
+    $overrides->{description} = $self->lib // q{};
+
+    return { %$response, %$overrides };
+}
+
 =head2 Internal methods
 
 =head3 _check_is_integer_only

Koha/BackgroundJob.pm

@@ -26,6 +26,7 @@ use C4::Context;
 use Koha::DateUtils qw( dt_from_string );
 use Koha::Exceptions;
 use Koha::Exceptions::BackgroundJob;
+use Koha::Plugins;
 
 use base qw( Koha::Object );
 
@@ -164,6 +165,12 @@ sub process {
 
     return {} if ref($self) ne 'Koha::BackgroundJob';
 
+    # Our background jobs are called in forked processes
+    # to ensure we have all plugin hooks and data we call
+    # get_enabled_plugins at the star of processing
+    # to populate the cache
+    Koha::Plugins->get_enabled_plugins();
+
     my $derived_class = $self->_derived_class;
 
     $args ||= {};
@@ -464,14 +471,17 @@ sub plugin_types_to_classes {
             my $namespace = $metadata->{namespace};
 
             foreach my $type ( keys %{$tasks} ) {
-                my $class = $tasks->{$type};
+                my $class =
+                    ( ref $tasks->{$type} eq 'HASH' )
+                    ? $tasks->{$type}{class}
+                    : $tasks->{$type};
 
                 # skip if conditions not met
                 next unless $type and $class;
 
                 my $key = "plugin_$namespace" . "_$type";
 
-                $self->{_plugin_mapping}->{$key} = $tasks->{$type};
+                $self->{_plugin_mapping}->{$key} = $class;
             }
         }
     }

Koha/BackgroundJob/ImportKBARTFile.pm

@@ -229,26 +229,29 @@ sub read_file {
     my $delimiter    = $file->{filename} =~ /\.tsv$/    ? "\t"                                   : ",";
     my $quote_char   = $file->{filename} =~ /\.tsv$/    ? "\""                                   : "\"";
 
-    open my $fh, "<", \$file_content or die;
+    open my $fh, "<", \$file_content or die "Could not open file $file->{filename}: $!";
     my $csv = Text::CSV_XS->new(
         {
             sep_char           => $delimiter,
             quote_char         => $quote_char,
             binary             => 1,
-            allow_loose_quotes => 1
+            allow_loose_quotes => 1,
+            formula            => 'empty'
         }
     );
     my $headers_to_check = $csv->getline($fh);
     my $column_headers   = rescue_EBSCO_files($headers_to_check);
     my $lines            = $csv->getline_all( $fh, 0 );
-
-    my ( $cde, $str, $pos ) = $csv->error_diag();
-    my $error = $cde ? "$cde, $str, $pos" : "";
-    warn $error if $error;
-
     close($fh);
 
-    return ( $column_headers, $lines, $error );
+    unless($csv->eof()) {
+        my ( $cde, $str, $pos ) = $csv->error_diag();
+        my $error = $cde ? "$cde, $str, $pos" : "";
+        warn $error if $error;
+        return ( $column_headers, $lines, $error );
+    }
+
+    return ( $column_headers, $lines );
 }
 
 =head3 create_title_hash_from_line_data

Koha/BackgroundJob/StageMARCForImport.pm

@@ -144,7 +144,7 @@ sub process {
                 $matcher_code    = $matcher->code();
                 $num_with_matches =
                   BatchFindDuplicates( $batch_id, $matcher, 10, 50,
-                    sub { my $job_progress = shift; $self->progress( $self->progress + $job_progress )->store } );
+                    sub { my $job_progress = shift; $self->progress( $job_progress )->store } );
                 SetImportBatchMatcher( $batch_id, $matcher_id );
                 SetImportBatchOverlayAction( $batch_id, $overlay_action );
                 SetImportBatchNoMatchAction( $batch_id, $nomatch_action );

Koha/Biblio.pm

@@ -52,6 +52,7 @@ use Koha::Item::Transfer::Limits;
 use Koha::Items;
 use Koha::Libraries;
 use Koha::Old::Checkouts;
+use Koha::Old::Holds;
 use Koha::Ratings;
 use Koha::Recalls;
 use Koha::RecordProcessor;
@@ -263,8 +264,13 @@ sub can_be_edited {
     Koha::Exceptions::MissingParameter->throw( error => "The patron parameter is missing or invalid" )
         unless $patron && ref($patron) eq 'Koha::Patron';
 
+    my $editcatalogue =
+        $self->frameworkcode eq 'FA'
+        ? [ 'fast_cataloging', 'edit_catalogue' ]
+        : 'edit_catalogue';
+
     return (
-        ( $self->metadata->source_allows_editing && $patron->has_permission( { editcatalogue => 'edit_catalogue' } ) )
+        ( $self->metadata->source_allows_editing && $patron->has_permission( { editcatalogue => $editcatalogue } ) )
             || $patron->has_permission( { editcatalogue => 'edit_locked_records' } ) ) ? 1 : 0;
 }
 
@@ -457,15 +463,14 @@ the I<OpacHiddenItems> system preference.
 sub hidden_in_opac {
     my ( $self, $params ) = @_;
 
+    return 0 unless C4::Context->preference('OpacHiddenItemsHidesRecord');
+
     my $rules = $params->{rules} // {};
 
     my @items = $self->items->as_list;
 
     return 0 unless @items; # Do not hide if there is no item
 
-    # Ok, there are items, don't even try the rules unless OpacHiddenItemsHidesRecord
-    return 0 unless C4::Context->preference('OpacHiddenItemsHidesRecord');
-
     return !(any { !$_->hidden_in_opac({ rules => $rules }) } @items);
 }
 
@@ -608,7 +613,8 @@ sub items {
 
     my $items_rs = $self->_result->items;
 
-    return Koha::Items->_new_from_dbic( $items_rs ) unless $params->{host_items};
+    return Koha::Items->_new_from_dbic($items_rs)
+        unless $params->{host_items} && C4::Context->preference('EasyAnalyticalRecords');
 
     my @itemnumbers = $items_rs->get_column('itemnumber')->all;
     my $host_itemnumbers = $self->_host_itemnumbers();
@@ -645,7 +651,7 @@ sub host_items {
 
     my $host_itemnumbers = $self->_host_itemnumbers;
 
-    return Koha::Items->search( { itemnumber => { -in => $host_itemnumbers } } );
+    return Koha::Items->search( { "me.itemnumber" => { -in => $host_itemnumbers } } );
 }
 
 =head3 _host_itemnumbers
@@ -702,6 +708,21 @@ sub holds {
     return Koha::Holds->_new_from_dbic($hold_rs);
 }
 
+=head3 old_holds
+
+my $old_holds = $biblio->old_holds();
+
+return the historic holds placed on this record
+
+=cut
+
+sub old_holds {
+    my ( $self, $params, $attributes ) = @_;
+    $attributes->{order_by} = 'priority' unless exists $attributes->{order_by};
+    my $old_hold_rs = $self->_result->old_reserves->search( $params, $attributes );
+    return Koha::Old::Holds->_new_from_dbic($old_hold_rs);
+}
+
 =head3 current_holds
 
 my $holds = $biblio->current_holds
@@ -1567,6 +1588,20 @@ sub normalized_upc {
     return $self->metadata_extractor->get_normalized_upc;
 }
 
+=head3 opac_suppressed
+
+    my $opac_suppressed = $biblio->opac_suppressed();
+
+Returns whether the record is flagged as suppressed in the OPAC.
+FIXME: Revisit after 38330 discussion
+
+=cut
+
+sub opac_suppressed {
+    my ($self) = @_;
+    return $self->metadata_extractor->get_opac_suppression();
+}
+
 =head3 normalized_oclc
 
     my $normalized_oclc = $biblio->normalized_oclc

Koha/Biblio/Metadata/Extractor/MARC.pm

@@ -27,9 +27,6 @@ use Modern::Perl;
 
 use Koha::Exceptions;
 
-use Koha::Biblio::Metadata::Extractor::MARC::MARC21;
-use Koha::Biblio::Metadata::Extractor::MARC::UNIMARC;
-
 =head1 API
 
 =head2 Class methods
@@ -58,6 +55,8 @@ sub new {
         unless $valid_schemas->{$schema};
 
     my $sub_class = "Koha::Biblio::Metadata::Extractor::MARC::$schema";
+    require "Koha/Biblio/Metadata/Extractor/MARC/$schema.pm";
+
     return $sub_class->new($params);
 }
 
@@ -77,6 +76,23 @@ sub metadata {
     return $self->{metadata};
 }
 
+=head2 get_opac_suppression
+
+    my $opac_suppressed = $extractor->get_opac_suppression();
+
+Returns whether the record is flagged as suppressed in the OPAC.
+FIXME: Revisit after 38330 discussion
+
+=cut
+
+sub get_opac_suppression {
+    my ($self) = @_;
+
+    my $record = $self->metadata;
+
+    return $record->subfield( '942', 'n' ) ? 1 : 0;
+}
+
 =head3 _normalize_string
 
     my $normalized_string = $self->_normalize_string($string);

Koha/Bookings.pm

@@ -32,17 +32,17 @@ Koha::Bookings - Koha Booking object set class
 
 =head2 Class Methods
 
-=head3 filter_by_future
+=head3 filter_by_active
 
-    $bookings->filter_by_future;
+    $bookings->filter_by_active;
 
-Will return the bookings starting from now.
+Will return the bookings that have not ended.
 
 =cut
 
-sub filter_by_future {
+sub filter_by_active {
     my ($self) = @_;
-    return $self->search( { start_date => { '>' => \'NOW()' } } );
+    return $self->search( { end_date => { '>=' => \'NOW()' } } );
 }
 
 =head2 Internal Methods

Koha/Checkout.pm

@@ -222,6 +222,21 @@ sub to_api_mapping {
     };
 }
 
+=head3 public_read_list
+
+This method returns the list of publicly readable database fields for both API and UI output purposes
+
+=cut
+
+sub public_read_list {
+    return [
+        'checkin_library', 'issue_id',        'borrowernumber',
+        'itemnumber',      'date_due',        'branchcode',
+        'returndate',      'lastreneweddate', 'issuedate',
+        'notedate',        'noteseen'
+    ];
+}
+
 =head3 claim_returned
 
   my $return_claim = $checkout->claim_returned();

Koha/Course.pm

@@ -17,14 +17,32 @@ package Koha::Course;
 
 use Modern::Perl;
 
-
 use base qw(Koha::Object);
 
 =head1 NAME
 
 Koha::Course - Koha Course Object class
 
-=head1 API
+=head2 Relations
+
+=head3 instructors
+
+  my $instructors = $course->instructors();
+
+Returns the related Koha::Patrons object containing the instructors for this course
+
+=cut
+
+sub instructors {
+    my ($self) = @_;
+
+    my $instructors = Koha::Patrons->search(
+        { 'course_instructors.course_id' => $self->course_id },
+        { join                           => 'course_instructors' }
+    );
+
+    return $instructors;
+}
 
 =head2 Internal methods
 

Koha/Database/Columns.pm

@@ -34,7 +34,8 @@ Koha::Database::Columns
 =head3 columns
 
 Static method that returns a hashref with mappings from column names
-to descriptions, for several tables. Currently
+to descriptions for several tables.
+Should be extended.
 
 =over
 
@@ -60,9 +61,17 @@ to descriptions, for several tables. Currently
 
 sub columns {
     return {
+        # TODO stub for the check in C4::Reports::Guided, get_columns (see BZ 37784)
+        accountlines            => {},
+        serial                  => {},
+        serialitems             => {},
+        subscriptionhistory     => {},
+        subscriptionroutinglist => {},
+        aqbooksellers           => {},
+
         aqorders => {
             "basketno"                      => __("Basket ID (internal)"),
-            "biblionumber"                  => __("Biblionumber (internal)"),
+            "biblionumber"                  => __("Biblio number (internal)"),
             "budget_id"                     => __("Fund ID (internal)"),
             "budgetdate"                    => __("Not used (deprecated)"),
             "cancellationreason"            => __("Cancellation reason"),
@@ -197,55 +206,59 @@ sub columns {
             "zipcode"                     => __("ZIP/Postal code"),
         },
         items => {
-            "biblioitemnumber"                  => __("Biblioitem number (internal)"),
             "barcode"                           => __("Barcode"),
-            "dateaccessioned"                   => __("Date acquired"),
+            "biblioitemnumber"                  => __("Biblio item number (internal)"),
+            "biblionumber"                      => __("Biblio number (internal)"),
+            "bookable"                          => __("Bookable"),
             "booksellerid"                      => __("Source of acquisition"),
-            "homebranch"                        => __("Permanent library"),
-            "price"                             => __("Price"),
-            "replacementprice"                  => __("Replacement price"),
-            "replacementpricedate"              => __("Price effective from"),
-            "datelastborrowed"                  => __("Date last checked out"),
-            "datelastseen"                      => __("Date last seen"),
-            "stack"                             => __("Shelving control number"),
-            "notforloan"                        => __("Not for loan"),
+            "ccode"                             => __("Collection"),
+            "cn_sort"                           => __("Koha normalized classification for sorting"),
+            "cn_source"                         => __("Source of classification or shelving scheme"),
+            "coded_location_qualifier"          => __("Coded location qualifier"),
+            "copynumber"                        => __("Copy number"),
             "damaged"                           => __("Damaged status"),
             "damaged_on"                        => __("Damaged on"),
+            "dateaccessioned"                   => __("Date acquired"),
+            "datelastborrowed"                  => __("Date last checked out"),
+            "datelastseen"                      => __("Date last seen"),
+            "deleted_on"                        => __("Deleted on"),
+            "enumchron"                         => __("Serial enumeraton/chronology"),
+            "exclude_from_local_holds_priority" => __("Exclude from local holds priority"),
+            "holdingbranch"                     => __("Current library"),
+            "homebranch"                        => __("Permanent library"),
+            "issues"                            => __("Total checkouts"),
+            "itemcallnumber"                    => __("Call number"),
             "itemlost"                          => __("Lost status"),
             "itemlost_on"                       => __("Lost on"),
-            "withdrawn"                         => __("Withdrawn status"),
-            "withdrawn_on"                      => __("Withdrawn on"),
-            "itemcallnumber"                    => __("Call number"),
-            "coded_location_qualifier"          => __("Coded location qualifier"),
-            "issues"                            => __("Total checkouts"),
-            "renewals"                          => __("Total renewals"),
-            "reserves"                          => __("Total holds"),
-            "restricted"                        => __("Use restrictions"),
             "itemnotes"                         => __("Public note"),
             "itemnotes_nonpublic"               => __("Internal note"),
-            "holdingbranch"                     => __("Current library"),
-            "timestamp"                         => __("Timestamp"),
-            "deleted_on"                        => __("Deleted on"),
+            "itemnumber"                        => __("Koha item number (autogenerated)"),
+            "itype"                             => __("Koha item type"),
+            "localuse"                          => __("Total local uses"),
             "location"                          => __("Shelving location"),
-            "permanent_location"                => __("Permanent shelving location"),
-            "onloan"                            => __("Due date"),
-            "cn_source"                         => __("Source of classification / shelving scheme"),
-            "cn_sort"                           => __("Koha normalized classification for sorting"),
-            "ccode"                             => __("Collection"),
             "materials"                         => __("Materials specified"),
-            "uri"                               => __("Uniform Resource Identifier"),
-            "itype"                             => __("Koha itemtype"),
             "more_subfields_xml"                => __("Additional subfields (XML)"),
-            "enumchron"                         => __("Serial enumeraton/chronology"),
-            "copynumber"                        => __("Copy number"),
-            "stocknumber"                       => __("Inventory number"),
             "new_status"                        => __("New status"),
-            "exclude_from_local_holds_priority" => __("Exclude from local holds priority"),
+            "notforloan"                        => __("Not for loan"),
+            "onloan"                            => __("Due date"),
+            "permanent_location"                => __("Permanent shelving location"),
+            "price"                             => __("Price"),
+            "renewals"                          => __("Total renewals"),
+            "replacementprice"                  => __("Replacement price"),
+            "replacementpricedate"              => __("Price effective from"),
+            "reserves"                          => __("Total holds"),
+            "restricted"                        => __("Use restrictions"),
+            "stack"                             => __("Shelving control number"),
+            "stocknumber"                       => __("Inventory number"),
+            "timestamp"                         => __("Modification date"),
+            "uri"                               => __("Uniform Resource Identifier"),
+            "withdrawn"                         => __("Withdrawn status"),
+            "withdrawn_on"                      => __("Withdrawn on"),
         },
         biblio => {
             "abstract"      => __("Abstract"),
             "author"        => __("Author"),
-            "biblionumber"  => __("Biblio number"),
+            "biblionumber"  => __("Biblio number (internal)"),
             "copyrightdate" => __("Copyright date"),
             "datecreated"   => __("Creation date"),
             "frameworkcode" => __("Framework code"),
@@ -261,44 +274,52 @@ sub columns {
             "unititle"      => __("Uniform title"),
         },
         biblioitems => {
-            "agerestriction"   => __("Age restriction"),
-            "biblioitemnumber" => __("Biblioitem number"),
-            "biblionumber"     => __("Biblio number"),
-            "classification"   => __("Classification"),
-            "dewey"            => __("Dewey/classification"),
-            "illus"            => __("Illustrations"),
-            "isbn"             => __("ISBN"),
-            "issn"             => __("ISSN"),
-            "itemtype"         => __("Biblio-level item type"),
-            "lccn"             => __("LCCN"),
-            "notes"            => __("Notes"),
-            "number"           => __("Number"),
-            "pages"            => __("Number of pages"),
-            "place"            => __("Place of publication"),
-            "publicationyear"  => __("Publication date"),
-            "publishercode"    => __("Publisher"),
-            "size"             => __("Size"),
-            "subclass"         => __("Sub classification"),
-            "timestamp"        => __("Timestamp"),
-            "title"            => __("Title"),
-            "url"              => __("URL"),
-            "volume"           => __("Volume number"),
-            "volumedate"       => __("Volume date"),
-            "volumedesc"       => __("Volume information"),
+            "agerestriction"        => __("Age restriction"),
+            "biblioitemnumber"      => __("Biblio item number (internal)"),
+            "biblionumber"          => __("Biblio number (internal)"),
+            "cn_class"              => __("Classification part"),
+            "cn_item"               => __("Item part"),
+            "cn_sort"               => __("Koha normalized classification for sorting"),
+            "cn_source"             => __("Source of classification or shelving scheme"),
+            "cn_suffix"             => __("Call number suffix"),
+            "collectiontitle"       => __("Series statement"),
+            "collectionissn"        => __("Series ISSN"),
+            "collectionvolume"      => __("Series volume"),
+            "ean"                   => __("EAN"),
+            "editionresponsibility" => __("Edition responsibility"),
+            "editionstatement"      => __("Edition statement"),
+            "illus"                 => __("Other physical details"),
+            "isbn"                  => __("ISBN"),
+            "issn"                  => __("ISSN"),
+            "itemtype"              => __("Biblio-level item type"),
+            "lccn"                  => __("LCCN"),
+            "notes"                 => __("Notes"),
+            "number"                => __("Number"),
+            "pages"                 => __("Number of pages"),
+            "place"                 => __("Place of publication"),
+            "publicationyear"       => __("Publication date"),
+            "publishercode"         => __("Publisher"),
+            "size"                  => __("Size"),
+            "timestamp"             => __("Modification date"),
+            "totalissues"           => __("Koha issues (borrowed), all copies"),
+            "url"                   => __("URL"),
+            "volume"                => __("Volume number"),
+            "volumedate"            => __("Volume date"),
+            "volumedesc"            => __("Volume information"),
         },
         statistics => {
-            "datetime"       => __("Statistics date and time"),
+            "borrowernumber" => __("Borrower number"),
             "branch"         => __("Library"),
-            "value"          => __("Value"),
-            "type"           => __("Transaction type"),
-            "other"          => __("SIP mode"),
+            "categorycode"   => __("Patron category"),
+            "ccode"          => __("Collection"),
+            "datetime"       => __("Statistics date and time"),
+            "interface"      => __("Interface"),
             "itemnumber"     => __("Item number"),
             "itemtype"       => __("Koha item type"),
             "location"       => __("Shelving location"),
-            "borrowernumber" => __("Borrower number"),
-            "ccode"          => __("Collection"),
-            "categorycode"   => __("Patron category"),
-            "interface"      => __("Interface"),
+            "other"          => __("SIP mode"),
+            "type"           => __("Transaction type"),
+            "value"          => __("Value"),
         },
         subscription => {
             "branchcode"  => __("Library"),
@@ -314,7 +335,7 @@ sub columns {
             "collectiontitle" => __("Collection title"),
             "copyrightdate"   => __("Copyright date"),
             "isbn"            => __("ISBN"),
-            "itemtype"        => __("Item type"),
+            "itemtype"        => __("Koha item type"),
             "note"            => __("Note"),
             "patronreason"    => __("Patron reason"),
             "place"           => __("Place of publication"),

Koha/EDI.pm

@@ -45,6 +45,7 @@ use Koha::Plugins; # Adds plugin dirs to @INC
 use Koha::Plugins::Handler;
 use Koha::Acquisition::Baskets;
 use Koha::Acquisition::Booksellers;
+use Koha::Util::FrameworkPlugin qw( biblio_008 );
 
 our $VERSION = 1.1;
 
@@ -684,6 +685,10 @@ sub quote_item {
     if ( !defined $bib ) {
         $bib = {};
         my $bib_record = _create_bib_from_quote( $item, $quote );
+
+        # Check for and add default 008 as this is a mandatory field
+        $bib_record = _handle_008_field($bib_record);
+
         ( $bib->{biblionumber}, $bib->{biblioitemnumber} ) =
           AddBiblio( $bib_record, q{} );
         $logger->trace("New biblio added $bib->{biblionumber}");
@@ -1215,6 +1220,17 @@ sub _create_item_from_quote {
     return $item_hash;
 }
 
+sub _handle_008_field {
+    my ($bib_record) = @_;
+
+    if ( !$bib_record->field('008') ) {
+        my $default008 = biblio_008();
+        $bib_record->insert_fields_ordered( MARC::Field->new( '008', $default008 ) );
+    }
+
+    return $bib_record;
+}
+
 1;
 __END__
 
@@ -1373,6 +1389,14 @@ Koha::EDI
      Returns the Aqbudget object for the active budget given the passed budget_code
      or undefined if one does not exist
 
+=head2 _handle_008_field
+
+     bib_record = _handle_008_field($bib_record)
+
+     Checks whether an 008 field exists on the record and adds a default field it does not
+
+     Returns the bib_record
+
 =head1 AUTHOR
 
    Colin Campbell <colin.campbell@ptfs-europe.com>

Koha/ERM/EUsage/CounterFile.pm

@@ -302,7 +302,7 @@ sub validate {
     my ($self) = @_;
 
     open my $fh, "<", \$self->file_content or die;
-    my $csv = Text::CSV_XS->new( { binary => 1, always_quote => 1, eol => $/, decode_utf8 => 1 } );
+    my $csv = Text::CSV_XS->new( { binary => 1, always_quote => 1, eol => $/, decode_utf8 => 1, formula => 'empty' } );
 
     $csv->column_names(qw( header_key header_value ));
     my @header_rows = $csv->getline_hr_all( $fh, 0, 12 );
@@ -329,7 +329,7 @@ sub _set_report_type_from_file {
     my ($self) = @_;
 
     open my $fh, "<", \$self->file_content or die;
-    my $csv = Text::CSV_XS->new( { binary => 1, always_quote => 1, eol => $/, decode_utf8 => 1 } );
+    my $csv = Text::CSV_XS->new( { binary => 1, always_quote => 1, eol => $/, decode_utf8 => 1, formula => 'empty' } );
 
     $csv->column_names(qw( header_key header_value ));
     my @header_rows = $csv->getline_hr_all( $fh, 0, 12 );
@@ -352,7 +352,7 @@ sub _get_rows_from_COUNTER_file {
     my ($self) = @_;
 
     open my $fh, "<", \$self->file_content or die;
-    my $csv = Text::CSV_XS->new( { binary => 1, always_quote => 1, eol => $/, decode_utf8 => 1 } );
+    my $csv = Text::CSV_XS->new( { binary => 1, always_quote => 1, eol => $/, decode_utf8 => 1, formula => 'empty' } );
 
     my $header_columns = $csv->getline_all( $fh, 13, 1 );
     $csv->column_names( @{$header_columns}[0] );

Koha/ERM/EUsage/SushiCounter.pm

@@ -86,7 +86,7 @@ sub _build_COUNTER_report_file {
     my @report = ( @{$header}, @{$column_headings}, @{$body} );
 
     #TODO: change this to tab instead of comma
-    csv( in => \@report, out => \my $counter_file, encoding => "utf-8" );
+    csv( in => \@report, out => \my $counter_file, encoding => "utf-8", formula => 'empty' );
 
     return $counter_file;
 

Koha/ERM/EUsage/UsageDataProvider.pm

@@ -27,6 +27,7 @@ use Koha::Exceptions;
 
 use base qw(Koha::Object);
 
+use Koha;
 use Koha::ERM::EUsage::CounterFile;
 use Koha::ERM::EUsage::CounterFiles;
 use Koha::ERM::EUsage::UsageTitles;
@@ -173,13 +174,17 @@ sub harvest_sushi {
     $self->{report_type} = $args->{report_type};
     $self->{begin_date}  = $args->{begin_date};
     $self->{end_date}    = $args->{end_date};
-    my $url      = $self->_build_url_query;
-    my $request  = HTTP::Request->new( 'GET' => $url );
-    my $ua       = LWP::UserAgent->new;
+    my $url     = $self->_build_url_query;
+    my $request = HTTP::Request->new( 'GET' => $url );
+    my $ua      = LWP::UserAgent->new;
+    $ua->agent( 'Koha/' . Koha::version() );
     my $response = $ua->simple_request($request);
 
+    my $result = decode_json( $response->decoded_content );
+
     if ( $response->code >= 400 ) {
-        my $result = decode_json( $response->decoded_content );
+
+        return if $self->_sushi_errors($result);
 
         my $message;
         if ( ref($result) eq 'ARRAY' ) {
@@ -197,7 +202,6 @@ sub harvest_sushi {
             }
         }
 
-        #TODO: May want to add a job error message here?
         warn sprintf "ERROR - SUSHI service %s returned %s - %s\n", $url,
             $response->code, $message;
         if ( $response->code == 404 ) {
@@ -206,7 +210,6 @@ sub harvest_sushi {
             Koha::Exceptions::Authorization::Unauthorized->throw($message);
         } else {
 
-            #TODO: May want to add a job error message here?
             die sprintf "ERROR requesting SUSHI service\n%s\ncode %s: %s\n",
                 $url, $response->code,
                 $message;
@@ -215,12 +218,10 @@ sub harvest_sushi {
         return;
     }
 
-    my $decoded_response = decode_json( $response->decoded_content );
-
-    return if $self->_sushi_errors($decoded_response);
+    return if $self->_sushi_errors($result);
 
     # Parse the SUSHI response
-    my $sushi_counter = Koha::ERM::EUsage::SushiCounter->new( { response => $decoded_response } );
+    my $sushi_counter = Koha::ERM::EUsage::SushiCounter->new( { response => $result } );
     my $counter_file  = $sushi_counter->get_COUNTER_from_SUSHI;
 
     return if $self->_counter_file_size_too_large($counter_file);
@@ -275,10 +276,11 @@ sub test_connection {
     $url .= 'status';
     $url .= '?customer_id=' . $self->customer_id;
     $url .= '&requestor_id=' . $self->requestor_id if $self->requestor_id;
-    $url .= '&api_key=' . $self->api_key if $self->api_key;
+    $url .= '&api_key=' . $self->api_key           if $self->api_key;
 
-    my $request  = HTTP::Request->new( 'GET' => $url );
-    my $ua       = LWP::UserAgent->new;
+    my $request = HTTP::Request->new( 'GET' => $url );
+    my $ua      = LWP::UserAgent->new;
+    $ua->agent( 'Koha/' . Koha::version() );
     my $response = $ua->simple_request($request);
 
     if ( $response->{_rc} >= 400 ) {
@@ -298,10 +300,10 @@ sub test_connection {
     my $status;
     if ( ref($result) eq 'ARRAY' ) {
         for my $r (@$result) {
-            $status = $r->{Service_Active};
+            $status = $r->{Service_Active} // $r->{ServiceActive};
         }
     } else {
-        $status = $result->{Service_Active};
+        $status = $result->{Service_Active} // $result->{ServiceActive};
     }
 
     if ($status) {
@@ -393,9 +395,10 @@ sub _build_url_query {
     $url .= lc $self->{report_type};
     $url .= '?customer_id=' . $self->customer_id;
     $url .= '&requestor_id=' . $self->requestor_id if $self->requestor_id;
-    $url .= '&api_key=' . $self->api_key if $self->api_key;
+    $url .= '&api_key=' . $self->api_key           if $self->api_key;
     $url .= '&begin_date=' . substr $self->{begin_date}, 0, 7 if $self->{begin_date};
-    $url .= '&end_date=' . substr $self->{end_date}, 0, 7 if $self->{end_date};
+    $url .= '&end_date=' . substr $self->{end_date},     0, 7 if $self->{end_date};
+    $url .= '&platform=' . $self->service_platform if $self->service_platform;
 
     return $url;
 }
@@ -456,19 +459,24 @@ Additionally, adds background job report message(s) if that is the case
 sub _sushi_errors {
     my ( $self, $decoded_response ) = @_;
 
-    if ( $decoded_response->{Severity} ) {
+    my $severity = $decoded_response->{Severity} // $decoded_response->{severity};
+    my $message  = $decoded_response->{Message}  // $decoded_response->{message};
+    my $code     = $decoded_response->{Code}     // $decoded_response->{code};
+
+    if ($severity) {
         $self->{job_callbacks}->{add_message_callback}->(
             {
                 type    => 'error',
-                code    => $decoded_response->{Code},
-                message => $decoded_response->{Severity} . ' - ' . $decoded_response->{Message},
+                code    => $code,
+                message => $severity . ' - ' . $message,
             }
         ) if $self->{job_callbacks};
         return 1;
     }
 
-    if ( $decoded_response->{Report_Header}->{Exceptions} ) {
-        foreach my $exception ( @{ $decoded_response->{Report_Header}->{Exceptions} } ) {
+    my $exceptions = $decoded_response->{Report_Header}->{Exceptions} // $decoded_response->{Exceptions};
+    if ($exceptions) {
+        foreach my $exception ( @{$exceptions} ) {
             $self->{job_callbacks}->{add_message_callback}->(
                 {
                     type    => 'error',
@@ -480,7 +488,7 @@ sub _sushi_errors {
         return 1;
     }
 
-    if ( scalar @{ $decoded_response->{Report_Items} } == 0 ) {
+    if ( $decoded_response->{Report_Items} && scalar @{ $decoded_response->{Report_Items} } == 0 ) {
         $self->{job_callbacks}->{add_message_callback}->(
             {
                 type => 'error',

Koha/ERM/Providers/EBSCO.pm

@@ -210,10 +210,10 @@ sub embed {
 
 sub build_query_pagination {
     my ( $self, $params ) = @_;
-    my $per_page = $params->{per_page}
+    my $per_page = $params->{_per_page}
       // C4::Context->preference('RESTdefaultPageSize') // 20;
     if ( $per_page == -1 || $per_page > 100 ) { $per_page = 100; }
-    my $page = $params->{page} || 1;
+    my $page = $params->{_page} || 1;
 
     return ( $per_page, $page );
 }

Koha/Exceptions/ArticleRequest.pm

@@ -27,6 +27,10 @@ use Exception::Class (
         isa         => 'Koha::Exceptions::ArticleRequest',
         description => 'Article request limit was reached'
     },
+    'Koha::Exceptions::ArticleRequest::WrongFormat' => {
+        isa         => 'Koha::Exceptions::ArticleRequest',
+        description => 'Passed format is not locally supported'
+    },
 );
 
 =head1 NAME

Koha/Exceptions/Object.pm

@@ -33,6 +33,11 @@ use Exception::Class (
         description => "Foreign key constraint broken",
         fields      =>  ['broken_fk', 'value'],
     },
+    'Koha::Exceptions::Object::FKConstraintDeletion' => {
+        isa         => 'Koha::Exceptions::Object',
+        description => "Foreign key constraint broken on deleting resource",
+        fields      => [ 'column', 'fk', 'table', 'constraint' ],
+    },
     'Koha::Exceptions::Object::MethodNotFound' => {
         isa => 'Koha::Exceptions::Object',
         description => "Invalid method",

Koha/Exceptions/REST.pm

@@ -0,0 +1,39 @@
+package Koha::Exceptions::REST;
+
+use Modern::Perl;
+
+use Koha::Exception;
+
+use Exception::Class (
+    'Koha::Exceptions::REST' => {
+        isa => 'Koha::Exception',
+    },
+    'Koha::Exceptions::REST::Public::Authentication::Required' => {
+        description => "This public route requires authentication",
+    },
+    'Koha::Exceptions::REST::Public::Unauthorized' => {
+        description => "Unprivileged user cannot access another user's resources",
+    },
+    'Koha::Exceptions::REST::Query::InvalidOperator' => {
+        description => "Invalid operator found in query",
+        fields      => ['operator']
+    },
+);
+
+=head1 NAME
+
+Koha::Exceptions::REST - Base class for REST API exceptions
+
+=head1 Exceptions
+
+=head2 Koha::Exceptions::REST
+
+Generic REST API exception.
+
+=head2 Koha::Exceptions::REST::Query::InvalidOperator
+
+The passed query is not allowed.
+
+=cut
+
+1;

Koha/FrameworkPlugin.pm

@@ -111,6 +111,7 @@ Koha::FrameworkPlugin - Facilitate use of plugins in MARC/items editor
 =cut
 
 use Modern::Perl;
+use Cwd qw//;
 
 use base qw(Class::Accessor);
 
@@ -213,7 +214,16 @@ sub _load {
     my ( $rv, $file );
     return $self->_error( 'Plugin needs a name' ) if !$self->{name}; #2chk
     $self->{path} //= _valuebuilderpath();
+    #NOTE: Resolve symlinks and relative path components if present,
+    #so the base will compare correctly lower down
+    my $abs_base_path = Cwd::abs_path( $self->{path} );
     $file= $self->{path}. '/'. $self->{name};
+    #NOTE: Resolve relative path components to prevent loading files outside the base path
+    my $abs_file_path = Cwd::abs_path($file);
+    if ( $abs_file_path !~ /^\Q$abs_base_path\E/ ) {
+        warn "Attempt to load $file ($abs_file_path) in framework plugin!";
+        return $self->_error('File not found');
+    }
     return $self->_error( 'File not found' ) if !-e $file;
 
     # undefine oldschool subroutines before defining them again

Koha/Hold.pm

@@ -34,7 +34,6 @@ use Koha::Biblios;
 use Koha::Hold::CancellationRequests;
 use Koha::Items;
 use Koha::Libraries;
-use Koha::Old::Holds;
 use Koha::Calendar;
 use Koha::Plugins;
 
@@ -1014,6 +1013,7 @@ Move a hold to the old_reserve table following the same pattern as Koha::Patron-
 sub _move_to_old {
     my ($self) = @_;
     my $hold_infos = $self->unblessed;
+    require Koha::Old::Hold;
     return Koha::Old::Hold->new( $hold_infos )->store;
 }
 
@@ -1026,25 +1026,26 @@ on the API.
 
 sub to_api_mapping {
     return {
-        reserve_id       => 'hold_id',
-        borrowernumber   => 'patron_id',
-        reservedate      => 'hold_date',
-        biblionumber     => 'biblio_id',
-        branchcode       => 'pickup_library_id',
-        notificationdate => undef,
-        reminderdate     => undef,
-        cancellationdate => 'cancellation_date',
-        reservenotes     => 'notes',
-        found            => 'status',
-        itemnumber       => 'item_id',
-        waitingdate      => 'waiting_date',
-        expirationdate   => 'expiration_date',
+        reserve_id             => 'hold_id',
+        borrowernumber         => 'patron_id',
+        reservedate            => 'hold_date',
+        biblionumber           => 'biblio_id',
+        deleted_biblionumber   => 'deleted_biblio_id',
+        branchcode             => 'pickup_library_id',
+        notificationdate       => undef,
+        reminderdate           => undef,
+        cancellationdate       => 'cancellation_date',
+        reservenotes           => 'notes',
+        found                  => 'status',
+        itemnumber             => 'item_id',
+        waitingdate            => 'waiting_date',
+        expirationdate         => 'expiration_date',
         patron_expiration_date => undef,
-        lowestPriority   => 'lowest_priority',
-        suspend          => 'suspended',
-        suspend_until    => 'suspended_until',
-        itemtype         => 'item_type',
-        item_level_hold  => 'item_level',
+        lowestPriority         => 'lowest_priority',
+        suspend                => 'suspended',
+        suspend_until          => 'suspended_until',
+        itemtype               => 'item_type',
+        item_level_hold        => 'item_level',
     };
 }
 

Koha/ILL/Backend.pm

@@ -89,9 +89,9 @@ sub existing_statuses {
         }
     );
     while ( my $request = $ill_requests->next ) {
-        my $status_data = $request->strings_map;
+        my $status_data = $request->status_alias ? $request->strings_map : undef;
 
-        if ( $status_data->{status_alias} ) {
+        if ( $status_data && $status_data->{status_alias} ) {
             push @data, {
                   $status_data->{status_alias}->{str}  ? ( str => $status_data->{status_alias}->{str} )
                 : $status_data->{status_alias}->{code} ? ( str => $status_data->{status_alias}->{code} )

Koha/ILL/Request.pm

@@ -184,7 +184,7 @@ sub statusalias {
     return Koha::AuthorisedValues->search(
         {
             category         => 'ILL_STATUS_ALIAS',
-            authorised_value => $self->SUPER::status_alias
+            authorised_value => $self->get_column('status_alias'),
         },
         {},
         $self->branchcode
@@ -299,7 +299,7 @@ correct category and records the fact that the status has changed
 sub status_alias {
     my ($self, $new_status_alias) = @_;
 
-    my $current_status_alias = $self->SUPER::status_alias;
+    my $current_status_alias = $self->get_column('status_alias');
 
     if ($new_status_alias) {
         # Keep a record of the previous status before we change it,
@@ -315,7 +315,7 @@ sub status_alias {
         # We need a way of accepting implied undef, so we can nullify
         # the status_alias column, when called from $self->status
         my $val = $new_status_alias eq "-1" ? undef : $new_status_alias;
-        my $ret = $self->SUPER::status_alias($val);
+        my $ret = $self->set( { status_alias => $val } );
         my $val_to_log = $val ? $new_status_alias : scalar $self->status;
         if ($ret) {
             my $logger = Koha::ILL::Request::Logger->new;
@@ -334,7 +334,7 @@ sub status_alias {
     my $alias = Koha::AuthorisedValues->search(
         {
             category         => 'ILL_STATUS_ALIAS',
-            authorised_value => $self->SUPER::status_alias
+            authorised_value => $current_status_alias
         },
         {},
         $self->branchcode
@@ -360,8 +360,8 @@ and sends a notice if appropriate
 sub status {
     my ( $self, $new_status) = @_;
 
-    my $current_status = $self->SUPER::status;
-    my $current_status_alias = $self->SUPER::status_alias;
+    my $current_status = $self->get_column('status');
+    my $current_status_alias = $self->get_column('status_alias');
 
     if ($new_status) {
         # Keep a record of the previous status before we change it,
@@ -369,7 +369,7 @@ sub status {
         $self->{previous_status} = $current_status_alias ?
             $current_status_alias :
             $current_status;
-        my $ret = $self->SUPER::status($new_status)->store;
+        my $ret = $self->set( { status => $new_status } )->store;
         if ($current_status_alias) {
             # This is hackery to enable us to undefine
             # status_alias, since we need to have an overloaded
@@ -401,20 +401,23 @@ sub status {
 
     my $backend_plugin = $self->get_backend_plugin($backend_name);
 
-Returns the installed I<Koha::Plugin> corresponding to the given backend_id
+Returns the installed I<Koha::Plugin> corresponding to the given backend_id or undef if no plugin is found
 
 =cut
 
 sub get_backend_plugin {
     my ( $self, $backend_id ) = @_;
 
-    my @backend_plugins = Koha::Plugins->new()->GetPlugins(
+    my $koha_plugins    = Koha::Plugins->new();
+    my @backend_plugins = $koha_plugins
+        ? Koha::Plugins->new()->GetPlugins(
         {
             method   => 'ill_backend',
             metadata => { name => $backend_id },
             all      => 1,
         }
-    );
+        )
+        : ();
 
     return $backend_plugins[0];
 }
@@ -1174,7 +1177,7 @@ sub get_type {
     my $attr = $self->extended_attributes->find({ type => 'type'});
     return if !$attr;
     return $attr->value;
-};
+}
 
 =head3 get_type_disclaimer_value
 
@@ -2073,21 +2076,25 @@ sub strings_map {
     }
 
     my $status_string =
-      ( exists $status_graph_union->{ $self->status } && defined $status_graph_union->{ $self->status }->{name} )
-      ? $status_graph_union->{ $self->status }->{name}
-      : $self->status;
+        (      $self->status
+            && exists $status_graph_union->{ $self->status }
+            && defined $status_graph_union->{ $self->status }->{name} )
+        ? $status_graph_union->{ $self->status }->{name}
+        : $self->status;
 
     my $status_code =
-      ( exists $status_graph_union->{ $self->status } && defined $status_graph_union->{ $self->status }->{id} )
-      ? $status_graph_union->{ $self->status }->{id}
-      : $self->status;
+        (      $self->status
+            && exists $status_graph_union->{ $self->status }
+            && defined $status_graph_union->{ $self->status }->{id} )
+        ? $status_graph_union->{ $self->status }->{id}
+        : $self->status;
 
     my $strings = {
         status => {
-            backend => $self->backend, # the backend identifier
-            str     => $status_string, # the status description, taken from the status graph
-            code    => $status_code,   # the status id, taken from the status graph
-            type    => 'ill_status',   # fixed type
+            backend => $self->backend,    # the backend identifier
+            str     => $status_string,    # the status description, taken from the status graph
+            code    => $status_code,      # the status id, taken from the status graph
+            type    => 'ill_status',      # fixed type
         }
     };
 

Koha/ILL/Request/Config.pm

@@ -113,14 +113,17 @@ Returns a list of names for all the installed ILL backend plugins.
 =cut
 
 sub get_backend_plugin_names {
-    my ( $self ) = @_;
+    my ($self) = @_;
 
-    my @backend_plugins = Koha::Plugins->new()->GetPlugins(
+    my $koha_plugins    = Koha::Plugins->new();
+    my @backend_plugins = $koha_plugins
+        ? $koha_plugins->GetPlugins(
         {
             method => 'ill_backend',
             all    => 1
         }
-    );
+        )
+        : ();
 
     return map { $_->{metadata}->{name} } @backend_plugins;
 }

Koha/ILL/Requests.pm

@@ -23,7 +23,7 @@ use Koha::Database;
 use Koha::ILL::Request;
 use Koha::ILL::Request::Config;
 
-use base qw(Koha::Objects);
+use base qw(Koha::Objects::Mixin::ExtendedAttributes Koha::Objects);
 
 =head1 NAME
 
@@ -106,6 +106,19 @@ sub search_incomplete {
     } );
 }
 
+=head3 extended_attributes_config
+
+=cut
+
+sub extended_attributes_config {
+    my ($self) = @_;
+    return {
+        'id_field'     => { 'foreign' => 'illrequest_id', 'self' => 'illrequest_id' },
+        'key_field'    => 'type',
+        'schema_class' => 'Koha::Schema::Result::Illrequestattribute',
+    };
+}
+
 =head2 Internal methods
 
 =head3 _type

Koha/Import/Record.pm

@@ -56,7 +56,7 @@ sub get_marc_record {
         $format = 'UNIMARCAUTH';
     }
 
-    my $record = MARC::Record->new_from_xml($self->marcxml, $self->encoding, $format);
+    my $record = MARC::Record->new_from_xml( $self->marcxml, 'UTF-8', $format );
 
     return $record;
 }

Koha/Item.pm

@@ -98,6 +98,9 @@ sub store {
         $self->itype($self->biblio->biblioitem->itemtype);
     }
 
+    # Ensure barcode is either defined or undef
+    $self->barcode(undef) if ( defined($self->barcode) && $self->barcode eq '' );
+
     $self->barcode( C4::Circulation::barcodedecode( $self->barcode ) );
 
     my $today  = dt_from_string;
@@ -917,7 +920,7 @@ sub hidden_in_opac {
 
     foreach my $field ( keys %{$rules} ) {
 
-        if ( any { $self->$field eq $_ } @{ $rules->{$field} } ) {
+        if ( any { defined $self->$field && $self->$field eq $_ } @{ $rules->{$field} } ) {
             $hidden_in_opac = 1;
             last;
         }
@@ -994,23 +997,29 @@ sub pickup_locations {
     my $patron = $params->{patron};
 
     my $circ_control_branch = Koha::Policy::Holds->holds_control_library( $self, $patron );
-    my $branchitemrule =
-      C4::Circulation::GetBranchItemRule( $circ_control_branch, $self->itype );
+    my $branchitemrule      = C4::Circulation::GetBranchItemRule( $circ_control_branch, $self->itype );
 
-    return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} eq 'from_local_hold_group' && !$self->home_branch->validate_hold_sibling( {branchcode => $patron->branchcode} );
-    return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} eq 'from_home_library' && $self->home_branch->branchcode ne $patron->branchcode;
+    if ( $branchitemrule->{holdallowed} eq 'from_local_hold_group'
+        && !$self->home_branch->validate_hold_sibling( { branchcode => $patron->branchcode } )
+        || $branchitemrule->{holdallowed} eq 'from_home_library'
+        && $self->home_branch->branchcode ne $patron->branchcode )
+    {
+        return Koha::Libraries->new()->empty;
+    }
 
-    my $pickup_libraries = Koha::Libraries->search();
-    if ($branchitemrule->{hold_fulfillment_policy} eq 'holdgroup') {
+    my $pickup_libraries;
+    if ( $branchitemrule->{hold_fulfillment_policy} eq 'holdgroup' ) {
         $pickup_libraries = $self->home_branch->get_hold_libraries;
-    } elsif ($branchitemrule->{hold_fulfillment_policy} eq 'patrongroup') {
-        my $plib = Koha::Libraries->find({ branchcode => $patron->branchcode});
+    } elsif ( $branchitemrule->{hold_fulfillment_policy} eq 'patrongroup' ) {
+        my $plib = Koha::Libraries->find( { branchcode => $patron->branchcode } );
         $pickup_libraries = $plib->get_hold_libraries;
-    } elsif ($branchitemrule->{hold_fulfillment_policy} eq 'homebranch') {
-        $pickup_libraries = Koha::Libraries->search({ branchcode => $self->homebranch });
-    } elsif ($branchitemrule->{hold_fulfillment_policy} eq 'holdingbranch') {
-        $pickup_libraries = Koha::Libraries->search({ branchcode => $self->holdingbranch });
-    };
+    } elsif ( $branchitemrule->{hold_fulfillment_policy} eq 'homebranch' ) {
+        $pickup_libraries = Koha::Libraries->search( { branchcode => $self->homebranch } );
+    } elsif ( $branchitemrule->{hold_fulfillment_policy} eq 'holdingbranch' ) {
+        $pickup_libraries = Koha::Libraries->search( { branchcode => $self->holdingbranch } );
+    } else {
+        $pickup_libraries = Koha::Libraries->search();
+    }
 
     return $pickup_libraries->search(
         {
@@ -1022,7 +1031,8 @@ sub pickup_locations {
     ) unless C4::Context->preference('UseBranchTransferLimits');
 
     my $limittype = C4::Context->preference('BranchTransferLimitsType');
-    my ($ccode, $itype) = (undef, undef);
+    my $ccode;
+    my $itype;
     if( $limittype eq 'ccode' ){
         $ccode = $self->ccode;
     } else {
@@ -1734,9 +1744,7 @@ sub to_api {
 
     $overrides->{effective_item_type_id} = $self->effective_itemtype;
 
-    my $itype_notforloan = $self->itemtype->notforloan;
-    $overrides->{effective_not_for_loan_status} =
-        ( defined $itype_notforloan && !$self->notforloan ) ? $itype_notforloan : $self->notforloan;
+    $overrides->{effective_not_for_loan_status} = $self->effective_not_for_loan_status;
 
     return { %$response, %$overrides };
 }
@@ -1832,6 +1840,21 @@ sub item_type {
     return shift->itemtype;
 }
 
+=head3 effective_not_for_loan_status
+
+  my $nfl = $item->effective_not_for_loan_status;
+
+Returns the effective not for loan status of the item
+
+=cut
+
+sub effective_not_for_loan_status {
+    my ($self) = @_;
+    my $itemtype = $self->itemtype;
+    my $itype_notforloan = $itemtype ? $itemtype->notforloan : undef;
+    return ( defined($itype_notforloan) && !$self->notforloan ) ? $itype_notforloan : $self->notforloan;
+}
+
 =head3 orders
 
   my $orders = $item->orders();
@@ -2382,34 +2405,6 @@ sub check_recalls {
     return $recall;
 }
 
-=head3 is_notforloan
-
-    my $is_notforloan = $item->is_notforloan;
-
-Determine whether or not this item is "notforloan" based on
-the item's notforloan status or its item type
-
-=cut
-
-sub is_notforloan {
-    my ( $self ) = @_;
-    my $is_notforloan = 0;
-
-    if ( $self->notforloan ){
-        $is_notforloan = 1;
-    }
-    else {
-        my $itemtype = $self->itemtype;
-        if ($itemtype){
-            if ( $itemtype->notforloan ){
-                $is_notforloan = 1;
-            }
-        }
-    }
-
-    return $is_notforloan;
-}
-
 =head3 is_denied_renewal
 
     my $is_denied_renewal = $item->is_denied_renewal;

Koha/Items.pm

@@ -51,7 +51,10 @@ Koha::Items - Koha Item object set class
 
     my $filtered_items = $items->filter_by_for_hold;
 
-Return the items of the set that are *potentially* holdable.
+Return the items of the set that are *potentially* holdable. This routine
+checks only rules defined in the 'Standard rules for all libraries' and
+should only be used in the context where we don't have a patron to check
+policies for specifically.
 
 Caller has the responsibility to call C4::Reserves::CanItemBeReserved before
 placing a hold on one of those items.
@@ -61,15 +64,40 @@ placing a hold on one of those items.
 sub filter_by_for_hold {
     my ($self) = @_;
 
-    my @hold_not_allowed_itypes = Koha::CirculationRules->search(
+    my $default_rule = Koha::CirculationRules->get_effective_rule(
         {
             rule_name    => 'holdallowed',
-            branchcode   => undef,
-            categorycode => undef,
-            rule_value   => 'not_allowed',
         }
-    )->get_column('itemtype');
+    );
+    my @hold_not_allowed_itypes;
+    if ( defined $default_rule && $default_rule->rule_value eq 'not_allowed' ) {
+        # If the default rule is not allowed we get all itemtypes as not allowed
+        @hold_not_allowed_itypes = Koha::ItemTypes->search->get_column('itemtype');
+        my @hold_allowed_itypes = Koha::CirculationRules->search(
+            {
+                rule_name    => 'holdallowed',
+                rule_value   => { '!=' => 'not_allowed' },
+                branchcode   => undef,
+                categorycode => undef,
+            }
+        )->get_column('itemtype');
+        # We then only allow those explicitly defined in hold policies at the all libraries level
+        @hold_not_allowed_itypes = array_minus( @hold_not_allowed_itypes, @hold_allowed_itypes );
+    } else {
+        # If there is no default 'not_allowed' rule, then only those explicitly forbidden at the all libraries level are forbidden
+        @hold_not_allowed_itypes = Koha::CirculationRules->search(
+            {
+                rule_name    => 'holdallowed',
+                branchcode   => undef,
+                categorycode => undef,
+                rule_value   => 'not_allowed',
+            }
+        )->get_column('itemtype');
+    }
+
+    # We also forbid holds on any marked not for loan at the item level
     push @hold_not_allowed_itypes, Koha::ItemTypes->search({ notforloan => 1 })->get_column('itemtype');
+    @hold_not_allowed_itypes = uniq @hold_not_allowed_itypes;
 
     my $params = {
         itemlost   => 0,

Koha/Middleware/CSRF.pm

@@ -20,6 +20,8 @@ use Modern::Perl;
 use parent qw(Plack::Middleware);
 use Plack::Response;
 
+# If you need to update the logic here, please also update Koha::App::Plugin::CSRF
+
 sub call {
     my ( $self, $env ) = @_;
     my $req = Plack::Request->new($env);
@@ -42,6 +44,20 @@ sub call {
     my $request_method = $req->method  // q{};
     my $uri            = $req->uri     // q{};
     my $referer        = $req->referer // q{No referer};
+    my $request_path   = $req->path    // q{};
+
+    my %path_exceptions = (
+        '/ilsdi.pl' => 1,
+    );
+
+    #NOTE: Ignore ErrorDocument requests for CSRF
+    if ( $env->{'psgix.errordocument.SCRIPT_NAME'} ) {
+        return $self->app->($env);
+    } elsif ( $path_exceptions{$request_path} ) {
+
+        #NOTE: Ignore path exceptions for CSRF
+        return $self->app->($env);
+    }
 
     my ($error);
     if ( $stateless_methods{$request_method} && defined $original_op && $original_op =~ m{^cud-} ) {
@@ -71,16 +87,15 @@ sub call {
                 $error = "wrong_csrf_token";
             }
         }
+    } elsif ( !exists $stateless_methods{$request_method} && !exists $stateful_methods{$request_method} ) {
+        $error = sprintf "unknown or unsupported method %s", $request_method;
     }
 
-    #NOTE: It is essential to check for this environmental variable.
-    #NOTE: If we don't check for it, then we'll also throw an error for the "subrequest" that ErrorDocument uses to
-    #fetch the error page document. Then we'll wind up with a very ugly error page and not our pretty one.
-    if ( $error && !$env->{'plack.middleware.Koha.CSRF'} ) {
+    if ($error) {
 
         #NOTE: Other Middleware will take care of logging to correct place, as Koha::Logger doesn't know where to go here
         warn $error;
-        $env->{'plack.middleware.Koha.CSRF'} = $error;
+        $env->{'plack.middleware.Koha.CSRF'} = $error if !$env->{'plack.middleware.Koha.CSRF'};
         my $res = Plack::Response->new( 403, [ 'Content-Type' => 'text/plain' ], ["Wrong CSRF token"] );
         return $res->finalize;
     }

Koha/Number/Price.pm

@@ -19,6 +19,7 @@ package Koha::Number::Price;
 
 use Modern::Perl;
 
+use Math::BigFloat;
 use Number::Format;
 use C4::Context;
 use Koha::Acquisition::Currencies;
@@ -83,12 +84,7 @@ sub round {
 
     my $format_params = $self->_format_params;
 
-    # To avoid the system to crash, we will not format big number
-    # We divide per 100 because we want to keep the default DECIMAL_DIGITS (2)
-    # error - round() overflow. Try smaller precision or use Math::BigFloat
-    return $self->value if $self->value > Number::Format::MAX_INT/100;
-
-    return Number::Format->new(%$format_params)->round($self->value);
+    return Number::Format->new(%$format_params)->round( Math::BigFloat->new( $self->value ) )->bdstr();
 }
 
 sub _format_params {

Koha/Object.pm

@@ -233,11 +233,32 @@ Returns:
 sub delete {
     my ($self) = @_;
 
-    my $deleted = $self->_result()->delete;
+    my $deleted;
+
+    try {
+        $deleted = $self->_result()->delete;
+    } catch {
+        if ( ref($_) eq 'DBIx::Class::Exception' ) {
+            if ( $_->{msg} =~
+                /Cannot delete or update a parent row\: a foreign key constraint fails \(\`(?<database>.*?)\`\.\`(?<table>.*?)\`, CONSTRAINT \`(?<constraint>.*?)\` FOREIGN KEY \(\`(?<fk>.*?)\`\) REFERENCES \`.*\` \(\`(?<column>.*?)\`\)/
+                )
+            {
+                Koha::Exceptions::Object::FKConstraintDeletion->throw(
+                    column     => $+{column},
+                    constraint => $+{constraint},
+                    fk         => $+{fk},
+                    table      => $+{table},
+                );
+            }
+            $_->rethrow();
+        }
+    };
+
     if ( ref $deleted ) {
-        my $object_class  = Koha::Object::_get_object_class( $self->_result->result_class );
+        my $object_class = Koha::Object::_get_object_class( $self->_result->result_class );
         $deleted = $object_class->_new_from_dbic($deleted);
     }
+
     return $deleted;
 }
 

Koha/Object/Limit/Library.pm

@@ -19,7 +19,6 @@ use Modern::Perl;
 
 use Koha::Database;
 use Koha::Exceptions;
-use Koha::Libraries;
 
 use Try::Tiny qw( catch try );
 
@@ -87,6 +86,8 @@ sub get_library_limits {
 
     return unless @branchcodes;
 
+    require Koha::Libraries;
+
     my $filter = [ map { { branchcode => $_ } } @branchcodes ];
     my $libraries = Koha::Libraries->search( $filter );
 

Koha/Objects.pm

@@ -167,17 +167,19 @@ sub search_related {
 =cut
 
 sub delete {
-    my ($self) = @_;
+    my ( $self, $params ) = @_;
 
     if ( Class::Inspector->function_exists( $self->object_class, 'delete' ) ) {
         my $objects_deleted;
-        $self->_resultset->result_source->schema->txn_do( sub {
-            $self->reset; # If we iterated already over the set
-            while ( my $o = $self->next ) {
-                $o->delete;
-                $objects_deleted++;
+        $self->_resultset->result_source->schema->txn_do(
+            sub {
+                $self->reset;    # If we iterated already over the set
+                while ( my $o = $self->next ) {
+                    $o->delete($params);
+                    $objects_deleted++;
+                }
             }
-        });
+        );
         return $objects_deleted;
     }
 

Koha/Objects/Mixin/ExtendedAttributes.pm

@@ -0,0 +1,234 @@
+package Koha::Objects::Mixin::ExtendedAttributes;
+
+# Copyright 2024 PTFS Europe Ltd
+#
+# This file is part of Koha.
+#
+# Koha is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# Koha is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Koha; if not, see <http://www.gnu.org/licenses>.
+
+use Modern::Perl;
+use Scalar::Util qw( reftype );
+
+=head1 NAME
+
+Koha::Objects::Mixin::ExtendedAttributes
+
+=head2 Class methods
+
+
+=head3 search
+
+    Overwrites the search method to include extended attributes rewrite and dynamic relation accessors
+
+=cut
+
+sub search {
+    my ( $self, $params, $attributes ) = @_;
+
+    my $class = ref($self) ? ref($self) : $self;
+
+    $self->handle_query_extended_attributes(
+        {
+            attributes      => $attributes,
+            filtered_params => $params,
+        }
+    );
+
+    my $rs = $self->_resultset()->search( $params, $attributes );
+    return $class->_new_from_dbic($rs);
+}
+
+=head3 handle_query_extended_attributes
+
+    Checks for the presence of extended_attributes in a query
+    If present, it builds the dynamic extended attributes relations and rewrites the query to include the extended_attributes relation
+
+=cut
+
+sub handle_query_extended_attributes {
+    my ( $self, $args ) = @_;
+
+    my $attributes      = $args->{attributes};
+    my $filtered_params = $args->{filtered_params};
+
+    if (   reftype( $attributes->{prefetch} )
+        && reftype( $attributes->{prefetch} ) eq 'ARRAY'
+        && grep ( /extended_attributes/, @{ $attributes->{prefetch} } ) )
+    {
+
+        my @array = $self->_get_extended_attributes_entries($filtered_params);
+
+        # Calling our private method to build the extended attributes relations
+        my @joins = $self->_build_extended_attributes_relations( \@array );
+        push @{ $attributes->{join} }, @joins;
+
+    }
+}
+
+=head3 _get_extended_attributes_entries
+
+    $self->_get_extended_attributes_entries( $filtered_params, 0 )
+
+Recursive function that returns the rewritten extended_attributes query entries.
+
+Given:
+[
+    '-and',
+    [
+        {
+            'extended_attributes.code'      => 'CODE_1',
+            'extended_attributes.attribute' => { 'like' => '%Bar%' }
+        },
+        {
+            'extended_attributes.attribute' => { 'like' => '%Bar%' },
+            'extended_attributes.code'      => 'CODE_2'
+        }
+    ]
+];
+
+Returns :
+
+[
+    'CODE_1',
+    'CODE_2'
+]
+
+=cut
+
+sub _get_extended_attributes_entries {
+    my ( $self, $params, @array ) = @_;
+
+    if ( reftype($params) && reftype($params) eq 'HASH' ) {
+
+        # rewrite additional_field_values table query params
+        @array = _rewrite_related_metadata_query( $params, 'field_id', 'value', @array )
+            if $params->{'extended_attributes.field_id'};
+
+        # rewrite borrower_attributes table query params
+        @array = _rewrite_related_metadata_query( $params, 'code', 'attribute', @array )
+            if $params->{'extended_attributes.code'};
+
+        # rewrite illrequestattributes table query params
+        @array = _rewrite_related_metadata_query( $params, 'type', 'value', @array )
+            if $params->{'extended_attributes.type'};
+
+        foreach my $key ( keys %{$params} ) {
+            return $self->_get_extended_attributes_entries( $params->{$key}, @array );
+        }
+    } elsif ( reftype($params) && reftype($params) eq 'ARRAY' ) {
+        foreach my $ea_instance (@$params) {
+            @array = $self->_get_extended_attributes_entries( $ea_instance, @array );
+        }
+        return @array;
+    } else {
+        return @array;
+    }
+}
+
+=head3 _rewrite_related_metadata_query
+
+        $extended_attributes_entries =
+            _rewrite_related_metadata_query( $params, 'field_id', 'value', @array )
+
+Helper function that rewrites all subsequent extended_attributes queries to match the alias generated by the dbic self left join
+Take the below example (patrons):
+        [
+            {
+                "extended_attributes.attribute":{"like":"%123%"},
+                "extended_attributes.code":"CODE_1"
+            }
+        ],
+        [
+            {
+                "extended_attributes.attribute":{"like":"%abc%" },
+                "extended_attributes.code":"CODE_2"
+            }
+        ]
+
+It'll be rewritten as:
+        [
+            {
+                'extended_attributes_CODE_1.attribute' => { 'like' => '%123%' },
+                'extended_attributes_CODE_1.code' => 'CODE_1'
+            }
+        ],
+            [
+            {
+                'extended_attributes_CODE_2.attribute' => { 'like' => '%abc%' },
+                'extended_attributes_CODE_2.code' => 'CODE_2'
+            }
+        ]
+
+=cut
+
+sub _rewrite_related_metadata_query {
+    my ( $params, $key, $value, @array ) = @_;
+
+    if ( ref \$params->{ 'extended_attributes.' . $key } eq 'SCALAR' ) {
+        my $old_key_value = delete $params->{ 'extended_attributes.' . $key };
+        my $new_key_value = "extended_attributes_$old_key_value" . "." . $key;
+        $params->{$new_key_value} = $old_key_value;
+
+        my $old_value_value = delete $params->{ 'extended_attributes.' . $value };
+        my $new_value_value = "extended_attributes_$old_key_value" . "." . $value;
+        $params->{$new_value_value} = $old_value_value;
+        push @array, $old_key_value;
+    }
+
+    return @array;
+}
+
+=head3 _build_extended_attributes_relations
+
+    Method to dynamically add has_many relations for Koha classes that support extended_attributes.
+
+    Returns a list of relation accessor names.
+
+=cut
+
+sub _build_extended_attributes_relations {
+    my ( $self, $types ) = @_;
+
+    return if ( !grep ( /extended_attributes/, keys %{ $self->_resultset->result_source->_relationships } ) );
+
+    my $ea_config = $self->extended_attributes_config;
+
+    my $result_source = $self->_resultset->result_source;
+    for my $type ( @{$types} ) {
+        $result_source->add_relationship(
+            "extended_attributes_$type",
+            "$ea_config->{schema_class}",
+            sub {
+                my $args = shift;
+
+                return {
+                    "$args->{foreign_alias}.$ea_config->{id_field}->{foreign}" =>
+                        { -ident => "$args->{self_alias}.$ea_config->{id_field}->{self}" },
+                    "$args->{foreign_alias}.$ea_config->{key_field}" => { '=', $type },
+                };
+            },
+            {
+                accessor       => 'multi',
+                join_type      => 'LEFT',
+                cascade_copy   => 0,
+                cascade_delete => 0,
+                is_depends_on  => 0
+            },
+        );
+
+    }
+    return map { 'extended_attributes_' . $_ } @{$types};
+}
+
+1;

Koha/Old/Biblio.pm

@@ -126,7 +126,7 @@ sub to_api_mapping {
         seriestitle   => 'series_title',
         copyrightdate => 'copyright_date',
         datecreated   => 'creation_date',
-        deleted_on    => 'timestamp',
+        timestamp     => 'deleted_on',
     };
 }
 

Koha/Patron.pm

@@ -20,7 +20,7 @@ package Koha::Patron;
 
 use Modern::Perl;
 
-use List::MoreUtils qw( any uniq );
+use List::MoreUtils qw( any none uniq );
 use JSON qw( to_json );
 use Unicode::Normalize qw( NFKD );
 use Try::Tiny;
@@ -315,7 +315,7 @@ sub store {
                 }
 
                 foreach my $guarantor (@$guarantors) {
-                    if ( $guarantor->is_child or $guarantor->category->can_be_guarantee ) {
+                    if ( $guarantor->is_child ) {
                         Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw( invalid_guarantor => 1 );
                     }
                 }
@@ -372,7 +372,7 @@ sub store {
                 }
 
                 foreach my $guarantor (@$guarantors) {
-                    if ( $guarantor->is_child or $guarantor->category->can_be_guarantee ) {
+                    if ( $guarantor->is_child ) {
                         Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw( invalid_guarantor => 1 );
                     }
                 }
@@ -443,7 +443,7 @@ sub delete {
     Koha::Exceptions::Patron::FailedDeleteAnonymousPatron->throw() if $anonymous_patron && $self->id eq $anonymous_patron;
 
     # Check if patron is protected
-    Koha::Exceptions::Patron::FailedDeleteProtectedPatron->throw() if $self->protected == 1;
+    Koha::Exceptions::Patron::FailedDeleteProtectedPatron->throw() if defined $self->protected && $self->protected == 1;
 
     $self->_result->result_source->schema->txn_do(
         sub {
@@ -499,7 +499,7 @@ Returns a Koha::Library object representing the patron's home library.
 
 sub library {
     my ( $self ) = @_;
-    return Koha::Library->_new_from_dbic($self->_result->branchcode);
+    return Koha::Library->_new_from_dbic($self->_result->library);
 }
 
 =head3 sms_provider
@@ -533,6 +533,18 @@ sub guarantor_relationships {
     return Koha::Patron::Relationships->search( { guarantee_id => $self->id } );
 }
 
+=head3 is_guarantee
+
+Returns true if the patron has a guarantor.
+
+=cut
+
+sub is_guarantee {
+    my ($self) = @_;
+    return $self->guarantor_relationships()->count();
+}
+
+
 =head3 guarantee_relationships
 
 Returns Koha::Patron::Relationships object for this patron's guarantors
@@ -557,6 +569,18 @@ sub guarantee_relationships {
     );
 }
 
+=head3 is_guarantor
+
+Returns true if the patron is a guarantor.
+
+=cut
+
+sub is_guarantor {
+    my ($self) = @_;
+    return $self->guarantee_relationships()->count();
+}
+
+
 =head3 relationships_debt
 
 Returns the amount owed by the patron's guarantors *and* the other guarantees of those guarantors
@@ -1527,6 +1551,7 @@ sub get_age {
     my $date_of_birth = dt_from_string( $self->dateofbirth, undef, 'floating' );
     my $today         = dt_from_string(undef, undef, 'floating')->truncate( to => 'day' );
 
+    return 0 unless $date_of_birth;
     return $today->subtract_datetime( $date_of_birth )->years;
 }
 
@@ -1584,6 +1609,7 @@ Return all the historical holds for this patron
 sub old_holds {
     my ($self) = @_;
     my $old_holds_rs = $self->_result->old_reserves->search( {}, { order_by => 'reservedate' } );
+    require Koha::Old::Holds;
     return Koha::Old::Holds->_new_from_dbic($old_holds_rs);
 }
 
@@ -1629,23 +1655,27 @@ sub return_claims {
 
 =head3 notice_email_address
 
-  my $email = $patron->notice_email_address;
+    my $email = $patron->notice_email_address;
 
 Return the email address of patron used for notices.
 Returns the empty string if no email address.
 
 =cut
 
-sub notice_email_address{
-    my ( $self ) = @_;
+sub notice_email_address {
+    my ($self) = @_;
 
     my $which_address = C4::Context->preference("EmailFieldPrimary");
 
-    # if syspref is set to 'first valid' (value == OFF), look up email address
-    if ( $which_address eq 'OFF' ) {
-        return $self->first_valid_email_address;
+    if ( $which_address && ( none { $_ eq $which_address } qw{email emailpro B_email cardnumber MULTI} ) ) {
+        warn "Invalid value for EmailFieldPrimary ($which_address)";
+        $which_address = undef;
     }
 
+    # if syspref is set to 'first valid', look up email address
+    return $self->first_valid_email_address
+        unless $which_address;
+
     # if syspref is set to 'selected addresses' (value == MULTI), look up email addresses
     if ( $which_address eq 'MULTI' ) {
         my @addresses;
@@ -1654,11 +1684,10 @@ sub notice_email_address{
             my $email_address = $self->$email_field;
             push @addresses, $email_address if $email_address;
         }
-        return join(",",@addresses);
+        return join( ",", @addresses );
     }
 
     return $self->$which_address || '';
-
 }
 
 =head3 first_valid_email_address
@@ -1946,7 +1975,7 @@ sub libraries_where_can_see_things {
     my $subpermission = $params->{subpermission};
     my $group_feature = $params->{group_feature};
 
-    return $self->{"_restricted_branchcodes:$permission:$subpermission:$group_feature"}
+    return @{ $self->{"_restricted_branchcodes:$permission:$subpermission:$group_feature"} }
         if exists( $self->{"_restricted_branchcodes:$permission:$subpermission:$group_feature"} );
 
     my $userenv = C4::Context->userenv;
@@ -2179,17 +2208,22 @@ sub extended_attributes {
                 }
 
                 # Check globally mandatory types
-                my @required_attribute_types =
-                    Koha::Patron::Attribute::Types->search(
-                        {
-                            mandatory => 1,
-                            category_code => [ undef, $self->categorycode ],
-                            'borrower_attribute_types_branches.b_branchcode' =>
-                              undef,
-                        },
-                        { join => 'borrower_attribute_types_branches' }
-                    )->get_column('code');
-                for my $type ( @required_attribute_types ) {
+                my $interface = C4::Context->interface;
+                my $params = {
+                    mandatory                                        => 1,
+                    category_code                                    => [ undef, $self->categorycode ],
+                    'borrower_attribute_types_branches.b_branchcode' => undef,
+                };
+
+                if ( $interface eq 'opac' ) {
+                    $params->{opac_editable} = 1;
+                }
+
+                my @required_attribute_types = Koha::Patron::Attribute::Types->search(
+                    $params,
+                    { join => 'borrower_attribute_types_branches' }
+                )->get_column('code');
+                for my $type (@required_attribute_types) {
                     Koha::Exceptions::Patron::MissingMandatoryExtendedAttribute->throw(
                         type => $type,
                     ) if !$new_types->{$type};

Koha/Patron/Attribute.pm

@@ -59,6 +59,11 @@ sub store {
     Koha::Exceptions::Patron::Attribute::InvalidAttributeValue->throw( attribute => $self )
         unless $self->value_ok();
 
+    C4::Context->dbh->do(
+        "UPDATE borrowers SET updated_on = NOW() WHERE borrowernumber = ?", undef,
+        $self->borrowernumber
+    );
+
     return $self->SUPER::store();
 }
 

Koha/Patrons.pm

@@ -29,7 +29,7 @@ use Koha::Patron;
 use Koha::Exceptions::Patron;
 use Koha::Patron::Categories;
 
-use base qw(Koha::Objects);
+use base qw(Koha::Objects::Mixin::ExtendedAttributes Koha::Objects);
 
 =head1 NAME
 
@@ -566,6 +566,19 @@ sub filter_by_have_permission {
     );
 }
 
+=head3 extended_attributes_config
+
+=cut
+
+sub extended_attributes_config {
+    my ($self) = @_;
+    return {
+        'id_field'     => { 'foreign' => 'borrowernumber', 'self' => 'borrowernumber' },
+        'key_field'    => 'code',
+        'schema_class' => 'Koha::Schema::Result::BorrowerAttribute',
+    };
+}
+
 =head3 _type
 
 =cut

Koha/Patrons/Import.pm

@@ -61,7 +61,7 @@ has 'today_iso' => ( is => 'ro', lazy => 1,
     default => sub { output_pref( { dt => dt_from_string(), dateonly => 1, dateformat => 'iso' } ); }, );
 
 has 'text_csv' => ( is => 'rw', lazy => 1,
-    default => sub { Text::CSV->new( { binary => 1, } ); },  );
+    default => sub { Text::CSV->new( { binary => 1, formula => 'empty' } ); },  );
 
 sub import_patrons {
     my ($self, $params) = @_;
@@ -627,7 +627,7 @@ sub generate_patron_attributes {
     push (@$feedback, { feedback => 1, name => 'attribute string', value => $string });
     return [] unless $string; # Unit tests want the feedback, is it really needed?
 
-    my $csv = Text::CSV->new({binary => 1});  # binary needed for non-ASCII Unicode
+    my $csv = Text::CSV->new( { binary => 1, formula => "empty" } );    # binary needed for non-ASCII Unicode
     my $ok   = $csv->parse($string);  # parse field again to get subfields!
     my @list = $csv->fields();
     my @patron_attributes =

Koha/REST/Plugin/Auth/PublicRoutes.pm

@@ -0,0 +1,119 @@
+package Koha::REST::Plugin::Auth::PublicRoutes;
+
+# Copyright Hypernova Oy 2024
+#
+# This file is part of Koha.
+#
+# Koha is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# Koha is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Koha; if not, see <http://www.gnu.org/licenses>.
+
+use Modern::Perl;
+
+use Mojo::Base 'Mojolicious::Plugin';
+
+use Koha::Exception;
+use Koha::Exceptions;
+use Koha::Exceptions::REST;
+use Koha::Patrons;
+
+use Module::Load::Conditional;
+
+=head1 NAME
+
+Koha::REST::Plugin::Auth::PublicRoutes
+
+=head1 API
+
+=head2 Mojolicious::Plugin methods
+
+=head3 register
+
+=cut
+
+sub register {
+    my ( $self, $app ) = @_;
+
+=head2 Helper methods
+
+=head3 auth.public
+
+    $c->auth->public( $public_user_id );
+
+=cut
+
+    $app->helper(
+        'auth.public' => sub {
+            my ( $c, $public_user_id ) = @_;
+
+            unless ( $c->stash('is_public') ) {
+                Koha::Exception->throw( error => "This is not a public route!" );
+            }
+
+            my $user = $c->stash('koha.user');
+
+            unless ($user) {
+                Koha::Exceptions::REST::Public::Authentication::Required->throw( error => "Authentication failure." );
+            }
+
+            unless ($public_user_id) {
+                Koha::Exceptions::REST::Public::Unauthorized->throw(
+                    error => "Unprivileged user cannot access another user's resources" );
+            }
+
+            if ( $user->borrowernumber == $public_user_id ) {
+                return 1;
+            } else {
+                Koha::Exceptions::REST::Public::Unauthorized->throw(
+                    error => "Unprivileged user cannot access another user's resources" );
+            }
+        }
+    );
+
+=head3 auth.public_guarantor
+
+    $c->auth->public_guarantor( $public_user_id );
+
+=cut
+
+    $app->helper(
+        'auth.public_guarantor' => sub {
+            my ( $c, $public_user_id ) = @_;
+
+            unless ( $c->stash('is_public') ) {
+                Koha::Exception->throw( error => "This is not a public route!" );
+            }
+
+            my $user = $c->stash('koha.user');
+
+            unless ($user) {
+                Koha::Exceptions::REST::Public::Authentication::Required->throw( error => "Authentication failure." );
+            }
+
+            unless ($public_user_id) {
+                Koha::Exceptions::REST::Public::Unauthorized->throw(
+                    error => "Unprivileged user cannot access another user's resources" );
+            }
+
+            my $guarantees = $user->guarantee_relationships->guarantees->as_list;
+            foreach my $guarantee ( @{$guarantees} ) {
+                if ( $guarantee->borrowernumber == $public_user_id ) {
+                    return 1;
+                }
+            }
+            Koha::Exceptions::REST::Public::Unauthorized->throw(
+                error => "Unprivileged user cannot access another user's resources" );
+        }
+    );
+}
+
+1;

Koha/REST/Plugin/Exceptions.pm

@@ -59,6 +59,33 @@ sub register {
             my $path   = $req->url->to_abs->path;
             my $type = "";
 
+            if ( blessed $exception && ref($exception) eq 'Koha::Exceptions::REST::Query::InvalidOperator' ) {
+                return $c->render(
+                    status => 400,
+                    json   => {
+                        error      => printf( "Invalid operator in query: %s", $exception->operator ),
+                        error_code => 'invalid_query',
+                    }
+                );
+            }
+            elsif ( blessed $exception
+                && ref($exception) eq 'Koha::Exceptions::REST::Public::Authentication::Required' )
+            {
+                return $c->render(
+                    status => 401,
+                    json   => {
+                        error => $exception->error,
+                    }
+                );
+            } elsif ( blessed $exception && ref($exception) eq 'Koha::Exceptions::REST::Public::Unauthorized' ) {
+                return $c->render(
+                    status => 403,
+                    json   => {
+                        error => $exception->error,
+                    }
+                );
+            }
+
             if ( blessed $exception ) {
                 $type = "(" . ref($exception) . ")";
             }

Koha/REST/Plugin/Objects.pm

@@ -287,13 +287,7 @@ controller, and thus shouldn't be called twice in it.
             $c->stash('koha.pagination.base_total'   => $result_set->count);
             $c->stash('koha.pagination.query_params' => $args);
 
-            # Check and handle related metadata table joins
-            $c->dbic_extended_attributes_join(
-                {
-                    attributes      => $attributes,
-                    filtered_params => $filtered_params
-                }
-            );
+            $c->dbic_validate_operators( { filtered_params => $filtered_params } );
 
             # Generate the resultset
             my $objects_rs = $result_set->search( $filtered_params, $attributes );

Koha/REST/Plugin/Query.pm

@@ -23,6 +23,7 @@ use Scalar::Util qw( reftype );
 use JSON qw( decode_json );
 
 use Koha::Exceptions;
+use Koha::Exceptions::REST;
 
 =head1 NAME
 
@@ -159,31 +160,18 @@ Generates the DBIC prefetch attribute based on embedded relations, and merges in
         }
     );
 
-=head3 dbic_extended_attributes_join
+=head3 dbic_validate_operators
 
-    $attributes = $c->dbic_extended_attributes_join({ attributes => $attributes, result_set => $result_set });
+    $attributes = $c->dbic_validate_operators( { filtered_params => $filtered_params } );
 
-Generates the DBIC join attribute based on extended_attributes query entries, and merges into I<$attributes>.
+Validate operators in the passed query.
 
 =cut
 
     $app->helper(
-        'dbic_extended_attributes_join' => sub {
+        'dbic_validate_operators' => sub {
             my ( $c, $args ) = @_;
-
-            my $attributes      = $args->{attributes};
-            my $filtered_params = $args->{filtered_params};
-
-            if (   reftype( $attributes->{prefetch} )
-                && reftype( $attributes->{prefetch} ) eq 'ARRAY'
-                && grep ( /extended_attributes/, @{ $attributes->{prefetch} } ) )
-            {
-                my $ea_entries = $self->_get_extended_attributes_entries( $filtered_params, 0 );
-                while ( $ea_entries > 0 ) {
-                    push( @{ $attributes->{join} }, 'extended_attributes' );
-                    $ea_entries--;
-                }
-            }
+            _validate_query( $args->{filtered_params} );
         }
     );
 
@@ -516,129 +504,57 @@ sub _parse_dbic_query {
     }
 }
 
-=head3 _get_extended_attributes_entries
-
-    $self->_get_extended_attributes_entries( $filtered_params, 0 )
-
-Recursive function that returns the number of extended_attributes entries present in a query.
-Example: Returns 2 if given a $filtered_params containing the below:
-
-{
-    "-and":[
-        [
-            {
-                "extended_attributes.value":{
-                    "like":"abc%"
-                },
-                "extended_attributes.code":[
-                    [
-                        "arbitrary_attr_code",
-                        "another_attr_code"
-                    ]
-                ]
-            }
-        ],
-        [
-            {
-                "extended_attributes.value":{
-                    "like":"123%"
-                },
-                "extended_attributes.code":[
-                    [
-                        "arbitrary_attr_code",
-                        "another_attr_code"
-                    ]
-                ]
-            }
-        ]
-    ]
-}
+=head3 _validate_operator
 
 =cut
 
-sub _get_extended_attributes_entries {
-    my ( $self, $params, $extended_attributes_entries ) = @_;
-
-    if ( reftype($params) && reftype($params) eq 'HASH' ) {
-
-        # rewrite additional_field_values table query params
-        $extended_attributes_entries =
-            _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'field_id', 'value' )
-            if $params->{'extended_attributes.field_id'};
-
-        # rewrite borrower_attributes table query params
-        $extended_attributes_entries =
-            _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'code', 'attribute' )
-            if $params->{'extended_attributes.code'};
-
-        # rewrite illrequestattributes table query params
-        $extended_attributes_entries =
-            _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'type', 'value' )
-            if $params->{'extended_attributes.type'};
-
-        foreach my $key ( keys %{$params} ) {
-            return $self->_get_extended_attributes_entries( $params->{$key}, $extended_attributes_entries );
-        }
-    } elsif ( reftype($params) && reftype($params) eq 'ARRAY' ) {
-        foreach my $ea_instance (@$params) {
-            $extended_attributes_entries =
-                +$self->_get_extended_attributes_entries( $ea_instance, $extended_attributes_entries );
-        }
-        return $extended_attributes_entries;
-    } else {
-        return $extended_attributes_entries;
-    }
+sub _validate_operator {
+    my ($operator) = @_;
+    my %allowed_operators =
+        map { $_ => 1 }
+        qw(= != < > <= >= -in -ident -bool -not_bool -or like -like -not_like -between -not_between -not -and -regexp -not_in);
+    Koha::Exceptions::REST::Query::InvalidOperator->throw( operator => $operator )
+        unless exists $allowed_operators{$operator};
+    return;
 }
 
-=head3 _rewrite_related_metadata_query
-
-        $extended_attributes_entries =
-            _rewrite_related_metadata_query( $params, $extended_attributes_entries, 'field_id', 'value' )
-
-Helper function that rewrites all subsequent extended_attributes queries to match the alias generated by the dbic self left join
-Take the below example (patrons), assuming it is the third instance of an extended_attributes query:
-{
-    "extended_attributes.value":{
-        "like":"123%"
-    },
-    "extended_attributes.code":[
-        [
-            "arbitrary_attr_code",
-            "another_attr_code"
-        ]
-    ]
-}
-
-It'll be rewritten as:
-{
-    "extended_attributes_3.value":{
-        "like":"123%"
-    },
-    "extended_attributes_3.code":[
-        [
-            "arbitrary_attr_code",
-            "another_attr_code"
-        ]
-    ]
-}
+=head3 _validate_query
 
 =cut
 
-sub _rewrite_related_metadata_query {
-    my ( $params, $extended_attributes_entries, $key, $value ) = @_;
+sub _validate_query {
+    my ($query) = @_;
 
-    $extended_attributes_entries++;
-    if ( $extended_attributes_entries > 1 ) {
-        my $old_key_value = delete $params->{ 'extended_attributes.' . $key };
-        my $new_key_value = "extended_attributes_$extended_attributes_entries" . "." . $key;
-        $params->{$new_key_value} = $old_key_value;
+    if ( ref $query eq 'HASH' ) {
+        for my $field ( keys %$query ) {
+            my $value = $query->{$field};
 
-        my $old_value_value = delete $params->{ 'extended_attributes.' . $value };
-        my $new_value_value = "extended_attributes_$extended_attributes_entries" . "." . $value;
-        $params->{$new_value_value} = $old_value_value;
+            if ( ref $value eq 'HASH' ) {
+
+                # Hash reference with operators
+                for my $operator ( keys %$value ) {
+                    _validate_operator($operator);
+                }
+            } elsif ( ref $value eq 'ARRAY' ) {
+
+                # Array reference with operator as the first element
+                _validate_query($value);
+            } else {
+
+                # Simple key-value pairs (no operator to validate)
+            }
+        }
+    } elsif ( ref $query eq 'ARRAY' ) {
+
+        # Top-level OR combination or nested AND combination
+        for my $element (@$query) {
+            if ( ref $element eq 'ARRAY' && scalar @{$element} && $element->[0] eq '-and' ) {
+                _validate_query( $element->[1] );
+            } else {
+                _validate_query($element);
+            }
+        }
     }
-
-    return $extended_attributes_entries;
 }
 
 1;

Koha/REST/V1.pm

@@ -62,6 +62,17 @@ sub startup {
               if C4::Context->preference('AccessControlAllowOrigin');
         }
     );
+    $self->hook(
+        around_action => sub {
+            my ($next, $c, $action, $last) = @_;
+
+            # Flush memory caches before every request
+            Koha::Caches->flush_L1_caches();
+            Koha::Cache::Memory::Lite->flush();
+
+            return $next->();
+        }
+    );
 
     # Force charset=utf8 in Content-Type header for JSON responses
     $self->types->type( json    => 'application/json; charset=utf8' );
@@ -159,6 +170,7 @@ sub startup {
     $self->plugin('Koha::REST::Plugin::Exceptions');
     $self->plugin('Koha::REST::Plugin::Responses');
     $self->plugin('Koha::REST::Plugin::Auth::IdP');
+    $self->plugin('Koha::REST::Plugin::Auth::PublicRoutes');
     $self->plugin( 'Mojolicious::Plugin::OAuth2' => $oauth_configuration );
 }
 

Koha/REST/V1/ArticleRequests.pm

@@ -73,22 +73,23 @@ Controller function that handles cancelling a patron's Koha::ArticleRequest obje
 sub patron_cancel {
     my $c = shift->openapi->valid_input or return;
 
-    my $patron = Koha::Patrons->find( $c->param('patron_id') );
-
-    return $c->render_resource_not_found("Patron")
-        unless $patron;
-
-    # patron_id has been validated by the allow-owner check, so the following call to related
-    # article requests covers the case of article requests not belonging to the patron
-    my $article_request = $patron->article_requests->find( $c->param('article_request_id') );
-
-    return $c->render_resource_not_found("Article request")
-        unless $article_request;
-
-    my $reason = $c->param('cancellation_reason');
-    my $notes  = $c->param('notes');
-
+    my $patron_id = $c->param('patron_id');
     return try {
+        $c->auth->public($patron_id);
+        my $patron = Koha::Patrons->find($patron_id);
+
+        return $c->render_resource_not_found("Patron")
+            unless $patron;
+
+        # patron_id has been validated by the $c->auth->public check, so the following call to related
+        # article requests covers the case of article requests not belonging to the patron
+        my $article_request = $patron->article_requests->find( $c->param('article_request_id') );
+
+        return $c->render_resource_not_found("Article request")
+            unless $article_request;
+
+        my $reason = $c->param('cancellation_reason');
+        my $notes  = $c->param('notes');
 
         $article_request->cancel(
             {

Koha/REST/V1/Auth.pm

@@ -319,9 +319,7 @@ sub authenticate_api_request {
 
     my $permissions = $authorization->{'permissions'};
     # Check if the user is authorized
-    if ( ( defined($permissions) and haspermission($user->userid, $permissions) )
-        or allow_owner($c, $authorization, $user)
-        or allow_guarantor($c, $authorization, $user) ) {
+    if ( ( defined($permissions) and haspermission($user->userid, $permissions) ) ) {
 
         validate_query_parameters( $c, $spec );
 
@@ -360,158 +358,6 @@ sub validate_query_parameters {
     ) if @errors;
 }
 
-=head3 allow_owner
-
-Allows access to object for its owner.
-
-There are endpoints that should allow access for the object owner even if they
-do not have the required permission, e.g. access an own reserve. This can be
-achieved by defining the operation as follows:
-
-"/holds/{reserve_id}": {
-    "get": {
-        ...,
-        "x-koha-authorization": {
-            "allow-owner": true,
-            "permissions": {
-                "borrowers": "1"
-            }
-        }
-    }
-}
-
-=cut
-
-sub allow_owner {
-    my ($c, $authorization, $user) = @_;
-
-    return unless $authorization->{'allow-owner'};
-
-    return check_object_ownership($c, $user) if $user and $c;
-}
-
-=head3 allow_guarantor
-
-Same as "allow_owner", but checks if the object is owned by one of C<$user>'s
-guarantees.
-
-=cut
-
-sub allow_guarantor {
-    my ($c, $authorization, $user) = @_;
-
-    if (!$c || !$user || !$authorization || !$authorization->{'allow-guarantor'}){
-        return;
-    }
-
-    my $guarantees = $user->guarantee_relationships->guarantees->as_list;
-    foreach my $guarantee (@{$guarantees}) {
-        return 1 if check_object_ownership($c, $guarantee);
-    }
-}
-
-=head3 check_object_ownership
-
-Determines ownership of an object from request parameters.
-
-As introducing an endpoint that allows access for object's owner; if the
-parameter that will be used to determine ownership is not already inside
-$parameters, add a new subroutine that checks the ownership and extend
-$parameters to contain a key with parameter_name and a value of a subref to
-the subroutine that you created.
-
-=cut
-
-sub check_object_ownership {
-    my ($c, $user) = @_;
-
-    return if not $c or not $user;
-
-    my $parameters = {
-        accountlines_id => \&_object_ownership_by_accountlines_id,
-        borrowernumber  => \&_object_ownership_by_patron_id,
-        patron_id       => \&_object_ownership_by_patron_id,
-        checkout_id     => \&_object_ownership_by_checkout_id,
-        reserve_id      => \&_object_ownership_by_reserve_id,
-    };
-
-    foreach my $param ( keys %{ $parameters } ) {
-        my $check_ownership = $parameters->{$param};
-        if ($c->stash($param)) {
-            return &$check_ownership($c, $user, $c->stash($param));
-        }
-        elsif ($c->param($param)) {
-            return &$check_ownership($c, $user, $c->param($param));
-        }
-        elsif ($c->match->stack->[-1]->{$param}) {
-            return &$check_ownership($c, $user, $c->match->stack->[-1]->{$param});
-        }
-        elsif ($c->req->json && $c->req->json->{$param}) {
-            return 1 if &$check_ownership($c, $user, $c->req->json->{$param});
-        }
-    }
-}
-
-=head3 _object_ownership_by_accountlines_id
-
-Finds a Koha::Account::Line-object by C<$accountlines_id> and checks if it
-belongs to C<$user>.
-
-=cut
-
-sub _object_ownership_by_accountlines_id {
-    my ($c, $user, $accountlines_id) = @_;
-
-    my $accountline = Koha::Account::Lines->find($accountlines_id);
-    return $accountline && $user->borrowernumber == $accountline->borrowernumber;
-}
-
-=head3 _object_ownership_by_borrowernumber
-
-Compares C<$borrowernumber> to currently logged in C<$user>.
-
-=cut
-
-sub _object_ownership_by_patron_id {
-    my ($c, $user, $patron_id) = @_;
-
-    return $user->borrowernumber == $patron_id;
-}
-
-=head3 _object_ownership_by_checkout_id
-
-First, attempts to find a Koha::Checkout-object by C<$issue_id>. If we find one,
-compare its borrowernumber to currently logged in C<$user>. However, if an issue
-is not found, attempt to find a Koha::Old::Checkout-object instead and compare its
-borrowernumber to currently logged in C<$user>.
-
-=cut
-
-sub _object_ownership_by_checkout_id {
-    my ($c, $user, $issue_id) = @_;
-
-    my $issue = Koha::Checkouts->find($issue_id);
-    $issue = Koha::Old::Checkouts->find($issue_id) unless $issue;
-    return $issue && $issue->borrowernumber
-            && $user->borrowernumber == $issue->borrowernumber;
-}
-
-=head3 _object_ownership_by_reserve_id
-
-Finds a Koha::Hold-object by C<$reserve_id> and checks if it
-belongs to C<$user>.
-
-TODO: Also compare against old_reserves
-
-=cut
-
-sub _object_ownership_by_reserve_id {
-    my ($c, $user, $reserve_id) = @_;
-
-    my $reserve = Koha::Holds->find($reserve_id);
-    return $reserve && $user->borrowernumber == $reserve->borrowernumber;
-}
-
 =head3 _basic_auth
 
 Internal method that performs Basic authentication.

Koha/REST/V1/Auth/Password.pm

@@ -73,7 +73,7 @@ sub validate {
     return try {
         my ( $status, $THE_cardnumber, $THE_userid, $patron ) = C4::Auth::checkpw( $identifier, $password );
         unless ( $status && $status > 0 ) {
-            my $error_response = $status == -2 ? 'Password expired' : 'Validation failed';
+            my $error_response = ($status && $status == -2) ? 'Password expired' : 'Validation failed';
             return $c->render(
                 status  => 400,
                 openapi => { error => $error_response }

Koha/REST/V1/AuthorisedValueCategories.pm

@@ -36,8 +36,9 @@ sub list {
     my $c = shift->openapi->valid_input or return;
 
     return try {
-        my $authorised_value_categories_set = Koha::AuthorisedValueCategories->new;
-        my $authorised_value_categories = $c->objects->search( $authorised_value_categories_set );
+        my $authorised_value_categories_set =
+            Koha::AuthorisedValueCategories->search( {}, { order_by => 'category_name' } );
+        my $authorised_value_categories = $c->objects->search($authorised_value_categories_set);
         return $c->render( status => 200, openapi => $authorised_value_categories );
     }
     catch {

Koha/REST/V1/Checkouts.pm

@@ -27,6 +27,7 @@ use Koha::Checkouts;
 use Koha::Old::Checkouts;
 use Koha::Token;
 
+use Scalar::Util qw( blessed );
 use Try::Tiny qw( catch try );
 
 =head1 NAME
@@ -120,7 +121,7 @@ sub _check_availability {
 
         # Upgrade some confirmations to blockers
         my @should_block =
-          qw/TOO_MANY ISSUED_TO_ANOTHER RESERVED RESERVED_WAITING TRANSFERRED PROCESSING AGE_RESTRICTION/;
+          qw/TOO_MANY ISSUED_TO_ANOTHER RESERVED RESERVE_WAITING TRANSFERRED PROCESSING AGE_RESTRICTION/;
         for my $block (@should_block) {
             if ( exists( $confirmation->{$block} ) ) {
                 $impossible->{$block} = $confirmation->{$block};
@@ -152,7 +153,16 @@ sub get_availability {
     my $c = shift->openapi->valid_input or return;
     my $user = $c->stash('koha.user');
 
-    my $patron = Koha::Patrons->find( $c->param('patron_id') );
+    my $patron_id = $c->param('patron_id');
+
+    return if try {
+        $c->auth->public($patron_id) if $c->stash('is_public');
+        return 0;    # authorization successful, do not "return" after try-catch
+    } catch {
+        return $c->unhandled_exception($_);
+    };
+
+    my $patron = Koha::Patrons->find($patron_id);
     my $item   = Koha::Items->find( $c->param('item_id') );
 
     my ( $impossible, $confirmation, $warnings ) =
@@ -190,19 +200,19 @@ sub add {
     my $patron_id = $body->{patron_id};
     my $onsite    = $body->{onsite_checkout};
 
-    if ( $c->stash('is_public')
-        && !C4::Context->preference('OpacTrustedCheckout') )
-    {
-        return $c->render(
-            status  => 405,
-            openapi => {
-                error      => 'Feature disabled',
-                error_code => 'FEATURE_DISABLED'
-            }
-        );
-    }
-
     return try {
+        if ( $c->stash('is_public') ) {
+            $c->auth->public($patron_id);
+
+            return $c->render(
+                status  => 405,
+                openapi => {
+                    error      => 'Feature disabled',
+                    error_code => 'FEATURE_DISABLED'
+                }
+            ) if !C4::Context->preference('OpacTrustedCheckout');
+        }
+
         my $item = Koha::Items->find($item_id);
         unless ($item) {
             return $c->render(

Koha/REST/V1/ERM/EHoldings/Packages/EBSCO.pm

@@ -35,12 +35,7 @@ sub list {
         my $result     = $ebsco->request( GET => '/packages' . $params );
         my $base_total = $result->{totalResults};
 
-        my ( $per_page, $page ) = $ebsco->build_query_pagination(
-            {
-                per_page => $c->stash('koha.pagination.per_page'),
-                page     => $c->stash('koha.pagination.page'),
-            }
-        );
+        my ( $per_page, $page ) = $ebsco->build_query_pagination( $c->req->params->to_hash );
         my $additional_params =
           $ebsco->build_additional_params( $c->req->params->to_hash );
 

Koha/REST/V1/ERM/EHoldings/Resources/EBSCO.pm

@@ -73,12 +73,7 @@ sub list {
 
         my $base_total = $result->{totalResults};
 
-        my ( $per_page, $page ) = $ebsco->build_query_pagination(
-            {
-                per_page => $c->stash('koha.pagination.per_page'),
-                page     => $c->stash('koha.pagination.page'),
-            }
-        );
+        my ( $per_page, $page ) = $ebsco->build_query_pagination( $c->req->params->to_hash );
 
         my $additional_params = $ebsco->build_additional_params( $c->req->params->to_hash );
         my $searchfield = 'titlename';

Koha/REST/V1/ERM/EHoldings/Titles/EBSCO.pm

@@ -41,12 +41,7 @@ sub list {
         my $ebsco = Koha::ERM::Providers::EBSCO->new;
 
         # We cannot get base_total as a search kw is required by the API
-        my ( $per_page, $page ) = $ebsco->build_query_pagination(
-            {
-                per_page => $c->stash('koha.pagination.per_page'),
-                page     => $c->stash('koha.pagination.page'),
-            }
-        );
+        my ( $per_page, $page ) = $ebsco->build_query_pagination( $c->req->params->to_hash );
 
         my $additional_params = $ebsco->build_additional_params( $c->req->params->to_hash );
 

Koha/REST/V1/ERM/EHoldings/Titles/Local.pm

@@ -27,7 +27,6 @@ use Scalar::Util qw( blessed );
 use Try::Tiny    qw( catch try );
 use MIME::Base64 qw( decode_base64 encode_base64 );
 use POSIX        qw( floor );
-use Text::CSV_XS;
 
 =head1 API
 

Koha/REST/V1/ERM/EUsage/CounterRegistry.pm

@@ -47,7 +47,7 @@ sub list {
 
     my $search_string = $query_params_array[0]->{name};
 
-    my $url     = 'https://registry.projectcounter.org/api/v1/platform/';
+    my $url     = 'https://registry.countermetrics.org/api/v1/platform/';
     my $request = HTTP::Request->new(
         GET => $url,
     );

Koha/REST/V1/Patrons.pm

@@ -456,6 +456,8 @@ sub guarantors_can_see_charges {
     my $c = shift->openapi->valid_input or return;
 
     return try {
+        $c->auth->public( $c->param('patron_id') );
+
         if ( C4::Context->preference('AllowPatronToSetFinesVisibilityForGuarantor') ) {
             my $patron = $c->stash( 'koha.user' );
             my $privacy_setting = ($c->req->json->{allowed}) ? 1 : 0;
@@ -492,6 +494,8 @@ sub guarantors_can_see_checkouts {
     my $c = shift->openapi->valid_input or return;
 
     return try {
+        $c->auth->public( $c->param('patron_id') );
+
         if ( C4::Context->preference('AllowPatronToSetCheckoutsVisibilityForGuarantor') ) {
             my $patron = $c->stash( 'koha.user' );
             my $privacy_setting = ( $c->req->json->{allowed} ) ? 1 : 0;

Koha/REST/V1/Patrons/Account.pm

@@ -207,14 +207,14 @@ sub add_debit {
     my $c = shift->openapi->valid_input or return;
 
     my $patron = Koha::Patrons->find( $c->param('patron_id') );
+    my $user   = $c->stash('koha.user');
 
     return $c->render_resource_not_found("Patron")
-        unless $patron;
+      unless $patron;
 
     return try {
         my $data =
-          Koha::Account::Debit->new_from_api( $c->req->json )
-          ->unblessed;
+          Koha::Account::Debit->new_from_api( $c->req->json )->unblessed;
 
         $data->{library_id}       = delete $data->{branchcode};
         $data->{type}             = delete $data->{debit_type_code};
@@ -223,11 +223,10 @@ sub add_debit {
         $data->{transaction_type} = delete $data->{payment_type};
         $data->{interface}        = 'api'
           ; # Should this always be API, or should we allow the API consumer to choose?
-        $data->{user_id} = $patron->borrowernumber
-          ; # Should this be API user OR staff the API may be acting on behalf of?
+        $data->{user_id} = delete $data->{manager_id} || $user->id;
 
         my $debit = $patron->account->add_debit($data);
-        $debit = Koha::Account::Debit->_new_from_dbic($debit->{_result});
+        $debit = Koha::Account::Debit->_new_from_dbic( $debit->{_result} );
 
         $c->res->headers->location(
             $c->req->url->to_string . '/' . $debit->id );
@@ -248,13 +247,13 @@ sub add_debit {
             }
             elsif ( $_->isa('Koha::Exceptions::Account::AmountNotPositive') ) {
                 return $c->render(
-                    status => 400,
+                    status  => 400,
                     openapi => { error => $_->description }
                 );
             }
             elsif ( $_->isa('Koha::Exceptions::Account::UnrecognisedType') ) {
                 return $c->render(
-                    status => 400,
+                    status  => 400,
                     openapi => { error => $_->description }
                 );
             }

Koha/REST/V1/Patrons/Holds.pm

@@ -21,6 +21,8 @@ use Mojo::Base 'Mojolicious::Controller';
 
 use Koha::Patrons;
 
+use Try::Tiny;
+
 =head1 NAME
 
 Koha::REST::V1::Patrons::Holds
@@ -72,6 +74,8 @@ sub delete_public {
     my $c = shift->openapi->valid_input or return;
 
     return try {
+        $c->auth->public( $c->param('patron_id') );
+
         my $hold = $c->objects->find_rs( Koha::Holds->new, $c->param('hold_id') );
 
         unless ( $hold and $c->param('patron_id') == $hold->borrowernumber ) {

Koha/REST/V1/Patrons/Password.pm

@@ -91,34 +91,25 @@ sub set_public {
 
     my $user = $c->stash('koha.user');
 
-    unless ( $user->borrowernumber == $patron_id ) {
-        return $c->render(
-            status  => 403,
-            openapi => {
-                error => "Changing other patron's password is forbidden"
-            }
-        );
-    }
-
-    unless ( $user->category->effective_change_password ) {
-        return $c->render(
-            status  => 403,
-            openapi => {
-                error => "Changing password is forbidden"
-            }
-        );
-    }
-
-    my $old_password = $body->{old_password};
-    my $password     = $body->{password};
-    my $password_2   = $body->{password_repeated};
-
-    unless ( $password eq $password_2 ) {
-        return $c->render( status => 400, openapi => { error => "Passwords don't match" } );
-    }
-
     return try {
-        unless ( checkpw_internal($user->userid, $old_password ) ) {
+        $c->auth->public($patron_id);
+
+        unless ( $user->category->effective_change_password ) {
+            return $c->render(
+                status  => 403,
+                openapi => { error => "Changing password is forbidden" }
+            );
+        }
+
+        my $old_password = $body->{old_password};
+        my $password     = $body->{password};
+        my $password_2   = $body->{password_repeated};
+
+        unless ( $password eq $password_2 ) {
+            return $c->render( status => 400, openapi => { error => "Passwords don't match" } );
+        }
+
+        unless ( checkpw_internal( $user->userid, $old_password ) ) {
             return $c->render(
                 status  => 400,
                 openapi => { error => "Invalid password" }
@@ -126,19 +117,20 @@ sub set_public {
         }
 
         ## Change password
-        $user->set_password({ password => $password });
+        $user->set_password( { password => $password } );
 
         return $c->render( status => 200, openapi => "" );
-    }
-    catch {
+    } catch {
         if ( blessed $_ and $_->isa('Koha::Exceptions::Password') ) {
             return $c->render(
                 status  => 400,
                 openapi => { error => "$_" }
             );
+        } elsif ( blessed $_ and $_->isa('Koha::Exceptions::REST::Public::Unauthorized') ) {
+            return $c->render( status => 403, json => { error => "Changing other patron's password is forbidden" } );
+        } else {
+            $c->unhandled_exception($_);
         }
-
-        $c->unhandled_exception($_);
     };
 }