Bug 26999

Summary: "Any library" not translatable on the hold list
Product: Koha Reporter: Jonathan Druart <jonathan.druart+koha>
Component: Hold requestsAssignee: Tomás Cohen Arazi <tomascohen>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: andrew, fridolin.somers, gmcharlt, martin.renvoize, tomascohen, victor
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.05,20.05.11
Bug Depends on: 22284    
Bug Blocks:    
Attachments: Bug 26999: Make 'Any library' translatable when placing a hold
Bug 26999: (follow-up) Simplify code
Bug 26999: Make 'Any library' translatable when placing a hold
Bug 26999: (follow-up) Simplify code
Bug 26999: Make 'Any library' translatable when placing a hold
Bug 26999: (follow-up) Simplify code

Description Jonathan Druart 2020-11-11 13:45:05 UTC
https://snipboard.io/JvQant.jpg

It's hardcoded in the controller:
reserve/request.pl
571                         $item->{pickup_locations} = 'Any library';

reserve/request.tt
 684                                                         [% itemloo.pickup_locations | html %]
Comment 1 Tomás Cohen Arazi 2021-03-15 11:40:37 UTC
Created attachment 118228 [details] [review]
Bug 26999: Make 'Any library' translatable when placing a hold

This patch makes the 'Any library' string translatable, by converting it
into a flag and using it accordingly on the request.tt template.

To test:
1. Have the 'Hold pickup library match' set to 'any library' on the
   circultation rules.
2. Open the page to place a hold on a biblio with some items
=> SUCCESS: The item says 'Any library' on the 'Allowed pickup
locations' column.
3. Apply this patch
4. Repeat 2
=> SUCCESS: No behavior change
=> SUCCESS: The string is on the template
5. Sign off :-D
Comment 2 Tomás Cohen Arazi 2021-03-15 11:40:43 UTC
Created attachment 118230 [details] [review]
Bug 26999: (follow-up) Simplify code

Doing
$ git grep pickup_locations_code

shows there's some calculated data that is not actually used anywhere.
We can get rid of it.

This patch also reuses $item_object (which is in the same loop scope) to
avoid an extra DB call.

To test:
1. Run:
   $ git grep pickup_locations_code
=> FAIL: It is only used/set as a comma separated string, inside
request.pl
2. Apply this patch
3. Repeat 1
=> SUCCESS: The unused stuff is not there anymore
4. Open the page for placing some holds
=> SUCCESS: It doesn't explode
5. Sign off :-D
Comment 3 Owen Leonard 2021-03-15 14:11:23 UTC
Created attachment 118233 [details] [review]
Bug 26999: Make 'Any library' translatable when placing a hold

This patch makes the 'Any library' string translatable, by converting it
into a flag and using it accordingly on the request.tt template.

To test:
1. Have the 'Hold pickup library match' set to 'any library' on the
   circultation rules.
2. Open the page to place a hold on a biblio with some items
=> SUCCESS: The item says 'Any library' on the 'Allowed pickup
locations' column.
3. Apply this patch
4. Repeat 2
=> SUCCESS: No behavior change
=> SUCCESS: The string is on the template
5. Sign off :-D

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Owen Leonard 2021-03-15 14:11:26 UTC
Created attachment 118234 [details] [review]
Bug 26999: (follow-up) Simplify code

Doing
$ git grep pickup_locations_code

shows there's some calculated data that is not actually used anywhere.
We can get rid of it.

This patch also reuses $item_object (which is in the same loop scope) to
avoid an extra DB call.

To test:
1. Run:
   $ git grep pickup_locations_code
=> FAIL: It is only used/set as a comma separated string, inside
request.pl
2. Apply this patch
3. Repeat 1
=> SUCCESS: The unused stuff is not there anymore
4. Open the page for placing some holds
=> SUCCESS: It doesn't explode
5. Sign off :-D

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 5 Martin Renvoize 2021-03-25 14:39:43 UTC
Created attachment 118814 [details] [review]
Bug 26999: Make 'Any library' translatable when placing a hold

This patch makes the 'Any library' string translatable, by converting it
into a flag and using it accordingly on the request.tt template.

To test:
1. Have the 'Hold pickup library match' set to 'any library' on the
   circultation rules.
2. Open the page to place a hold on a biblio with some items
=> SUCCESS: The item says 'Any library' on the 'Allowed pickup
locations' column.
3. Apply this patch
4. Repeat 2
=> SUCCESS: No behavior change
=> SUCCESS: The string is on the template
5. Sign off :-D

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2021-03-25 14:39:46 UTC
Created attachment 118815 [details] [review]
Bug 26999: (follow-up) Simplify code

Doing
$ git grep pickup_locations_code

shows there's some calculated data that is not actually used anywhere.
We can get rid of it.

This patch also reuses $item_object (which is in the same loop scope) to
avoid an extra DB call.

To test:
1. Run:
   $ git grep pickup_locations_code
=> FAIL: It is only used/set as a comma separated string, inside
request.pl
2. Apply this patch
3. Repeat 1
=> SUCCESS: The unused stuff is not there anymore
4. Open the page for placing some holds
=> SUCCESS: It doesn't explode
5. Sign off :-D

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2021-03-25 14:40:17 UTC
Works as expected, nice bit of cleanup, no regressions.

Passing QA
Comment 8 Jonathan Druart 2021-04-01 16:53:52 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 9 Fridolin Somers 2021-04-13 15:03:58 UTC
Pushed to 20.11.x for 20.11.05
Comment 10 Andrew Fuerste-Henry 2021-04-24 14:49:53 UTC
Pushed to 20.05.x for 20.05.11
Comment 11 Victor Grousset/tuxayo 2021-04-24 17:11:41 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.