Bugzilla – Attachment 146431 Details for
Bug 31051
Show patron's 'savings' on the OPAC
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 31051: (QA follow-up) Use a single query and avoid duplicated sums
Bug-31051-QA-follow-up-Use-a-single-query-and-avoi.patch (text/plain), 3.55 KB, created by
Tomás Cohen Arazi (tcohen)
on 2023-02-09 14:35:28 UTC
(
hide
)
Description:
Bug 31051: (QA follow-up) Use a single query and avoid duplicated sums
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2023-02-09 14:35:28 UTC
Size:
3.55 KB
patch
obsolete
>From f509e8d3095eeb3d18a1591fa1cf457967958c9b Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Thu, 9 Feb 2023 11:26:56 -0300 >Subject: [PATCH] Bug 31051: (QA follow-up) Use a single query and avoid > duplicated sums > >This patch makes the `get_savings` method use a single query to >calculate the sum of the replacement prices. This way we save one query >per item and just rely on the DB features. > >It has a side effect: we are not summing items twice. > >Added tests for the 'itenumber is null' pathological but common use >case (specially in old_issues), as mentioned by Lucas. Handling for this >is added (grep filtering out undefined ones) and also in the return, for >the empty case, with // 0. > >To test: >1. Apply this patch >2. Run: > qa -c 6 --run-tests >=> SUCCESS: All good >3. Sign off :-D > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > Koha/Patron.pm | 25 +++++++------------------ > t/db_dependent/Koha/Patron.t | 10 +++++++++- > 2 files changed, 16 insertions(+), 19 deletions(-) > >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index dc329ed7063..9282b38307f 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -2437,27 +2437,16 @@ Use the replacement price of patron's old and current issues to calculate how mu > =cut > > sub get_savings { >- my ( $self ) = @_; >+ my ($self) = @_; > >- my $savings = 0; >+ my @itemnumbers = grep { defined $_ } ( $self->old_checkouts->get_column('itemnumber'), $self->checkouts->get_column('itemnumber') ); > >- # get old checkouts >- my @old_checkouts = $self->old_checkouts->as_list; >- foreach my $old_checkout ( @old_checkouts ) { >- if ( $old_checkout->item ) { >- $savings += $old_checkout->item->replacementprice; >+ return Koha::Items->search( >+ { itemnumber => { -in => \@itemnumbers } }, >+ { select => [ { sum => 'me.replacementprice' } ], >+ as => ['total_savings'] > } >- } >- >- # get current checkouts >- my @checkouts = $self->checkouts->as_list; >- foreach my $checkout ( @checkouts ) { >- if ( $checkout->item ) { >- $savings += $checkout->item->replacementprice; >- } >- } >- >- return $savings; >+ )->next->get_column('total_savings') // 0; > } > > =head2 Internal methods >diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t >index f3b83ec4d14..f0910bd9591 100755 >--- a/t/db_dependent/Koha/Patron.t >+++ b/t/db_dependent/Koha/Patron.t >@@ -1352,7 +1352,8 @@ subtest 'notify_library_of_registration()' => sub { > }; > > subtest 'get_savings tests' => sub { >- plan tests => 2; >+ >+ plan tests => 4; > > $schema->storage->txn_begin; > >@@ -1365,6 +1366,13 @@ subtest 'get_savings tests' => sub { > my $item1 = $builder->build_object({ class => 'Koha::Items' }, { value => { biblionumber => $biblio1->biblionumber, replacementprice => '5.00', holdingbranch => $library->branchcode, homebranch => $library->branchcode } }); > my $item2 = $builder->build_object({ class => 'Koha::Items' }, { value => { biblionumber => $biblio1->biblionumber, replacementprice => '5.00', holdingbranch => $library->branchcode, homebranch => $library->branchcode } }); > >+ is( $patron->get_savings, 0, 'No checkouts, no savings' ); >+ >+ # Add an old checkout with deleted itemnumber >+ $builder->build_object({ class => 'Koha::Old::Checkouts', value => { itemnumber => undef, borrowernumber => $patron->id } }); >+ >+ is( $patron->get_savings, 0, 'No checkouts with itemnumber, no savings' ); >+ > AddIssue( $patron->unblessed, $item1->barcode ); > AddIssue( $patron->unblessed, $item2->barcode ); > >-- >2.34.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 31051
:
136564
|
136664
|
136698
|
136699
|
136770
|
136772
|
136773
|
136779
|
136781
|
139100
|
139101
|
139102
|
139408
|
139409
|
139410
|
139911
|
139912
|
139913
|
139914
|
139937
|
143384
|
144082
|
144759
|
144760
|
144761
|
144762
|
144763
| 146431 |
146432
|
147189
|
147190