Bug 33948 - Replace GetAllIssues with Koha::Checkouts - staff
Summary: Replace GetAllIssues with Koha::Checkouts - staff
Status: RESOLVED FIXED
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:
: 8483 (view as bug list)
Depends on: 33946
Blocks: 33947 35335 35366
  Show dependency treegraph
 
Reported: 2023-06-08 07:46 UTC by Jonathan Druart
Modified: 2023-11-24 07:06 UTC (History)
10 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.11.00


Attachments
Bug 33948: Replace GetAllIssues with Koha::Checkouts - staff (9.78 KB, patch)
2023-06-08 11:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33948: Replace GetAllIssues with Koha::Checkouts - staff (9.84 KB, patch)
2023-07-27 21:24 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 33948: Replace GetAllIssues with Koha::Checkouts - staff (9.90 KB, patch)
2023-09-22 13:02 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2023-06-08 07:46:46 UTC

    
Comment 1 Jonathan Druart 2023-06-08 08:22:09 UTC
*** Bug 8483 has been marked as a duplicate of this bug. ***
Comment 2 Jonathan Druart 2023-06-08 11:02:15 UTC
Created attachment 152154 [details] [review]
Bug 33948: Replace GetAllIssues with Koha::Checkouts - staff

This patch replace GetAllIssues in readingrec.pl which is the only
occurrence in the staff code.

To test this patch you will need several items in your checkout history
in order to confirm that the display is the same before and after the
patch.

However there is one change! And it's a bug fix (or an enhancement, it
depends on how you see things). It includes checkouts with deleted items, which is
a long standing bug (see bug 8483).

Performance will be compared as well.
Comment 3 Fridolin Somers 2023-07-27 21:24:06 UTC
Created attachment 154005 [details] [review]
Bug 33948: Replace GetAllIssues with Koha::Checkouts - staff

This patch replace GetAllIssues in readingrec.pl which is the only
occurrence in the staff code.

To test this patch you will need several items in your checkout history
in order to confirm that the display is the same before and after the
patch.

However there is one change! And it's a bug fix (or an enhancement, it
depends on how you see things). It includes checkouts with deleted items, which is
a long standing bug (see bug 8483).

Performance will be compared as well.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 4 Fridolin Somers 2023-07-27 21:28:26 UTC
I fixed on missing space before $KohaDates : 
[% checkout.returndate |$KohaDates with_hours => 1 %]

Performance looks OK.
Comment 5 Kyle M Hall 2023-09-22 13:02:17 UTC
Created attachment 156091 [details] [review]
Bug 33948: Replace GetAllIssues with Koha::Checkouts - staff

This patch replace GetAllIssues in readingrec.pl which is the only
occurrence in the staff code.

To test this patch you will need several items in your checkout history
in order to confirm that the display is the same before and after the
patch.

However there is one change! And it's a bug fix (or an enhancement, it
depends on how you see things). It includes checkouts with deleted items, which is
a long standing bug (see bug 8483).

Performance will be compared as well.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Tomás Cohen Arazi 2023-09-22 15:39:14 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 7 Fridolin Somers 2023-09-22 20:30:54 UTC
Enhancement not pushed to 23.05.x
Comment 8 Andrii Nugged 2023-11-14 18:17:30 UTC
koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt

                                <td><a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% issue.issuer_id | uri %]">[% issue.firstname | html %] [% issue.surname | html %]</a></td>

changed to:

                                [% SET issuer = checkout.issuer %]
                                <td><a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% issuer.issuer_id | uri %]">[% issuer.firstname | html %] [% issuer.surname | html %]</a></td>

that leads to error:

    Koha::Patron->issuer_id is not covered by tests!

should be:


    issuer.borrowernumber

I suppose.
Comment 9 Peter Vashchuk 2023-11-14 18:34:36 UTC
Created a ticket where the fix for this problem will be provided ->
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35335
Comment 10 Jonathan Druart 2023-11-15 13:28:08 UTC
(In reply to Andrii Nugged from comment #8)
> koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt
> 
>                                 <td><a
> href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% issue.issuer_id |
> uri %]">[% issue.firstname | html %] [% issue.surname | html %]</a></td>
> 
> changed to:
> 
>                                 [% SET issuer = checkout.issuer %]
>                                 <td><a
> href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% issuer.issuer_id |
> uri %]">[% issuer.firstname | html %] [% issuer.surname | html %]</a></td>
> 
> that leads to error:
> 
>     Koha::Patron->issuer_id is not covered by tests!
> 
> should be:
> 
> 
>     issuer.borrowernumber
> 
> I suppose.

Yes, can you provide a fix?