Bug 31051 - Show patron's 'savings' on the OPAC
Summary: Show patron's 'savings' on the OPAC
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low new feature (vote)
Assignee: Aleisha Amohia
QA Contact: Fridolin Somers
URL:
Keywords:
Depends on: 26648
Blocks: 35078
  Show dependency treegraph
 
Reported: 2022-06-27 04:21 UTC by Aleisha Amohia
Modified: 2023-11-13 15:16 UTC (History)
11 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/681
Text to go in the release notes:
This new feature shows a patron how much they have saved by using the library rather than purchasing items. Savings are calculated based on item replacement prices. The system preference allows you to choose where to display the savings - the user page, the summary box on the OPAC homepage, or the checkout history page.
Version(s) released in:
23.05.00


Attachments
Bug 31051: [WIP] Add OPACShowSavings preference (3.98 KB, patch)
2022-06-27 05:12 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 31051: Show patron savings on the OPAC (8.41 KB, patch)
2022-06-28 05:02 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 31051: Add OPACShowSavings system preference (3.88 KB, patch)
2022-06-28 20:35 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 31051: Show patron savings on the OPAC (8.41 KB, patch)
2022-06-28 20:35 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 31051: Show patron savings on the OPAC (8.43 KB, patch)
2022-06-30 13:42 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 31051: Add OPACShowSavings system preference (3.90 KB, patch)
2022-06-30 13:56 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 31051: Show patron savings on the OPAC (8.42 KB, patch)
2022-06-30 13:57 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 31051: Add OPACShowSavings system preference (3.96 KB, patch)
2022-06-30 14:08 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 31051: Show patron savings on the OPAC (8.48 KB, patch)
2022-06-30 14:08 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 31051: Add OPACShowSavings system preference (4.10 KB, patch)
2022-08-15 01:38 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 31051: Show patron savings on the OPAC (8.58 KB, patch)
2022-08-15 01:39 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 31051: (follow-up) Simplify savings wording for OPAC (1.15 KB, patch)
2022-08-15 01:39 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 31051: Add OPACShowSavings system preference (4.13 KB, patch)
2022-08-18 17:30 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 31051: Show patron savings on the OPAC (8.59 KB, patch)
2022-08-18 17:30 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 31051: (follow-up) Simplify savings wording for OPAC (1.20 KB, patch)
2022-08-18 17:30 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 31051: Add OPACShowSavings system preference (4.19 KB, patch)
2022-08-28 13:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31051: Show patron savings on the OPAC (8.64 KB, patch)
2022-08-28 13:19 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31051: (follow-up) Simplify savings wording for OPAC (1.25 KB, patch)
2022-08-28 13:19 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31051: (QA follow-up) Simplify savings wording for reading history (1.28 KB, patch)
2022-08-28 13:19 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31051: (follow-up) Tests for get_savings and more (5.22 KB, patch)
2022-08-29 04:32 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 31051: (follow-up) Tests for get_savings and more (6.59 KB, patch)
2022-11-08 01:32 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 31051: (follow-up) Tests for get_savings and more (6.63 KB, patch)
2022-11-18 21:06 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 31051: Add OPACShowSavings system preference (4.25 KB, patch)
2022-12-21 07:14 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 31051: Show patron savings on the OPAC (8.70 KB, patch)
2022-12-21 07:14 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 31051: (follow-up) Simplify savings wording for OPAC (1.31 KB, patch)
2022-12-21 07:14 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 31051: (QA follow-up) Simplify savings wording for reading history (1.34 KB, patch)
2022-12-21 07:14 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 31051: (follow-up) Tests for get_savings and more (6.69 KB, patch)
2022-12-21 07:14 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 31051: (QA follow-up) Use a single query and avoid duplicated sums (3.55 KB, patch)
2023-02-09 14:35 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31051: (QA follow-up) Fix misplaced syspref (2.63 KB, patch)
2023-02-09 14:35 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31051: Fix test (2.10 KB, patch)
2023-02-23 08:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 31051: Generate items correctly in test (1.86 KB, patch)
2023-02-23 08:21 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 Aleisha Amohia 2022-06-27 04:21:02 UTC
It would be cool to have a feature which would show patrons via the OPAC how much they've saved by using the library, instead of purchasing items themselves.
Comment 1 Aleisha Amohia 2022-06-27 05:12:06 UTC Comment hidden (obsolete)
Comment 2 Aleisha Amohia 2022-06-28 05:02:34 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2022-06-28 11:36:35 UTC
This didn't work at first when using my own test data. I got this error:

