Bug 31141 - We can remove 'select_column' from waiting_holds.inc
Summary: We can remove 'select_column' from waiting_holds.inc
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-07-12 10:21 UTC by Martin Renvoize
Modified: 2023-06-08 22:26 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00, 22.05.05, 21.11.12


Attachments
Bug 31141: Template simplification (3.22 KB, patch)
2022-07-12 10:24 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31141: Template simplification (3.29 KB, patch)
2022-07-12 11:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 31141: Template simplification (3.34 KB, patch)
2022-07-12 12:13 UTC, David Nind
Details | Diff | Splinter Review
Bug 31141: Remove 'select_column' from waiting_holds.inc (3.36 KB, patch)
2022-07-12 12:41 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2022-07-12 10:21:17 UTC
We only call waiting_holds.inc from one template and in both cases we have hard coded the select_column variable to be passed as true.

As such, we can simplify and just remove the condition entirely.
Comment 1 Martin Renvoize 2022-07-12 10:24:14 UTC
Created attachment 137617 [details] [review]
Bug 31141: Template simplification

In the only place we use the waiting_holds include, we were hard coded
to always pass the 'select_column' variable as true.

We can simply and clarify the logic by just removing this superflous
variable.

Test plan
1. Confirm that the waiting holds tables continue to display as expected
Comment 2 Jonathan Druart 2022-07-12 11:38:31 UTC
Created attachment 137621 [details] [review]
Bug 31141: Template simplification

In the only place we use the waiting_holds include, we were hard coded
to always pass the 'select_column' variable as true.

We can simply and clarify the logic by just removing this superflous
variable.

Test plan
1. Confirm that the waiting holds tables continue to display as expected

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 3 Jonathan Druart 2022-07-12 11:38:56 UTC
I would suggest to improve the commit title to make it more explicit. Maybe "Remove 'select_column' from waiting_holds.inc"?
Comment 4 David Nind 2022-07-12 12:13:52 UTC
Created attachment 137623 [details] [review]
Bug 31141: Template simplification

In the only place we use the waiting_holds include, we were hard coded
to always pass the 'select_column' variable as true.

We can simply and clarify the logic by just removing this superflous
variable.

Test plan
1. Confirm that the waiting holds tables continue to display as expected

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2022-07-12 12:22:20 UTC
Testing notes (using koha-testing-docker):

1. Place some holds for several items.
2. Check items in and confirm holds.
3. Check the list of holds awaiting pickup (Circulation > Holds > Holds awaiting pickup)
4. After applying the patch, there should be no change to the table and things should work as expected.
Comment 6 Martin Renvoize 2022-07-12 12:41:28 UTC
Created attachment 137624 [details] [review]
Bug 31141: Remove 'select_column' from waiting_holds.inc

In the only place we use the waiting_holds include, we were hard coded
to always pass the 'select_column' variable as true.

We can simply and clarify the logic by just removing this superflous
variable.

Test plan
1. Confirm that the waiting holds tables continue to display as expected

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: David Nind <david@davidnind.com>
Comment 7 Martin Renvoize 2022-07-12 12:42:01 UTC
Good call on the commit title, updated.
Comment 8 Tomás Cohen Arazi 2022-07-13 15:23:09 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 9 Lucas Gass 2022-08-23 15:58:30 UTC
Backported to 22.05.x for 22.05.05
Comment 10 Arthur Suzuki 2022-08-30 09:17:11 UTC
thx pushed to 21.11.x for 21.11.12
Comment 11 Victor Grousset/tuxayo 2022-09-09 23:06:08 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document, marking resolved.