From 312e773d65624f37bc9597df2db471c3f7877127 Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Fri, 29 Nov 2019 10:57:18 +0000
Subject: [PATCH] Bug 18355: (QA follow-up) Rearrange comments, improve code

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>
---
 .../prog/en/modules/catalogue/detail.tt       | 21 +++++++++----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
index 9b2d57dfd7..a518fb1c4c 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
@@ -267,19 +267,18 @@
                     <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>
-- 
2.39.5