Can't call method "replacementprice" on an undefined value at /kohadevbox/koha/Koha/Patron.pm line 2206

It seems to come from there being lines in old_issues where the itemnumber is null. I'm not sure if that's normal or not. 

The only other issue I have is that I don't think "do not display anywhere on the OPAC" should be one of the preference options. It's contradictory to be able to select that *and* the other options. Users can select no options if they want the information not to appear.
Comment 4 Aleisha Amohia 2022-06-28 20:32:06 UTC
Sorry the first patch is a mistake, I seem to have missed attaching the correct one. Will fix
Comment 5 Aleisha Amohia 2022-06-28 20:35:39 UTC Comment hidden (obsolete)
Comment 6 Aleisha Amohia 2022-06-28 20:35:43 UTC Comment hidden (obsolete)
Comment 7 Marie-Luce Laflamme 2022-06-30 13:25:17 UTC
testing note:
add an item replacement cost (952$ v)  on checkout items 
it works well
Comment 8 ByWater Sandboxes 2022-06-30 13:42:37 UTC Comment hidden (obsolete)
Comment 9 Axelle Aix-Marseille Université 2022-06-30 13:56:10 UTC
Tested with a UNIMARC sandbox : items replacement prices = 995$g in UNIMARC.
It works well too.
Comment 10 Biblibre Sandboxes 2022-06-30 13:56:59 UTC Comment hidden (obsolete)
Comment 11 Biblibre Sandboxes 2022-06-30 13:57:04 UTC Comment hidden (obsolete)
Comment 12 Victor Grousset/tuxayo 2022-06-30 14:08:34 UTC Comment hidden (obsolete)
Comment 13 Victor Grousset/tuxayo 2022-06-30 14:08:46 UTC Comment hidden (obsolete)
Comment 14 Victor Grousset/tuxayo 2022-06-30 14:11:09 UTC
Marie-Luce and Axelle tested at the same time and one signoff line was lost. I just reattached the patches with both signoff lines.
Comment 15 Fridolin Somers 2022-08-13 06:43:04 UTC
Looking at this for QA.

I have a question :
                            Congratulations, you have saved a total of [% savings | $Price with_symbol => 1 %] by using the [% IF ( LibraryNameTitle ) %][% LibraryNameTitle | html %][% ELSE %]Koha online[% END %] catalog.

When LibraryNameTitle is not defined the text will say "you have saved a total of X by using the Koha online catalog".

Looks not good, the savings are caused by the checkouts so in the "real life" catalog, not the online.

Also often the content of LibraryNameTitle makes a weird text with catalog appended.

In my opinion we should use a more simple :
Congratulations, you have saved a total of [% savings | $Price with_symbol => 1 %] by using the library.
Comment 16 Aleisha Amohia 2022-08-15 01:38:58 UTC
Created attachment 139100 [details] [review]
Bug 31051: Add OPACShowSavings system preference

Signed-off-by: Marie-Luce <marie-luce.laflamme@inlibro.com>
Signed-off-by: Axelle <axelle.clarisse@univ-amu.fr>
Comment 17 Aleisha Amohia 2022-08-15 01:39:04 UTC
Created attachment 139101 [details] [review]
Bug 31051: Show patron savings on the OPAC

