Bug 35782 - Remove Koha::Template::Plugin::Biblio::HoldsCount
Summary: Remove Koha::Template::Plugin::Biblio::HoldsCount
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on:
Blocks: 35783 35787
  Show dependency treegraph
 
Reported: 2024-01-12 08:04 UTC by Jonathan Druart
Modified: 2024-01-15 13:48 UTC (History)
4 users (show)

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


Attachments
Bug 35782: Replace TT plugin's method Biblio::HoldsCount (8.07 KB, patch)
2024-01-12 08:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Screenshot - Placing hold - No value in brackets (65.63 KB, image/png)
2024-01-12 11:06 UTC, David Nind
Details
Bug 35782: Fix scoping problem with "biblio" (7.23 KB, patch)
2024-01-12 14:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35782: Replace TT plugin's method Biblio::HoldsCount (8.10 KB, patch)
2024-01-12 16:19 UTC, David Nind
Details | Diff | Splinter Review
Bug 35782: Fix scoping problem with "biblio" (7.27 KB, patch)
2024-01-12 16:19 UTC, David Nind
Details | Diff | Splinter Review
Bug 35782: Replace TT plugin's method Biblio::HoldsCount (8.17 KB, patch)
2024-01-15 13:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35782: Fix scoping problem with "biblio" (7.33 KB, patch)
2024-01-15 13:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2024-01-12 08:04:46 UTC
HoldsCount must be replaced with biblio.holds.count
Comment 1 Jonathan Druart 2024-01-12 08:18:51 UTC
Created attachment 160898 [details] [review]
Bug 35782: Replace TT plugin's method Biblio::HoldsCount

We can use biblio.holds.count instead.

The main idea here is to make sure we are passing a Koha::Biblio object
as 'biblio' to all the templates including biblio-view-menu.inc

Test plan:
1. Go to the biblio detail view, click on the different entries in the menu
on the left. Confirm that the "Holds" tab always has the correct number
of Holds display in the parenthesis.

2. Run a search and confirm that the number of holds are still displayed
for each result.

QA:
git grep biblio-view-menu.inc
notice the tt list, open the corresponding perl controllers and confirm
that 'biblio' is passed and that it is a Koha::Biblio object.
The only missing place I found was in viewlog.
Note that we are not removing the TT method yet, we are marking it as
deprecated and also display a warning during the update DB process in
case one of the notice templates is using it.
Comment 2 David Nind 2024-01-12 11:06:22 UTC
Created attachment 160930 [details]
Screenshot - Placing hold - No value in brackets

When I go to place a hold in the staff interface, the display for the Holds tab has an empty bracket. See the screenshot.

After the hold is made, it displays correctly.

The search results is fine, and displays the correct number of holds.
Comment 3 Jonathan Druart 2024-01-12 14:23:58 UTC
Created attachment 160969 [details] [review]
Bug 35782: Fix scoping problem with "biblio"

The "biblio" Koha::Biblio object we send from the controller needs to be
accessible from the include files.
To avoid a clash we rename it biblio_info
Comment 4 Jonathan Druart 2024-01-12 14:24:13 UTC
Good catch, David, Thanks!

Should be fixed now.
Comment 5 David Nind 2024-01-12 16:19:04 UTC
Created attachment 160987 [details] [review]
Bug 35782: Replace TT plugin's method Biblio::HoldsCount

We can use biblio.holds.count instead.

The main idea here is to make sure we are passing a Koha::Biblio object
as 'biblio' to all the templates including biblio-view-menu.inc

Test plan:
1. Go to the biblio detail view, click on the different entries in the menu
on the left. Confirm that the "Holds" tab always has the correct number
of Holds display in the parenthesis.

2. Run a search and confirm that the number of holds are still displayed
for each result.

QA:
git grep biblio-view-menu.inc
notice the tt list, open the corresponding perl controllers and confirm
that 'biblio' is passed and that it is a Koha::Biblio object.
The only missing place I found was in viewlog.
Note that we are not removing the TT method yet, we are marking it as
deprecated and also display a warning during the update DB process in
case one of the notice templates is using it.

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2024-01-12 16:19:06 UTC
Created attachment 160988 [details] [review]
Bug 35782: Fix scoping problem with "biblio"

The "biblio" Koha::Biblio object we send from the controller needs to be
accessible from the include files.
To avoid a clash we rename it biblio_info

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2024-01-12 17:16:11 UTC
Testing notes (using KTD):

1. Apply the patch, run updatedatabase, restart_all

2. Go to a record with items in the staff interface.

3. Place several holds - the number of holds shown in brackets on the Holds tab label should display and be correct.

4. Do a search that returns the record(s) with the holds - the Holds (X) should be displayed correctly in the search results.
Comment 8 Tomás Cohen Arazi 2024-01-15 13:11:22 UTC
Created attachment 161036 [details] [review]
Bug 35782: Replace TT plugin's method Biblio::HoldsCount

We can use biblio.holds.count instead.

The main idea here is to make sure we are passing a Koha::Biblio object
as 'biblio' to all the templates including biblio-view-menu.inc

Test plan:
1. Go to the biblio detail view, click on the different entries in the menu
on the left. Confirm that the "Holds" tab always has the correct number
of Holds display in the parenthesis.

2. Run a search and confirm that the number of holds are still displayed
for each result.

QA:
git grep biblio-view-menu.inc
notice the tt list, open the corresponding perl controllers and confirm
that 'biblio' is passed and that it is a Koha::Biblio object.
The only missing place I found was in viewlog.
Note that we are not removing the TT method yet, we are marking it as
deprecated and also display a warning during the update DB process in
case one of the notice templates is using it.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Tomás Cohen Arazi 2024-01-15 13:11:25 UTC
Created attachment 161037 [details] [review]
Bug 35782: Fix scoping problem with "biblio"

The "biblio" Koha::Biblio object we send from the controller needs to be
accessible from the include files.
To avoid a clash we rename it biblio_info

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>