Bug 30068 - Wrong reference to table_borrowers in circulation.tt
Summary: Wrong reference to table_borrowers in circulation.tt
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 17353 21034
Blocks:
  Show dependency treegraph
 
Reported: 2022-02-10 10:20 UTC by Jonathan Druart
Modified: 2024-04-18 14:51 UTC (History)
4 users (show)

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


Attachments
Bug 30068: Remove table_settings_borrowers_table from circulation.tt (1.46 KB, patch)
2024-04-17 10:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30068: Remove table_settings_borrowers_table from circulation.tt (1.46 KB, patch)
2024-04-17 14:11 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
Bug 30068: Remove table_settings_borrowers_table from circulation.tt (1.53 KB, patch)
2024-04-17 14:11 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
Bug 30068: Remove table_settings_borrowers_table from circulation.tt (1.60 KB, patch)
2024-04-17 15:58 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 Jonathan Druart 2022-02-10 10:20:57 UTC
There is a reference to table_borrowers in the table settings yaml file, and this:
985         columns_settings_borrowers_table = [% TablesSettings.GetColumns( 'circ', 'circulation', 'table_borrowers', 'json' ) | $raw %]
in circulation.tt

But #table_borrowers is not a table displaying patrons, but items.

The confusion is coming from this commit that first introduced the id on this table:
  commit 7d598d9130c3d171af1e1a6a0a168cea043c277d
  Bug 21034: Re-indent circulation.tt

Then:
  commit f6599b2c5480d2e096bac71e4e7ac759686b9fbc
  Bug 17353: Add phone number column to checkout search
That added the columns settings line.
Comment 1 Nick Clemens 2022-02-10 15:00:17 UTC
olumns_settings_borrowers_table = [% TablesSettings.GetColumns( 'circ', 'circulation', 'table_borrowers', 'json' ) | $raw %]
This is just fetching the settings from the yaml, they are only applied to "#table_borrowers" in circ-patron-search-results.js

The table_borrowers that shows items is simply a class on the itemSearchFallBack table - this class is not referenced anywhere so can simply be removed
Comment 2 Caroline Cyr La Rose 2022-07-28 14:27:22 UTC
I'm not sure if this is the bug I'm looking for, but I think table_borrowers doesn't refer to any table anymore? It used to be the patron search when searching for a patron for a checkout, but now this search simply falls back to the regular patron search (table is memberresultst).

Just to make sure we're talking about the same thing:
1) In the main page of the staff interface, in the search bar at the top, with the tab 'Check out' selected, search for part of a name (e.g. "mar" should give several results with the sample patrons)
--> This table used to be table_borrowers, in the past, we could configure it under Administration > Table settings > Circulation
--> Note that the tab selected in the search bar at the top is now 'Search patrons'
2) In the main staff interface, in the search bar at the top, with the tab 'Search patrons' selected, search for part of a name
--> This table is memberresultst, we can configure it under Administration > Table settings > Patrons

Something was consolidated under 'Search patrons', which is fine, but I think the reference to that table under Administration > Table settings > Circulation was forgotten.
Comment 3 Caroline Cyr La Rose 2022-07-28 15:33:55 UTC
Oh I think I found where table_borrowers is used: when searching for a patron to place a hold. 
So ignore my previous comment!
Comment 4 Jonathan Druart 2024-04-17 10:06:57 UTC
Created attachment 164989 [details] [review]
Bug 30068: Remove table_settings_borrowers_table from circulation.tt

The history is a mess, but here we init a JS variable that is never
used.

Test plan:
  git grep table_settings_borrowers_table
should not return anything after this patch applied
Comment 5 Caroline Cyr La Rose 2024-04-17 14:11:06 UTC
Created attachment 165015 [details] [review]
Bug 30068: Remove table_settings_borrowers_table from circulation.tt

The history is a mess, but here we init a JS variable that is never
used.

Test plan:
  git grep table_settings_borrowers_table
should not return anything after this patch applied
Comment 6 Caroline Cyr La Rose 2024-04-17 14:11:30 UTC
oops, forgot the so line...
Comment 7 Caroline Cyr La Rose 2024-04-17 14:11:50 UTC
Created attachment 165016 [details] [review]
Bug 30068: Remove table_settings_borrowers_table from circulation.tt

The history is a mess, but here we init a JS variable that is never
used.

Test plan:
  git grep table_settings_borrowers_table
should not return anything after this patch applied

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 8 Caroline Cyr La Rose 2024-04-17 14:13:10 UTC
I also tried a couple of things like searching for patrons and placing a hold and nothing exploded.
Comment 9 Martin Renvoize 2024-04-17 15:58:07 UTC
Created attachment 165027 [details] [review]
Bug 30068: Remove table_settings_borrowers_table from circulation.tt

The history is a mess, but here we init a JS variable that is never
used.

Test plan:
  git grep table_settings_borrowers_table
should not return anything after this patch applied

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Katrin Fischer 2024-04-18 14:51:53 UTC
Pushed for 24.05!

Well done everyone, thank you!