Bug 32272 - Last borrower and previous borrower display on moredetail.pl is broken
Summary: Last borrower and previous borrower display on moredetail.pl is broken
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords: regression, rel_22_11_candidate
Depends on: 31315
Blocks: 33788
  Show dependency treegraph
 
Reported: 2022-11-18 15:44 UTC by Nick Clemens
Modified: 2023-12-28 20:43 UTC (History)
7 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.02


Attachments
Bug 32272: Restore last and previous borrowers on moredetail (4.12 KB, patch)
2022-11-21 09:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32272: Restore last and previous borrowers on moredetail (4.15 KB, patch)
2023-01-06 21:28 UTC, Michael Hafen
Details | Diff | Splinter Review
Bug 32272: Restore last and previous borrowers on moredetail (4.16 KB, patch)
2023-01-09 17:59 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 32272: Restore last and previous borrowers on moredetail (4.25 KB, patch)
2023-01-13 09:44 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 32272: Restore last and previous borrowers on moredetail (4.24 KB, patch)
2023-01-13 10:17 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2022-11-18 15:44:50 UTC
Bug 31315 removed GetItemsInfo from moredetail.pl

In the template we expected the item to contain the borrowernumber and cardnumber for last three borrowers from old issues:

341                 [% IF ( ITEM_DAT.card0 ) %]<li><span class="label">Last borrower:</span> <a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% ITEM_DAT.borrower0 | uri %]">[% ITEM_DAT.card0 | html %]</a>&nbsp;</li>[% END %]
342                 [% IF ( ITEM_DAT.card1 ) %]<li><span class="label">Previous borrower:</span> <a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% ITEM_DAT.borrower1 | uri %]">[% ITEM_DAT.card1 | html %]</a>&nbsp;</li>[% END %]
343                 [% IF ( ITEM_DAT.card2 ) %]<li><span class="label">Previous borrower:</span> <a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% ITEM_DAT.borrower2 | uri %]">[% ITEM_DAT.card2 | html %]</a>&nbsp;</li>[% END %]


We should either fix this, or remove this display
Comment 1 Jonathan Druart 2022-11-21 09:11:58 UTC
What's your opinion on this?

I don't mind reintroducing the "feature", but there is something inconsistent: We are displaying "Last returned by" if StoreLastBorrower is set, but then display the last 3 patrons who borrowed the items whatever is the value of StoreLastBorrower.
Comment 2 Jonathan Druart 2022-11-21 09:23:14 UTC
Created attachment 144107 [details] [review]
Bug 32272: Restore last and previous borrowers on moredetail

GetItemsInfo added a card0, card1 and card2 info with the cardnumbers of
the last 3 borrowers.
But 31315 forgot to adjust the code when it has been moved to using
Koha::Items

Test plan:
Check an item in and out to several patrons
Go to the moredetail page (items) and confirm that the "last borrowers"
info is displayed
Comment 3 Jonathan Druart 2022-11-21 09:24:31 UTC
Marking as 22.11 candidate but could be for .01
Comment 4 Andrew Fuerste-Henry 2022-11-21 22:44:31 UTC
(In reply to Jonathan Druart from comment #1)
> What's your opinion on this?
> 
> I don't mind reintroducing the "feature", but there is something
> inconsistent: We are displaying "Last returned by" if StoreLastBorrower is
> set, but then display the last 3 patrons who borrowed the items whatever is
> the value of StoreLastBorrower.

Yeah, those are two parallel but distinct systems with some confusing labels. Both are meaningful and needed.
Comment 5 Andrew Fuerste-Henry 2022-11-21 22:45:12 UTC
Patch doesn't apply:
Applying: Bug 32272: Restore last and previous borrowers on moredetail
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt
error: Failed to merge in the changes.
Patch failed at 0001 Bug 32272: Restore last and previous borrowers on moredetail
[33mhint: Use 'git am --show-current-patch=diff' to see the failed patch[m
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-32272-Restore-last-and-previous-borrowers-on-m-ilGOtR.patch
Apply? [(y)es, (n)o, (i)nteractive]
Comment 6 Michael Hafen 2023-01-06 21:28:35 UTC
Created attachment 145107 [details] [review]
Bug 32272: Restore last and previous borrowers on moredetail

GetItemsInfo added a card0, card1 and card2 info with the cardnumbers of
the last 3 borrowers.
But 31315 forgot to adjust the code when it has been moved to using
Koha::Items

Test plan:
Check an item in and out to several patrons
Go to the moredetail page (items) and confirm that the "last borrowers"
info is displayed

[Rebase 2023-01-06]
Comment 7 Lucas Gass 2023-01-09 17:59:45 UTC
Created attachment 145156 [details] [review]
Bug 32272: Restore last and previous borrowers on moredetail

GetItemsInfo added a card0, card1 and card2 info with the cardnumbers of
the last 3 borrowers.
But 31315 forgot to adjust the code when it has been moved to using
Koha::Items

Test plan:
Check an item in and out to several patrons
Go to the moredetail page (items) and confirm that the "last borrowers"
info is displayed

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 8 Marcel de Rooy 2023-01-13 09:42:40 UTC
Michael: Normally a rebase does not imply an author change?
Comment 9 Marcel de Rooy 2023-01-13 09:44:10 UTC
Created attachment 145255 [details] [review]
Bug 32272: Restore last and previous borrowers on moredetail

GetItemsInfo added a card0, card1 and card2 info with the cardnumbers of
the last 3 borrowers.
But 31315 forgot to adjust the code when it has been moved to using
Koha::Items

Test plan:
Check an item in and out to several patrons
Go to the moredetail page (items) and confirm that the "last borrowers"
info is displayed

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Jonathan Druart 2023-01-13 10:17:54 UTC
Created attachment 145258 [details] [review]
Bug 32272: Restore last and previous borrowers on moredetail

GetItemsInfo added a card0, card1 and card2 info with the cardnumbers of
the last 3 borrowers.
But 31315 forgot to adjust the code when it has been moved to using
Koha::Items

Test plan:
Check an item in and out to several patrons
Go to the moredetail page (items) and confirm that the "last borrowers"
info is displayed

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Jonathan Druart 2023-01-13 10:18:06 UTC
Restored date and author.
Comment 12 Martin Renvoize 2023-01-18 15:50:07 UTC
Nice work everyone!

Pushed to 23.05.x for the next release
Comment 13 Jacob O'Mara 2023-01-18 17:05:51 UTC
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.
Comment 14 Lucas Gass 2023-01-24 20:34:23 UTC
Missing dependencies for 22.05.x, no backport.