From f177d30ab43c64e9f0f5c06cf041fcfb37e53797 Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Mon, 29 Aug 2022 16:30:01 +1200 Subject: [PATCH] Bug 31051: (follow-up) Tests for get_savings and more - Added tests in t/db_dependent/Koha/Patron.t - Added wording to OPACShowSavings syspref about anonymised checkout history - Added IDs to the savings messages on the OPAC - Prevent explosion if a checked out item has been deleted Sponsored-by: Horowhenua Libraries Trust Signed-off-by: Hammat Wele Signed-off-by: Fridolin Somers Signed-off-by: Tomas Cohen Arazi --- Koha/Patron.pm | 26 +++++++--------- .../en/modules/admin/preferences/opac.pref | 1 + .../en/modules/opac-readingrecord.tt | 2 +- .../bootstrap/en/modules/opac-user.tt | 2 +- t/db_dependent/Koha/Patron.t | 31 ++++++++++++++++++- 5 files changed, 45 insertions(+), 17 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 7c4ea58d1e..dc329ed706 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -2441,22 +2441,20 @@ sub get_savings { my $savings = 0; - # get old issues - my $old_issues_rs = $self->_result->old_issues; - my @old_itemnumbers = $old_issues_rs->get_column('itemnumber')->all; - - foreach my $itemnumber ( @old_itemnumbers ) { - my $item = Koha::Items->find( $itemnumber ); - $savings += $item->replacementprice; + # 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; + } } - # get current issues - my $issues_rs = $self->_result->issues; - my @itemnumbers = $issues_rs->get_column('itemnumber')->all; - - foreach my $itemnumber ( @itemnumbers ) { - my $item = Koha::Items->find( $itemnumber ); - $savings += $item->replacementprice; + # get current checkouts + my @checkouts = $self->checkouts->as_list; + foreach my $checkout ( @checkouts ) { + if ( $checkout->item ) { + $savings += $checkout->item->replacementprice; + } } return $savings; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref index 15675aa651..342d1853ce 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref @@ -537,6 +537,7 @@ OPAC: checkouthistory: "on patron's checkout history page (the system preference opacreadinghistory must be enabled)" summary: "in user summary box on OPAC homepage (the system preference OPACUserSummary must be enabled)" user: "on patron's 'your summary' page" + - ". Note that displayed savings may be inaccurate if checkout history is anonymized." OpenURL: - - 'Complete URL of OpenURL resolver (starting with http:// or https://):' diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt index 65a62334d8..1e80809a8a 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt @@ -56,7 +56,7 @@
[% IF savings %] -
+
Congratulations, you have saved a total of [% savings | $Price with_symbol => 1 %] by using the library.
[% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt index 647a788699..37d36399ea 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt @@ -163,7 +163,7 @@ [% END # / IF patron_flagged %] [% IF savings %] -
+
Congratulations, you have saved a total of [% savings | $Price with_symbol => 1 %] by using the library.
[% END %] diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index e92b054d86..f3b83ec4d1 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 19; +use Test::More tests => 20; use Test::Exception; use Test::Warn; @@ -29,6 +29,7 @@ use Koha::DateUtils qw(dt_from_string); use Koha::ArticleRequests; use Koha::Patrons; use Koha::Patron::Relationships; +use C4::Circulation qw( AddIssue AddReturn ); use t::lib::TestBuilder; use t::lib::Mocks; @@ -1349,3 +1350,31 @@ subtest 'notify_library_of_registration()' => sub { $schema->storage->txn_rollback; }; + +subtest 'get_savings tests' => sub { + plan tests => 2; + + $schema->storage->txn_begin; + + my $library = $builder->build_object({ class => 'Koha::Libraries' }); + my $patron = $builder->build_object({ class => 'Koha::Patrons' }, { value => { branchcode => $library->branchcode } }); + + t::lib::Mocks::mock_userenv({ patron => $patron, branchcode => $library->branchcode }); + + my $biblio1 = $builder->build_object({ class => 'Koha::Biblios' }); + 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 } }); + + AddIssue( $patron->unblessed, $item1->barcode ); + AddIssue( $patron->unblessed, $item2->barcode ); + + my $savings = $patron->get_savings; + is( $savings, $item1->replacementprice + $item2->replacementprice, "Savings correctly calculated from current issues" ); + + AddReturn( $item2->barcode, $item2->homebranch ); + + $savings = $patron->get_savings; + is( $savings, $item1->replacementprice + $item2->replacementprice, "Savings correctly calculated from current and old issues" ); + + $schema->storage->txn_rollback; +}; -- 2.39.5