This new feature shows a patron how much they have saved by using the
library rather than purchasing items. Savings are calculated based on
item replacement prices. The system preference allows you to choose
where to display the savings - the user page, the summary box on the
OPAC homepage, or the checkout history page.

To test:
1. Update database and restart services
2. Confirm the new OPACShowSavings system preference is found in the
OPAC tab of Administration -> global system preferences. There should be
no options selected.
3. Find a patron with a checkout history, or check out a few items to a
patron.
4. Test with different values of OPACShowSavings and confirm that
savings are shown in the expected places.

Sponsored-by: Horowhenua Libraries Trust

Signed-off-by: Marie-Luce <marie-luce.laflamme@inlibro.com>
Signed-off-by: Axelle <axelle.clarisse@univ-amu.fr>
Comment 18 Aleisha Amohia 2022-08-15 01:39:08 UTC
Created attachment 139102 [details] [review]
Bug 31051: (follow-up) Simplify savings wording for OPAC
Comment 19 Fridolin Somers 2022-08-16 05:23:30 UTC
Great ;)
Comment 20 Owen Leonard 2022-08-18 17:30:46 UTC
Created attachment 139408 [details] [review]
Bug 31051: Add OPACShowSavings system preference

Signed-off-by: Marie-Luce <marie-luce.laflamme@inlibro.com>
Signed-off-by: Axelle <axelle.clarisse@univ-amu.fr>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 21 Owen Leonard 2022-08-18 17:30:50 UTC
Created attachment 139409 [details] [review]
Bug 31051: Show patron savings on the OPAC

This new feature shows a patron how much they have saved by using the
library rather than purchasing items. Savings are calculated based on
item replacement prices. The system preference allows you to choose
where to display the savings - the user page, the summary box on the
OPAC homepage, or the checkout history page.

To test:
1. Update database and restart services
2. Confirm the new OPACShowSavings system preference is found in the
OPAC tab of Administration -> global system preferences. There should be
no options selected.
3. Find a patron with a checkout history, or check out a few items to a
patron.
4. Test with different values of OPACShowSavings and confirm that
savings are shown in the expected places.

Sponsored-by: Horowhenua Libraries Trust

Signed-off-by: Marie-Luce <marie-luce.laflamme@inlibro.com>
Signed-off-by: Axelle <axelle.clarisse@univ-amu.fr>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 22 Owen Leonard 2022-08-18 17:30:56 UTC
Created attachment 139410 [details] [review]
Bug 31051: (follow-up) Simplify savings wording for OPAC

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 23 Katrin Fischer 2022-08-28 13:18:57 UTC
Created attachment 139911 [details] [review]
Bug 31051: Add OPACShowSavings system preference

Signed-off-by: Marie-Luce <marie-luce.laflamme@inlibro.com>
Signed-off-by: Axelle <axelle.clarisse@univ-amu.fr>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Katrin Fischer 2022-08-28 13:19:02 UTC
Created attachment 139912 [details] [review]
Bug 31051: Show patron savings on the OPAC

This new feature shows a patron how much they have saved by using the
library rather than purchasing items. Savings are calculated based on
item replacement prices. The system preference allows you to choose
where to display the savings - the user page, the summary box on the
OPAC homepage, or the checkout history page.

To test:
1. Update database and restart services
2. Confirm the new OPACShowSavings system preference is found in the
OPAC tab of Administration -> global system preferences. There should be
no options selected.
3. Find a patron with a checkout history, or check out a few items to a
patron.
4. Test with different values of OPACShowSavings and confirm that
savings are shown in the expected places.

Sponsored-by: Horowhenua Libraries Trust

Signed-off-by: Marie-Luce <marie-luce.laflamme@inlibro.com>
Signed-off-by: Axelle <axelle.clarisse@univ-amu.fr>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Katrin Fischer 2022-08-28 13:19:08 UTC
Created attachment 139913 [details] [review]
Bug 31051: (follow-up) Simplify savings wording for OPAC

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Katrin Fischer 2022-08-28 13:19:13 UTC
Created attachment 139914 [details] [review]
Bug 31051: (QA follow-up) Simplify savings wording for reading history

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 27 Katrin Fischer 2022-08-28 13:24:25 UTC
This works nicely and I quite like the idea! Only one real blocker and some notes/suggestions:

