Bug 22021: Improve item status display when placing holds in staff
authorKatrin Fischer <katrin.fischer.83@web.de>
Sat, 13 Jul 2019 12:23:19 +0000 (14:23 +0200)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Fri, 19 Jul 2019 12:55:53 +0000 (13:55 +0100)
This patch is a bit of a clean-up to bring the item status display
more in line with the display on the detail page:

- show descrpition of authorised value for not for loan instead of hardcoded text
- Show description of authorised value for lost instead of hardcoded text
  This is also a translatability fix, as the text came from the .pl
- Show description of authorised value damaged instead of hardcoded text
- Make sequence of status match display on details page:
  lost - damaged - not for loan

To test:
- On a record with multiple items
  - Add different status to the items
    damaged, lost, not for loan
  - Make sure you have items with one status and multiple status at the same time
- Look at how the status display on the detail page
- Place a hold, compare display
- Apply patch
- Repeat

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

koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
reserve/request.pl

index f11f0cf..5a96a1f 100644 (file)
                     </td>
                 [% END %]
                     <td>
-                [% IF ( itemloo.onloan ) %]
-                    <span title="[% itemloo.date_due | html %]" class="checkedout">Due [% itemloo.date_due | $KohaDates  as_due_date => 1 %]</span>
-                [% ELSE %]
-                    <span title="0000-00-00">
-                        [% IF ( itemloo.transfertwhen ) %]
-                            In transit from [% Branches.GetName( itemloo.transfertfrom ) | html %],
-                            to [% Branches.GetName( itemloo.transfertto ) | html %], since [% itemloo.transfertwhen | html %]
+                        [% IF ( itemloo.onloan ) %]
+                            <span title="[% itemloo.date_due | html %]" class="checkedout">Due [% itemloo.date_due | $KohaDates  as_due_date => 1 %]</span>
+                        [% ELSE %]
+                            <span title="0000-00-00">
+                                [% IF ( itemloo.transfertwhen ) %]
+                                    In transit from [% Branches.GetName( itemloo.transfertfrom ) | html %],
+                                    to [% Branches.GetName( itemloo.transfertto ) | html %], since [% itemloo.transfertwhen | html %]
+                                [% END %]
+                            </span>
                         [% END %]
-                    </span>
-                [% END %]
-                [% IF ( itemloo.message ) %]
-                   Unavailable (lost or missing)
-                [% END %]
 
-                [% IF ( itemloo.notforloan ) %]
-                   Not for loan ([% itemloo.notforloanvalue | html %])
-                [% END %]
+                        [% IF ( itemloo.reservedate ) %]
+                            [% IF ( itemloo.nocancel ) %]
+                                    Can't be cancelled when item is in transit
+                            [% ELSE %]
+                            [% IF ( itemloo.waitingdate ) %]Waiting[% ELSE %]On hold[% END %]
+                            [% IF ( itemloo.canreservefromotherbranches ) %]for <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% itemloo.ReservedFor.borrowernumber | uri %]">[% itemloo.ReservedFor.firstname | html %] [% itemloo.ReservedFor.surname | html %]</a>[% END %] [% IF ( itemloo.waitingdate ) %]at[% ELSE %]expected at[% END %] [% Branches.GetName( itemloo.ExpectedAtLibrary ) | html %]
+                            since
+                            [% IF ( itemloo.waitingdate ) %][% itemloo.waitingdate | $KohaDates %][% ELSE %][% IF ( itemloo.reservedate ) %][% itemloo.reservedate | html %][% END %][% END %]. <a class="info" href="modrequest.pl?CancelBiblioNumber=[% itemloo.biblionumber | html %]&amp;CancelBorrowerNumber=[% itemloo.ReservedFor.borrowernumber | html %]&amp;CancelItemnumber=[% itemloo.itemnumber | html %]"  onclick="return confirmDelete(MSG_CONFIRM_DELETE_HOLD);">Cancel hold</a>
+                            [% END %]
+                        [% ELSE %]
+                            Not on hold
+                        [% END %]
 
