Bug 32272

Summary: Last borrower and previous borrower display on moredetail.pl is broken
Product: Koha Reporter: Nick Clemens <nick>
Component: Staff interfaceAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: andrewfh, gmcharlt, jonathan.druart, katrin.fischer, lucas, m.de.rooy, michael.hafen
Version: MainKeywords: regression, rel_22_11_candidate
Hardware: All   
OS: All   
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
Bug Depends on: 31315    
Bug Blocks: 33788    
Attachments: Bug 32272: Restore last and previous borrowers on moredetail
Bug 32272: Restore last and previous borrowers on moredetail
Bug 32272: Restore last and previous borrowers on moredetail
Bug 32272: Restore last and previous borrowers on moredetail
Bug 32272: Restore last and previous borrowers on moredetail

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.