1) +=head3 get_savings (blocker)

We need some unit tests here.

2) issues + old_issues (not blocker)

A lot of libraries nowadays choose to anonymize the checkout history rather fast. So as we look at issues and old_issues here, the amount saved will go up and down quite a bit.

I wonder if it could make sense to add a note to the pref so libraries have this in mind when activating the feature.

3) Wording (follow-up patch added)

I have provided another follow-up for the reading history page following Owen's suggestion as this will also ease translation (combined constructs like x catalog are always a little hard)

4) id (not blocker)

It could be nice to have a specific id on the message in the summary page and reading history in case libraries want to do some formatting or rephrasing.
Comment 28 Aleisha Amohia 2022-08-29 04:32:07 UTC
Created attachment 139937 [details] [review]
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

Sponsored-by: Horowhenua Libraries Trust
Comment 29 Lucas Gass 2022-09-02 01:30:58 UTC
I've tried adding something similar to this in an ISSUESLIP and I always run into the problem where things explode when delete an item that was in the patrons old issues. The same thing seems to happen here. If I delete something from a patrons old issues I get:

Can't call method "replacementprice" on an undefined value at /kohadevbox/koha/Koha/Patron.pm line 2218
Comment 30 Lucas Gass 2022-09-12 21:48:00 UTC
Perhaps if we could make Bug 26648 move it would solve the problem here as well.
Comment 31 Lucas Gass 2022-10-03 18:31:13 UTC
Aleisha,

I think if we make this one dependent on Bug 26648 we can move it back to NSO.
Comment 32 Aleisha Amohia 2022-11-08 01:32:24 UTC
Created attachment 143384 [details] [review]
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
Comment 33 Aleisha Amohia 2022-11-08 01:33:25 UTC
(In reply to Lucas Gass from comment #31)
> Aleisha,
> 
> I think if we make this one dependent on Bug 26648 we can move it back to
> NSO.

Addressed in latest patch, ready for testing.
Comment 34 Hammat wele 2022-11-18 21:06:56 UTC
Created attachment 144082 [details] [review]
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 <hammat.wele@inlibro.com>
Comment 35 Fridolin Somers 2022-12-21 07:14:28 UTC
Created attachment 144759 [details] [review]
Bug 31051: Add OPACShowSavings system preference

Signed-off-by: Marie-Luce <marie-luce.laflamme@inlibro.com>
Signed-off-by: Axelle <axelle.clarisse@univ-amu.fr>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 36 Fridolin Somers 2022-12-21 07:14:33 UTC
Created attachment 144760 [details] [review]
Bug 31051: Show patron savings on the OPAC

This new feature shows a patron how much they have saved by using the
library rather than purchasing items. Savings are calculated based on
item replacement prices. The system preference allows you to choose
where to display the savings - the user page, the summary box on the
OPAC homepage, or the checkout history page.

To test:
1. Update database and restart services
2. Confirm the new OPACShowSavings system preference is found in the
OPAC tab of Administration -> global system preferences. There should be
no options selected.
3. Find a patron with a checkout history, or check out a few items to a
patron.
4. Test with different values of OPACShowSavings and confirm that
savings are shown in the expected places.

Sponsored-by: Horowhenua Libraries Trust

Signed-off-by: Marie-Luce <marie-luce.laflamme@inlibro.com>
Signed-off-by: Axelle <axelle.clarisse@univ-amu.fr>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 37 Fridolin Somers 2022-12-21 07:14:38 UTC
Created attachment 144761 [details] [review]
Bug 31051: (follow-up) Simplify savings wording for OPAC

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 38 Fridolin Somers 2022-12-21 07:14:43 UTC
Created attachment 144762 [details] [review]
Bug 31051: (QA follow-up) Simplify savings wording for reading history

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 39 Fridolin Somers 2022-12-21 07:14:49 UTC
Created attachment 144763 [details] [review]
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 <hammat.wele@inlibro.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 40 Fridolin Somers 2022-12-21 07:16:01 UTC
Fixed QA failure 
 FAIL	installer/data/mysql/atomicupdate/bug_31051_-_add_OPACShowSavings_syspref.pl
   FAIL	  file permissions
		File must have the exec flag

atomic file permission set to a+x
Comment 41 Tomás Cohen Arazi 2023-02-09 13:47:23 UTC
I generally like the feature.

I have two QA remarks to make:

1. I expected to see some 'DISTINCT' in the queries, or a 'uniq' in the code. Because I'm not sure how the saving should be counted (i.e. if the book is recalled and I need to borrow it again, then I didn't save two times the replacement prince, but one). I guess this is subject to debate so I would mark 'In discussion' until this is settled.

