Bug 30352 - "Not for loan" in result list doesn't translate in OPAC
Summary: "Not for loan" in result list doesn't translate in OPAC
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Marcel de Rooy
QA Contact: Katrin Fischer
URL:
Keywords:
: 32008 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-03-24 14:33 UTC by Katrin Fischer
Modified: 2023-12-28 20:47 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.06,22.05.13
Circulation function:


Attachments
Bug 30352: Allow translation of Not for loan in OPAC XSLT results (7.82 KB, patch)
2023-04-20 06:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30352: Add a comment to the substatus translation (3.92 KB, patch)
2023-04-20 06:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30352: Allow translation of Not for loan in OPAC XSLT results (7.83 KB, patch)
2023-04-22 22:39 UTC, David Nind
Details | Diff | Splinter Review
Bug 30352: Add a comment to the substatus translation (3.93 KB, patch)
2023-04-22 22:39 UTC, David Nind
Details | Diff | Splinter Review
Bug 30352: Allow translation of Not for loan in OPAC XSLT results (7.88 KB, patch)
2023-04-23 10:26 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 30352: Add a comment to the substatus translation (3.98 KB, patch)
2023-04-23 10:26 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2022-03-24 14:33:50 UTC
When there are items marked es "Not for loan" by the item type in the result list in OPAC, the "Not for loan" text doesn't translate.
Comment 1 Katrin Fischer 2022-06-19 15:01:29 UTC
The problem is somewhere around here:

https://git.koha-community.org/Koha-community/Koha/src/branch/master/C4/XSLT.pm#L396

I think it was assumed that you'd usually have a description stemming from the authorised value, but this is not the case when the itemtype determines that the item is Not for loan. We need to have this string translatable.

