Bug 27836: Document that CirControl changes which calendar is being used

You can verify that which branch's Koha calendar is being used to
calculate fines by reading for example the code in
C4::Circulation::_CalculateAndUpdateFine() There you can see that
$control_branchcode variable is being passed to
C4::Overdues::CalcFine() and if you trace that you will see in the end
it decides which branch's calendar to use for calculating chargeable
overdue fine days.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

C4/Breeding.pm

@@ -618,6 +618,8 @@ sub Z3950SearchAuth {
                             my $heading;
                             my $heading_authtype_code;
                             $heading_authtype_code = GuessAuthTypeCode($marcrecord);
+                            next if ( not defined $heading_authtype_code ) ;
+
                             $heading = C4::AuthoritiesMarc::GetAuthorizedHeading({ record => $marcrecord });
 
                             my $breedingid = ImportBreedingAuth( $marcrecord, $serverhost[$k], $encoding[$k], $heading );

C4/Utils/DataTables.pm

@@ -104,10 +104,15 @@ sub dt_build_orderby {
 
     return unless @orderbys;
 
+    my @sanitized_orderbys;
+
+    # Trick for virtualshelves, should not be extended
+    push @sanitized_orderbys, 'count asc' if grep {$_ eq 'count asc'} @orderbys;
+    push @sanitized_orderbys, 'count desc' if grep {$_ eq 'count desc'} @orderbys;
+
     # Must be "branches.branchname asc", "borrowers.firstname desc", etc.
     @orderbys = grep { /^\w+\.\w+ (asc|desc)$/ } @orderbys;
 
-    my @sanitized_orderbys;
     for my $orderby (@orderbys) {
       my ($identifier, $direction) = split / /, $orderby, 2;
       my ($table, $column) = split /\./, $identifier, 2;

Koha/Account.pm

@@ -128,7 +128,7 @@ sub pay {
 
         # Attempt to renew the item associated with this debit if
         # appropriate
-        if ($fine->renewable) {
+        if ($fine->is_renewable) {
             # We're ignoring the definition of $interface above, by all
             # accounts we can't rely on C4::Context::interface, so here
             # we're only using what we've been explicitly passed
@@ -203,7 +203,7 @@ sub pay {
         # If we need to make a note of the item associated with this line,
         # in order that we can potentially renew it, do so.
         my $amt = $old_amountoutstanding - $amount_to_pay;
-        if ( $fine->renewable ) {
+        if ( $fine->is_renewable ) {
             my $outcome = $fine->renew_item({ interface => $interface });
             push @{$renew_outcomes}, $outcome if $outcome;
         }

Koha/Account/Line.pm

@@ -596,7 +596,7 @@ sub apply {
 
             # Attempt to renew the item associated with this debit if
             # appropriate
-            if ( $self->credit_type_code ne 'FORGIVEN' && $debit->renewable ) {
+            if ( $self->credit_type_code ne 'FORGIVEN' && $debit->is_renewable ) {
                 $debit->renew_item( { interface => $params->{interface} } );
             }
 
@@ -887,13 +887,13 @@ sub to_api_mapping {
 
 }
 
-=head3 renewable
+=head3 is_renewable
 
-    my $bool = $line->renewable;
+    my $bool = $line->is_renewable;
 
 =cut
 
-sub renewable {
+sub is_renewable {
     my ($self) = @_;
 
     return (
@@ -913,7 +913,7 @@ sub renewable {
 
 Conditionally attempt to renew an item and return the outcome. This is
 as a consequence of the fine on an item being fully paid off.
-Caller must call renewable before.
+Caller must call is_renewable before.
 
 =cut
 

Koha/REST/V1/Holds.pm

@@ -508,4 +508,53 @@ sub pickup_locations {
     };
 }
 
+=head3 update_pickup_location
+
+Method that handles modifying the pickup location of a Koha::Hold object
+
+=cut
+
+sub update_pickup_location {
+    my $c = shift->openapi->valid_input or return;
+
+    my $hold_id = $c->validation->param('hold_id');
+    my $body    = $c->validation->param('body');
+    my $pickup_library_id = $body->{pickup_library_id};
+
+    my $hold = Koha::Holds->find($hold_id);
+
+    unless ($hold) {
+        return $c->render(
+            status  => 404,
+            openapi => { error => "Hold not found" }
+        );
+    }
+
+    return try {
+
+        $hold->set_pickup_location({ library_id => $pickup_library_id });
+
+        return $c->render(
+            status  => 200,
+            openapi => {
+                pickup_library_id => $pickup_library_id
+            }
+        );
+    }
+    catch {
+
+        if ( blessed $_ and $_->isa('Koha::Exceptions::Hold::InvalidPickupLocation') ) {
+            return $c->render(
+                status  => 400,
+                openapi => {
+                    error => "$_"
+                }
+            );
+        }
+
+        $c->unhandled_exception($_);
+    };
+}
+
+
 1;

api/v1/swagger/paths.json

@@ -71,6 +71,9 @@
   "/holds/{hold_id}/pickup_locations": {
     "$ref": "paths/holds.json#/~1holds~1{hold_id}~1pickup_locations"
   },
+  "/holds/{hold_id}/pickup_location": {
+    "$ref": "paths/holds.json#/~1holds~1{hold_id}~1pickup_location"
+  },
   "/items": {
     "$ref": "paths/items.json#/~1items"
   },

api/v1/swagger/paths/holds.json

@@ -702,5 +702,96 @@
         }
       }
     }
+  },
+  "/holds/{hold_id}/pickup_location": {
+    "put": {
+      "x-mojo-to": "Holds#update_pickup_location",
+      "operationId": "updateHoldPickupLocation",
+      "tags": ["holds"],
+      "parameters": [
+        {
+          "$ref": "../parameters.json#/hold_id_pp"
+        },
+        {
+          "name": "body",
+          "in": "body",
+          "description": "Pickup location",
+          "required": true,
+          "schema": {
+            "type": "object",
+            "properties": {
+              "pickup_library_id": {
+                "type": "string",
+                "description": "Internal identifier for the pickup library"
+              }
+            }
+          }
+        }
+      ],
+      "produces": [
+        "application/json"
+      ],
+      "responses": {
+        "200": {
+          "description": "The new pickup location value for the hold",
+          "schema": {
+            "type": "object",
+            "properties": {
+              "pickup_library_id": {
+                "type": "string",
+                "description": "Internal identifier for the pickup library"
+              }
+            }
+          }
+        },
+        "400": {
+          "description": "Missing or wrong parameters",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "401": {
+          "description": "Authentication required",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "403": {
+          "description": "Access forbidden",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "404": {
+          "description": "Hold not found",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "409": {
+          "description": "Unable to perform action on hold",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "500": {
+          "description": "Internal error",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "503": {
+          "description": "Under maintenance",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        }
+      },
+      "x-koha-authorization": {
+        "permissions": {
+          "reserveforothers": "place_holds"
+        }
+      }
+    }
   }
 }

koha-tmpl/intranet-tmpl/prog/en/modules/admin/authtypes.tt

@@ -103,8 +103,9 @@
                             <input type="hidden" name="checked" value="0" />
                             <input type="hidden" name="authtypecode" value="[% authority_type.authtypecode | html %]" />[% authority_type.authtypecode | html %]
                     [% ELSE %]
+                            <div class="hint">10 characters maximum</div>
                             <label for="authtypecode" class="required">Authority type: </label>
-                            <input id="authtypecode" type="text" class="required" required="required" name="authtypecode" size="10" maxlength="10" />
+                            <input id="authtypecode" type="text" class="required" required="required" name="authtypecode" size="20" maxlength="10" />
                             <span class="required">Required</span>
                     [% END %]
                 </li>

koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt

@@ -55,9 +55,9 @@
             <li><label for="job_status">Status: </label>[% job.status | html %]</li>
             <li><label for="job_progress">Progress: </label>[% job.progress || 0 | html %] / [% job.size | html %]</li>
             <li><label for="job_type">Type: </label>[% job.type | html %]</li>
-            <li><label for="job_enqueued_on">Enqueued on: </label>[% job.enqueued_on | html %]</li>
-            <li><label for="job_started_on">Started on: </label>[% job.started_on | html %]</li>
-            <li><label for="job_ended_on">Ended on: </label>[% job.ended_on | html %]</li>
+            <li><label for="job_enqueued_on">Queued: </label>[% job.enqueued_on | html %]</li>
+            <li><label for="job_started_on">Started: </label>[% job.started_on | html %]</li>
+            <li><label for="job_ended_on">Ended: </label>[% job.ended_on | html %]</li>
             <li><label for="job_data">Report: </label>
                 [% SWITCH job.type %]
                 [% CASE 'batch_biblio_record_modification' %]
@@ -164,9 +164,9 @@
                     <th>Status</th>
                     <th>Progress</th>
                     <th>Type</th>
-                    <th>Enqueued on</th>
-                    <th>Started on</th>
-                    <th>Ended on</th>
+                    <th>Queued</th>
+                    <th>Started</th>
+                    <th>Ended</th>
                     <th class="noExport">Actions</th>
                 </tr>
             </thead>

koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref

@@ -262,7 +262,7 @@ Circulation:
                   0: "Don't use"
             - the transport cost matrix for calculating optimal holds filling between libraries.
         -
-            - Use the checkout and fines rules of
+            - Use the calendar and circulation rules of
             - pref: CircControl
               type: choice
               choices:

koha-tmpl/intranet-tmpl/prog/en/modules/authorities/searchresultlist.tt

@@ -56,7 +56,7 @@
     [% FOREACH resul IN result %]
     <tr data-authid="[% resul.authid | html %]">
       <td>[% PROCESS authresult summary=resul.summary authid=resul.authid %]</td>
-      <td>[% resul.summary.label | html %]</td>
+      <td>[% resul.authtype | html %]</td>
     [% UNLESS ( resul.isEDITORS ) %]
       <td>
       [% IF resul.used > 0 %]

koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt

@@ -523,6 +523,24 @@
                         },{
                             'name': 'template_path',
                             'value': 'virtualshelves/tables/shelves_results.tt',
+                        },{
+                            'name': 'shelfname_sorton',
+                            'value': 'vs.shelfname',
+                        },{
+                            'name': 'is_shared_sorton',
+                            'value': 'vs.category',
+                        },{
+                            'name': 'owner_sorton',
+                            'value': 'vs.owner',
+                        },{
+                            'name': 'sortby_sorton',
+                            'value': 'vs.sortfield',
+                        },{
+                            'name': 'created_on_sorton',
+                            'value': 'vs.created_on',
+                        },{
+                            'name': 'modification_time_sorton',
+                            'value': 'vs.lastmodified',
                         });
                         $.ajax({
                             'dataType': 'json',

koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl

@@ -712,14 +712,14 @@
                                 <xsl:attribute name="href">/cgi-bin/koha/catalogue/search.pl?q=<xsl:call-template name="subfieldSelect">
                                     <xsl:with-param name="codes">avxyz</xsl:with-param>
                                     <xsl:with-param name="delimeter"> AND </xsl:with-param>
-                                    <xsl:with-param name="prefix">(su<xsl:value-of select="$SubjectModifier"/>:<xsl:value-of select="$TracingQuotesLeft"/></xsl:with-param>
+                                    <xsl:with-param name="prefix">(index-term-genre<xsl:value-of select="$SubjectModifier"/>:<xsl:value-of select="$TracingQuotesLeft"/></xsl:with-param>
                                     <xsl:with-param name="suffix"><xsl:value-of select="$TracingQuotesRight"/>)</xsl:with-param>
                                     <xsl:with-param name="urlencode">1</xsl:with-param>
                                 </xsl:call-template>
                                 </xsl:attribute>
                             </xsl:when>
                             <xsl:otherwise>
-                                <xsl:attribute name="href">/cgi-bin/koha/catalogue/search.pl?q=su<xsl:value-of select="$SubjectModifier"/>:<xsl:value-of select="$TracingQuotesLeft"/><xsl:value-of select="marc:subfield[@code='a']"/><xsl:value-of select="$TracingQuotesRight"/></xsl:attribute>
+                                <xsl:attribute name="href">/cgi-bin/koha/catalogue/search.pl?q=index-term-genre<xsl:value-of select="$SubjectModifier"/>:<xsl:value-of select="$TracingQuotesLeft"/><xsl:value-of select="marc:subfield[@code='a']"/><xsl:value-of select="$TracingQuotesRight"/></xsl:attribute>
                             </xsl:otherwise>
                         </xsl:choose>
                     <xsl:call-template name="subfieldSelect">

koha-tmpl/intranet-tmpl/prog/js/holds.js

@@ -224,12 +224,11 @@ $(document).ready(function() {
                     var cur_select = $(this);
                     var res_id = $(this).attr('reserve_id');
                     $(this).after('<div id="updating_reserveno'+res_id+'" class="waiting"><img src="/intranet-tmpl/prog/img/spinner-small.gif" alt="" /><span class="waiting_msg"></span></div>');
-                    var api_url = '/api/v1/holds/'+res_id;
-                    var update_info = JSON.stringify({ pickup_library_id: $(this).val(), priority: parseInt($(this).attr("priority"),10) });
+                    var api_url = '/api/v1/holds/' + encodeURIComponent(res_id) + '/pickup_location';
                     $.ajax({
                         method: "PUT",
                         url: api_url,
-                        data: update_info ,
+                        data: JSON.stringify({ "pickup_library_id": $(this).val() }),
                         success: function( data ){ holdsTable.api().ajax.reload(); },
                         error: function( jqXHR, textStatus, errorThrown) {
                             alert('There was an error:'+textStatus+" "+errorThrown);

koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt

@@ -180,6 +180,7 @@
                                         </div>
                                         [% UNLESS tag %]
                                             <div class="sort_by col-sm-auto">
+                                                <label for="sort_by" class="sr-only">Sort by:</label>
                                                 <select id="sort_by" class="resort form-control form-control-sm" name="sort_by">
                                                     [% INCLUDE 'resort_form.inc' %]
                                                 </select>

koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACDetail.xsl

@@ -786,14 +786,14 @@
                                     <xsl:attribute name="href">/cgi-bin/koha/opac-search.pl?q=<xsl:call-template name="subfieldSelectSubject">
                                         <xsl:with-param name="codes">avxyz</xsl:with-param>
                                         <xsl:with-param name="delimeter"> AND </xsl:with-param>
-                                        <xsl:with-param name="prefix">(su<xsl:value-of select="$SubjectModifier"/>:<xsl:value-of select="$TracingQuotesLeft"/></xsl:with-param>
+                                        <xsl:with-param name="prefix">(index-term-genre<xsl:value-of select="$SubjectModifier"/>:<xsl:value-of select="$TracingQuotesLeft"/></xsl:with-param>
                                         <xsl:with-param name="suffix"><xsl:value-of select="$TracingQuotesRight"/>)</xsl:with-param>
                                         <xsl:with-param name="urlencode">1</xsl:with-param>
                                     </xsl:call-template>
                                     </xsl:attribute>
                                 </xsl:when>
                                 <xsl:otherwise>
-                                    <xsl:attribute name="href">/cgi-bin/koha/opac-search.pl?q=su<xsl:value-of select="$SubjectModifier"/>:<xsl:value-of select="$TracingQuotesLeft"/><xsl:value-of select="str:encode-uri(marc:subfield[@code='a'], true())"/><xsl:value-of select="$TracingQuotesRight"/></xsl:attribute>
+                                    <xsl:attribute name="href">/cgi-bin/koha/opac-search.pl?q=index-term-genre<xsl:value-of select="$SubjectModifier"/>:<xsl:value-of select="$TracingQuotesLeft"/><xsl:value-of select="str:encode-uri(marc:subfield[@code='a'], true())"/><xsl:value-of select="$TracingQuotesRight"/></xsl:attribute>
                                 </xsl:otherwise>
                             </xsl:choose>
                         <xsl:call-template name="subfieldSelect">

misc/cronjobs/longoverdue.pl

@@ -270,7 +270,7 @@ if ( ! defined($lost) ) {
     else {
         pod2usage( {
                 -exitval => 1,
-                -msg => q|ERROR: No --lost (-l) option defined|,
+                -msg => q|ERROR: No --lost (-l) option or system preferences DefaultLongOverdueLostValue/DefaultLongOverdueDays defined|,
         } );
     }
 }

t/db_dependent/Koha/Account/Line.t

@@ -441,15 +441,15 @@ subtest 'Renewal related tests' => sub {
         interface         => 'commandline',
     })->store;
 
-    is( $line->renewable, 1, "Item is returned as renewable when it meets the conditions" );
+    is( $line->is_renewable, 1, "Item is returned as renewable when it meets the conditions" );
     $line->amountoutstanding(5);
-    is( $line->renewable, 0, "Item is returned as unrenewable when it has outstanding fine" );
+    is( $line->is_renewable, 0, "Item is returned as unrenewable when it has outstanding fine" );
     $line->amountoutstanding(0);
     $line->debit_type_code("VOID");
-    is( $line->renewable, 0, "Item is returned as unrenewable when it has the wrong account type" );
+    is( $line->is_renewable, 0, "Item is returned as unrenewable when it has the wrong account type" );
     $line->debit_type_code("OVERDUE");
     $line->status("RETURNED");
-    is( $line->renewable, 0, "Item is returned as unrenewable when it has the wrong account status" );
+    is( $line->is_renewable, 0, "Item is returned as unrenewable when it has the wrong account status" );
 
 
     t::lib::Mocks::mock_preference( 'RenewAccruingItemWhenPaid', 0 );

t/db_dependent/api/v1/holds.t

@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 12;
+use Test::More tests => 13;
 use Test::MockModule;
 use Test::Mojo;
 use t::lib::TestBuilder;
@@ -1054,3 +1054,126 @@ subtest 'add() tests' => sub {
 
     $schema->storage->txn_rollback;
 };
+
+subtest 'PUT /holds/{hold_id}/pickup_location tests' => sub {
+
+    plan tests => 16;
+
+    $schema->storage->txn_begin;
+
+    my $password = 'AbcdEFG123';
+
+    my $library_1 = $builder->build_object({ class => 'Koha::Libraries' });
+    my $library_2 = $builder->build_object({ class => 'Koha::Libraries' });
+    my $library_3 = $builder->build_object({ class => 'Koha::Libraries' });
+
+    my $patron = $builder->build_object(
+        { class => 'Koha::Patrons', value => { flags => 0 } } );
+    $patron->set_password( { password => $password, skip_validation => 1 } );
+    my $userid = $patron->userid;
+    $builder->build(
+        {
+            source => 'UserPermission',
+            value  => {
+                borrowernumber => $patron->borrowernumber,
+                module_bit     => 6,
+                code           => 'place_holds',
+            },
+        }
+    );
+
+    # Disable logging
+    t::lib::Mocks::mock_preference( 'HoldsLog',      0 );
+    t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 );
+
+    my $libraries_query = { branchcode => [ $library_1->branchcode, $library_2->branchcode ] };
+
+    my $mocked_biblio = Test::MockModule->new('Koha::Biblio');
+    $mocked_biblio->mock( 'pickup_locations', sub {
+        return Koha::Libraries->search($libraries_query);
+    });
+
+    my $mocked_item = Test::MockModule->new('Koha::Item');
+    $mocked_item->mock( 'pickup_locations', sub {
+        return Koha::Libraries->search($libraries_query);
+    });
+
+    my $biblio = $builder->build_sample_biblio;
+    my $item   = $builder->build_sample_item(
+        {
+            biblionumber => $biblio->biblionumber,
+            library      => $library_1->branchcode
+        }
+    );
+
+    # biblio-level hold
+    my $hold = Koha::Holds->find(
+        AddReserve(
+            {
+                branchcode     => $library_1->branchcode,
+                borrowernumber => $patron->borrowernumber,
+                biblionumber   => $biblio->biblionumber,
+                priority       => 1,
+                itemnumber     => undef,
+            }
+        )
+    );
+
+    $t->put_ok( "//$userid:$password@/api/v1/holds/"
+          . $hold->id
+          . "/pickup_location" => json => { pickup_library_id => $library_2->branchcode } )
+      ->status_is(200)
+      ->json_is({ pickup_library_id => $library_2->branchcode });
+
+    is( $hold->discard_changes->branchcode->branchcode, $library_2->branchcode, 'pickup library adjusted correctly' );
+
+    $libraries_query = { branchcode => $library_1->branchcode };
+
+    $t->put_ok( "//$userid:$password@/api/v1/holds/"
+          . $hold->id
+          . "/pickup_location" => json => { pickup_library_id => $library_3->branchcode } )
+      ->status_is(400)
+      ->json_is({ error => '[The supplied pickup location is not valid]' });
+
+    is( $hold->discard_changes->branchcode->branchcode, $library_2->branchcode, 'pickup library unchanged' );
+
+    # item-level hold
+    $hold = Koha::Holds->find(
+        AddReserve(
+            {
+                branchcode     => $library_1->branchcode,
+                borrowernumber => $patron->borrowernumber,
+                biblionumber   => $biblio->biblionumber,
+                priority       => 1,
+                itemnumber     => $item->itemnumber,
+            }
+        )
+    );
+
+    $libraries_query = { branchcode => $library_1->branchcode };
+
+    # Attempt to use an invalid pickup locations ends in 400
+    $t->put_ok( "//$userid:$password@/api/v1/holds/"
+          . $hold->id
+          . "/pickup_location" => json => { pickup_library_id => $library_2->branchcode } )
+      ->status_is(400)
+      ->json_is({ error => '[The supplied pickup location is not valid]' });
+
+    is( $hold->discard_changes->branchcode->branchcode, $library_1->branchcode, 'pickup library unchanged' );
+
+    $libraries_query = {
+        branchcode => {
+            '-in' => [ $library_1->branchcode, $library_2->branchcode ]
+        }
+    };
+
+    $t->put_ok( "//$userid:$password@/api/v1/holds/"
+          . $hold->id
+          . "/pickup_location" => json => { pickup_library_id => $library_2->branchcode } )
+      ->status_is(200)
+      ->json_is({ pickup_library_id => $library_2->branchcode });
+
+    is( $hold->discard_changes->branchcode->branchcode, $library_2->branchcode, 'pickup library adjusted correctly' );
+
+    $schema->storage->txn_rollback;
+};