2. MySQL (and DBIC) has ways to calculate this SUM in a single DB query. So we shouldn't need to look trough many checkouts and items summing things in checkouts+old_checkouts DB queries. This is a blocker in my opinion. Happy to guide/help if needed.
Comment 42 Tomás Cohen Arazi 2023-02-09 14:35:28 UTC
Created attachment 146431 [details] [review]
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>
Comment 43 Tomás Cohen Arazi 2023-02-09 14:35:31 UTC
Created attachment 146432 [details] [review]
Bug 31051: (QA follow-up) Fix misplaced syspref

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 44 Tomás Cohen Arazi 2023-02-09 14:37:36 UTC
(In reply to Tomás Cohen Arazi from comment #41)
> 2. MySQL (and DBIC) has ways to calculate this SUM in a single DB query. So
> we shouldn't need to look trough many checkouts and items summing things in
> checkouts+old_checkouts DB queries. This is a blocker in my opinion. Happy
> to guide/help if needed.

I submitted a candidate patch that makes things a bit more optimal. We could still make some joins to have a single query, but this is much better on its own.

> 1. I expected to see some 'DISTINCT' in the queries, or a 'uniq' in the
> code. Because I'm not sure how the saving should be counted (i.e. if the
> book is recalled and I need to borrow it again, then I didn't save two times
> the replacement prince, but one). I guess this is subject to debate so I
> would mark 'In discussion' until this is settled.

My implementation does what I think we should do. So letting the bug 'In discussion' until this is settled. Otherwise it would be ready.
Comment 45 Aleisha Amohia 2023-02-16 22:33:49 UTC
I tested the follow-up patches and they work well. Happy for this to be pushed.
Comment 46 Tomás Cohen Arazi 2023-02-22 13:11:15 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 47 Jonathan Druart 2023-02-23 08:04:29 UTC
Jenkins is failing, but I don't recreate locally.

Koha_Master/2394

