Bug 18355: (QA follow-up) Rearrange comments, improve code
authorMarcel de Rooy <m.de.rooy@rijksmuseum.nl>
Fri, 29 Nov 2019 10:57:18 +0000 (10:57 +0000)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Mon, 13 Jan 2020 13:58:05 +0000 (13:58 +0000)
Comments were hard to read (for me) but tastes differ..

Code should reflect that permanent_location is a code and that location
may be already an authval. See detail.pl:
$item->{'location'} = $shelflocations->{$shelfcode} if ( defined( $shelfcode ) && defined($shelflocations) && exists( $shelflocations->{$shelfcode} ) );

Obviously, this kind of logic divided over two places should be reduced.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt

index 9b2d57d..a518fb1 100644 (file)
                     <td class="homebranch">
                         [% Branches.GetName(item.homebranch) | html %]
                         <span class="shelvingloc">
-                            [%# 1 - If permanent location is defined %]
-                            [%#   a - display the description if available, display the code if not %]
-                            [%#   b - display the current location in parens %]
-                            [%# 2 - If permanent location is not defined, but location is defined %]
-                            [%#   a - display the current location description if available, display the code if not %]
-                            [%# 3 - If neither are defined, show nothing %]
+<!--
+If permanent location is defined, show description or code and display current location in parentheses. If not, display current location.
+Note that permanent location is a code, and location may be an authval.
+-->
                             [% IF item.permanent_location %]
-                                [% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.location', authorised_value => item.permanent_location ) | html %]
-                                [% IF item.location AND item.location != item.permanent_location %]
-                                    ([% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.location', authorised_value => item.location ) | html %])
+                                [% SET permloc_authval = AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.location', authorised_value => item.permanent_location ) %]
+                                [% permloc_authval | html %]
+                                [% IF item.location AND item.location != permloc_authval AND item.location != item.permanent_location %]
+                                    ([% item.location | html %])
                                 [% END %]
-                            [% ELSIF item.location %]
-                                [% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.location', authorised_value => item.permanent_location ) | html %]
+                            [% ELSE %]
+                                [% item.location | html %]
                             [% END %]
                         </span>
                     </td>