-                [% IF ( itemloo.reservedate ) %]
-                    [% IF ( itemloo.nocancel ) %]
-                            Can't be cancelled when item is in transit
-                    [% ELSE %]
-                    [% IF ( itemloo.waitingdate ) %]Waiting[% ELSE %]On hold[% END %]
-                    [% IF ( itemloo.canreservefromotherbranches ) %]for <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% itemloo.ReservedFor.borrowernumber | uri %]">[% itemloo.ReservedFor.firstname | html %] [% itemloo.ReservedFor.surname | html %]</a>[% END %] [% IF ( itemloo.waitingdate ) %]at[% ELSE %]expected at[% END %] [% Branches.GetName( itemloo.ExpectedAtLibrary ) | html %]
-                    since
-                    [% IF ( itemloo.waitingdate ) %][% itemloo.waitingdate | $KohaDates %][% ELSE %][% IF ( itemloo.reservedate ) %][% itemloo.reservedate | html %][% END %][% END %]. <a class="info" href="modrequest.pl?CancelBiblioNumber=[% itemloo.biblionumber | html %]&amp;CancelBorrowerNumber=[% itemloo.ReservedFor.borrowernumber | html %]&amp;CancelItemnumber=[% itemloo.itemnumber | html %]"  onclick="return confirmDelete(MSG_CONFIRM_DELETE_HOLD);">Cancel hold</a>
+                        [% IF itemloo.item_level_holds == "N" %]
+                            <br/>Item level hold not allowed from OPAC
+                        [% ELSIF itemloo.item_level_holds == "F" %]
+                            <br/>Item level hold forced from OPAC
+                        [% END %]
 
-                    [% END %]
-                [% ELSE %]
-                    Not on hold
-                [% END %]
+                        [% IF ( itemloo.itemlost ) %]
+                           <span class="lost">[% AuthorisedValues.GetByCode( 'LOST', itemloo.itemlost ) | html %]</span>
+                        [% END %]
 
-                [% IF itemloo.item_level_holds == "N" %]
-                    <br/>Item level hold not allowed from OPAC
-                [% ELSIF itemloo.item_level_holds == "F" %]
-                    <br/>Item level hold forced from OPAC
-                [% END %]
-                [% IF ( itemloo.damaged ) %]
-                    <br />Damaged
-                [% END %]
+                        [% IF ( itemloo.damaged ) %]
+                            <span class="dmg">[% AuthorisedValues.GetByCode( 'DAMAGED', itemloo.damaged ) | html %]</span>
+                        [% END %]
+
+                        [% IF ( itemloo.notforloan ) %]
+                           <span class="nfl">Not for loan ([% AuthorisedValues.GetByCode( 'NOT_LOAN', itemloo.notforloan ) | html %])</span>
+                        [% END %]
                     </td>
                 </tr>
             [% END %] <!--UNLESS item hide-->
index c709cec..8936808 100755 (executable)
@@ -346,10 +346,6 @@ foreach my $biblionumber (@biblionumbers) {
           }
     };
 
-    my $frameworkcode = GetFrameworkCode( $biblionumber );
-    my @notforloan_avs = Koha::AuthorisedValues->search_by_koha_field({ kohafield => 'items.notforloan', frameworkcode => $frameworkcode });
-    my $notforloan_label_of = { map { $_->authorised_value => $_->lib } @notforloan_avs };
-
     my @bibitemloop;
 
     my @available_itemtypes;
@@ -436,18 +432,10 @@ foreach my $biblionumber (@biblionumbers) {
             # Management of the notforloan document
             if ( $item->{notforloan} ) {
                 $item->{backgroundcolor} = 'other';
-                $item->{notforloanvalue} =
-                  $notforloan_label_of->{ $item->{notforloan} };
             }
 
             # Management of lost or long overdue items
             if ( $item->{itemlost} ) {
-
-                # FIXME localized strings should never be in Perl code
-                $item->{message} =
-                  $item->{itemlost} == 1 ? "(lost)"
-                    : $item->{itemlost} == 2 ? "(long overdue)"
-                      : "";
                 $item->{backgroundcolor} = 'other';
                 if ($logged_in_patron->category->hidelostitems && !$showallitems) {
                     $item->{hide} = 1;