koha_1       |     #   Failed test 'Savings correctly calculated from current issues'
koha_1       |     #   at t/db_dependent/Koha/Patron.t line 1380.
koha_1       |     #          got: '34583.70'
koha_1       |     #     expected: '34583.7'
koha_1       | 
koha_1       |     #   Failed test 'Savings correctly calculated from current and old issues'
koha_1       |     #   at t/db_dependent/Koha/Patron.t line 1385.
koha_1       |     #          got: '34583.70'
koha_1       |     #     expected: '34583.7'
koha_1       |     # Looks like you failed 2 tests of 4.
koha_1       | 
koha_1       | #   Failed test 'get_savings tests'
koha_1       | #   at t/db_dependent/Koha/Patron.t line 1388.
koha_1       | # Looks like you failed 1 test of 20.
koha_1       | [00:07:57] t/db_dependent/Koha/Patron.t
Comment 48 Jonathan Druart 2023-02-23 08:09:00 UTC
(In reply to Jonathan Druart from comment #47)
> Jenkins is failing, but I don't recreate locally.

Because it ends with '0'.
Comment 49 Jonathan Druart 2023-02-23 08:13:10 UTC
Created attachment 147189 [details] [review]
Bug 31051: Fix test

When savings ends with a "0"

koha_1       |     #   Failed test 'Savings correctly calculated from current issues'
koha_1       |     #   at t/db_dependent/Koha/Patron.t line 1380.
koha_1       |     #          got: '34583.70'
koha_1       |     #     expected: '34583.7'
koha_1       |
koha_1       |     #   Failed test 'Savings correctly calculated from current and old issues'
koha_1       |     #   at t/db_dependent/Koha/Patron.t line 1385.
koha_1       |     #          got: '34583.70'
koha_1       |     #     expected: '34583.7'
koha_1       |     # Looks like you failed 2 tests of 4.
koha_1       |
koha_1       | #   Failed test 'get_savings tests'
koha_1       | #   at t/db_dependent/Koha/Patron.t line 1388.
koha_1       | # Looks like you failed 1 test of 20.
koha_1       | [00:07:57] t/db_dependent/Koha/Patron.t
Comment 50 Jonathan Druart 2023-02-23 08:20:34 UTC
But, wait. Aren't we expecting "10" (5+5) here?
Comment 51 Jonathan Druart 2023-02-23 08:21:59 UTC
Created attachment 147190 [details] [review]
Bug 31051: Generate items correctly in test

There was a misplaced bracket in build_object.
To generate item you should need build_sample_item anyway.
Comment 52 Tomás Cohen Arazi 2023-02-23 11:09:00 UTC
(In reply to Jonathan Druart from comment #47)
> Jenkins is failing, but I don't recreate locally.
> 
> Koha_Master/2394
> 
> koha_1       |     #   Failed test 'Savings correctly calculated from
> current issues'
> koha_1       |     #   at t/db_dependent/Koha/Patron.t line 1380.
> koha_1       |     #          got: '34583.70'
> koha_1       |     #     expected: '34583.7'
> koha_1       | 
> koha_1       |     #   Failed test 'Savings correctly calculated from
> current and old issues'
> koha_1       |     #   at t/db_dependent/Koha/Patron.t line 1385.
> koha_1       |     #          got: '34583.70'
> koha_1       |     #     expected: '34583.7'
> koha_1       |     # Looks like you failed 2 tests of 4.
> koha_1       | 
> koha_1       | #   Failed test 'get_savings tests'
> koha_1       | #   at t/db_dependent/Koha/Patron.t line 1388.
> koha_1       | # Looks like you failed 1 test of 20.
> koha_1       | [00:07:57] t/db_dependent/Koha/Patron.t

This is interesting, as it passed for me when I tweaked the tests.

Anyway, thanks for the fixes, Jonathan!
Comment 53 Martin Renvoize 2023-02-27 13:30:16 UTC
New feature, won't be backported to stable 22.11.x series
Comment 54 Caroline Cyr La Rose 2023-05-11 13:25:59 UTC
Can the [% IF savings %] [% savings | $Price with_symbol => 1 %] be used in notices? It would be useful to replace https://wiki.koha-community.org/wiki/Notices_and_Slips_Library#Add_total_value_of_items_borrowed_to_date_on_issue_slip
Comment 55 Katrin Fischer 2023-05-14 11:29:33 UTC
(In reply to Caroline Cyr La Rose from comment #54)
> Can the [% IF savings %] [% savings | $Price with_symbol => 1 %] be used in
> notices? It would be useful to replace
> https://wiki.koha-community.org/wiki/
> Notices_and_Slips_Library#Add_total_value_of_items_borrowed_to_date_on_issue_
> slip

Hi Caroline, I tested in CHECKIN and this works:

[% USE Price %]
[% borrower.get_savings | $Price with_symbol => 1 %]