I am currently out of ideas on how to fix this :(
Comment 2 Katrin Fischer 2022-10-27 09:07:47 UTC
*** Bug 32008 has been marked as a duplicate of this bug. ***
Comment 3 Katrin Fischer 2023-04-05 15:20:49 UTC
This got worse with the rewrites/changes in the newer versions. The string "Not for loan" is no longer in its own HTML element, so it's not longer a simple search and replace by jQuery as a workaround. It's now much more difficult as it has the library name in the same element.

<span class="unavailable_Not for loan"><span class="ItemBranch">Bibliothek X: Not for loan</span> (1). </span>

The problem is here:
https://git.koha-community.org/Koha-community/Koha/src/branch/master/C4/XSLT.pm#L339

Can please someone help to fix this?
Comment 4 Marcel de Rooy 2023-04-06 05:43:26 UTC
Will have a look here soon.
Comment 5 Katrin Fischer 2023-04-06 08:33:53 UTC
Thanks Marcel, I tried, but I have run out of ideas and the code is a little complex.
Comment 6 Marcel de Rooy 2023-04-19 12:49:13 UTC
Looking here now
Comment 7 Marcel de Rooy 2023-04-19 14:26:53 UTC
Bumping here again on the lack of translated auth values ..
Comment 8 Katrin Fischer 2023-04-19 14:28:06 UTC
(In reply to Marcel de Rooy from comment #7)
> Bumping here again on the lack of translated auth values ..

But this is not coming from an authorised value in that case... It's the fallback text for the itemtype level setting that's failing.
Comment 9 Marcel de Rooy 2023-04-19 14:33:49 UTC
(In reply to Katrin Fischer from comment #8)
> (In reply to Marcel de Rooy from comment #7)
> > Bumping here again on the lack of translated auth values ..
> 
> But this is not coming from an authorised value in that case... It's the
> fallback text for the itemtype level setting that's failing.

Sure. But it's the same code or direct context in the .xsl
Comment 10 Katrin Fischer 2023-04-19 14:35:16 UTC
Cleared up on IRC: the item type value will be translated, but the AV values will not. With this fix we can still have at least all translated  one non-English language which often will help already.
Comment 11 Marcel de Rooy 2023-04-19 14:55:33 UTC
Example display
Availability: Items available for loan: Centerville (1)Call number: PERL A 1. Items available for reference: Centerville: NOT FOR LOAN (1)Call number: PERL A 4. Centerville: Staff collection (1)Call number: PERL A 5. Not available: Centerville: Damaged (1)Call number: PERL A 3. Centerville: Withdrawn (1)Call number: PERL A 2. 
# Toggling NFL statuses
Availability: Items available for loan: Centerville (1)Call number: PERL A 1. Items available for reference: Centerville: NOT FOR LOAN (1)Call number: PERL A 4. Not available: Centerville: Damaged (1)Call number: PERL A 3. Centerville: Staff collection (1)Call number: PERL A 5. Centerville: Withdrawn (1)Call number: PERL A 2. 
# Toggling NFL statuses
Availability: Items available for loan: Centerville (1)Call number: PERL A 1. Items available for reference: Centerville: Staff collection (1)Call number: PERL A 5. Not available: Centerville: Damaged (1)Call number: PERL A 3. Centerville: NOT FOR LOAN (1)Call number: PERL A 4. Centerville: Withdrawn (1)Call number: PERL A 2.
Comment 12 Marcel de Rooy 2023-04-19 14:56:11 UTC
Clarification: uc NOT FOR LOAN is the translation :)
Comment 13 Marcel de Rooy 2023-04-20 06:17:28 UTC
Created attachment 149919 [details] [review]
Bug 30352: Allow translation of Not for loan in OPAC XSLT results

Note: Test plan below can be followed in English. If you would like
to see the vernacular results, you need to update PO files and
translate 'Not for loan', and update templates (xslt files) accordingly.
The bootstrap PO does not yet include that string without %s suffix
in the current codebase.

Test plan:
[1] Apply this patch and make the following change in
MARC21slim2OPACResults.xsl, line 1207.
Change:
  <status english="Not for loan">Not for loan</status>
to:
  <status english="Not for loan">NOT for loan</status>
Important: Change text inside, not the attribute!
Restart all.

[2] Pick a biblio, add some items with different statuses: Checked out,
Damaged, Not for loan, Staff collection.
[3] Make it appear in a OPAC results display (not detail).
[4] You should see the different statuses, including NOT for loan. The
uppercase NOT serves to prove that the string has been translated. Also
verify that you see Staff collection as-is.
[5] Bonus: Toggle with values in Reference_NFL_Statuses to move results
from Reference section to Not available section v.v.
[6] Revert the change from step 1. Sign off :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2023-04-20 06:17:30 UTC
Created attachment 149920 [details] [review]
Bug 30352: Add a comment to the substatus translation

The tric of combining the XPath union operator (|) with the
value-of instruction works as a sort of ternary operation.
The value-of only picks the string value of the first node
from the union in document order.

So, if there is a translation, it will be the first node. If there
is no translation, the original substatus will be used. Note that
there will be no translation for authorised values like Staff
collection, etc.

Test plan:
None. Just comments.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 David Nind 2023-04-22 22:39:40 UTC
Created attachment 150107 [details] [review]
Bug 30352: Allow translation of Not for loan in OPAC XSLT results

Note: Test plan below can be followed in English. If you would like
to see the vernacular results, you need to update PO files and
translate 'Not for loan', and update templates (xslt files) accordingly.
The bootstrap PO does not yet include that string without %s suffix
in the current codebase.

Test plan:
[1] Apply this patch and make the following change in
MARC21slim2OPACResults.xsl, line 1207.
Change:
  <status english="Not for loan">Not for loan</status>
to:
  <status english="Not for loan">NOT for loan</status>
Important: Change text inside, not the attribute!
Restart all.

[2] Pick a biblio, add some items with different statuses: Checked out,
Damaged, Not for loan, Staff collection.
[3] Make it appear in a OPAC results display (not detail).
[4] You should see the different statuses, including NOT for loan. The
uppercase NOT serves to prove that the string has been translated. Also
verify that you see Staff collection as-is.
[5] Bonus: Toggle with values in Reference_NFL_Statuses to move results
from Reference section to Not available section v.v.
[6] Revert the change from step 1. Sign off :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Comment 16 David Nind 2023-04-22 22:39:43 UTC
Created attachment 150108 [details] [review]
Bug 30352: Add a comment to the substatus translation

The tric of combining the XPath union operator (|) with the
value-of instruction works as a sort of ternary operation.
The value-of only picks the string value of the first node
from the union in document order.

So, if there is a translation, it will be the first node. If there
is no translation, the original substatus will be used. Note that
there will be no translation for authorised values like Staff
collection, etc.

Test plan:
None. Just comments.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Comment 17 David Nind 2023-04-22 22:42:59 UTC
Testing notes (using KTD):

1. For step 1 of the test plan:
   . to edit file: vi koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACResults.xsl
  . line to edit for me was 1297

2. Second patch: I don't know enough to provide a useful comment, feel free to change the status back to needs signoff for someone else to comment if this is essential.
Comment 18 Katrin Fischer 2023-04-23 10:26:35 UTC
Created attachment 150121 [details] [review]
Bug 30352: Allow translation of Not for loan in OPAC XSLT results

Note: Test plan below can be followed in English. If you would like
to see the vernacular results, you need to update PO files and
translate 'Not for loan', and update templates (xslt files) accordingly.
The bootstrap PO does not yet include that string without %s suffix
in the current codebase.

Test plan:
[1] Apply this patch and make the following change in
MARC21slim2OPACResults.xsl, line 1207.
Change:
  <status english="Not for loan">Not for loan</status>
to:
  <status english="Not for loan">NOT for loan</status>
Important: Change text inside, not the attribute!
Restart all.

[2] Pick a biblio, add some items with different statuses: Checked out,
Damaged, Not for loan, Staff collection.
[3] Make it appear in a OPAC results display (not detail).
[4] You should see the different statuses, including NOT for loan. The
uppercase NOT serves to prove that the string has been translated. Also
verify that you see Staff collection as-is.
[5] Bonus: Toggle with values in Reference_NFL_Statuses to move results
from Reference section to Not available section v.v.
[6] Revert the change from step 1. Sign off :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 19 Katrin Fischer 2023-04-23 10:26:39 UTC
Created attachment 150122 [details] [review]
Bug 30352: Add a comment to the substatus translation

The tric of combining the XPath union operator (|) with the
value-of instruction works as a sort of ternary operation.
The value-of only picks the string value of the first node
from the union in document order.

So, if there is a translation, it will be the first node. If there
is no translation, the original substatus will be used. Note that
there will be no translation for authorised values like Staff
collection, etc.

Test plan:
None. Just comments.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Katrin Fischer 2023-04-23 10:27:51 UTC
Thanks Marcel.

I also verified that this is working by running the translation scripts and updating the po files. We now see a translated Not for loan according to selected language.
Comment 21 Marcel de Rooy 2023-04-24 06:51:42 UTC
(In reply to Katrin Fischer from comment #20)
> Thanks Marcel.
> 
> I also verified that this is working by running the translation scripts and
> updating the po files. We now see a translated Not for loan according to
> selected language.

Great, thx.
Comment 22 Tomás Cohen Arazi (tcohen) 2023-04-24 13:49:02 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 23 Martin Renvoize (ashimema) 2023-05-11 10:40:55 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release
Comment 24 Lucas Gass (lukeg) 2023-05-15 21:44:43 UTC
Backported to 22.05.x for upcoming 22.05.13
Comment 25 Tomás Cohen Arazi (tcohen) 2023-06-09 17:39:36 UTC
This was pushed to 21.11. And I think it broke search results display in 21.11.x.
Comment 26 Tomás Cohen Arazi (tcohen) 2023-06-09 18:29:51 UTC
I have now reverted this two patches on 21.11, to be released as 21.11.22.

If needed in 21.11, we can give it proper testing and make sure nothing is broken.