Bug 33210 - (Bug 31963 follow-up) No hold fee message on OPAC should be displayed when there is no fee
Summary: (Bug 31963 follow-up) No hold fee message on OPAC should be displayed when th...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: unspecified
Hardware: All All
: P3 trivial (vote)
Assignee: Janusz Kaczmarek
QA Contact: Testopia
URL:
Keywords:
Depends on: 31963
Blocks:
  Show dependency treegraph
 
Reported: 2023-03-13 14:06 UTC by Janusz Kaczmarek
Modified: 2023-12-28 20:43 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.06


Attachments
Bug 33210: (Bug 31963 follow-up) No hold fee message on OPAC should be displayed when there is no fee (1.88 KB, patch)
2023-03-13 14:11 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 33210: (Bug 31963 follow-up) No hold fee message on OPAC should be displayed when there is no fee (1.93 KB, patch)
2023-03-13 20:28 UTC, David Nind
Details | Diff | Splinter Review
Bug 33210: Add unit test (2.17 KB, patch)
2023-03-24 16:21 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 33210: (Bug 31963 follow-up) No hold fee message on OPAC should be displayed when there is no fee (1.93 KB, patch)
2023-04-04 21:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 33210: Add unit test (2.21 KB, patch)
2023-04-04 21:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 33210: Add unit test (2.26 KB, patch)
2023-04-17 18:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33210: (Bug 31963 follow-up) No hold fee message on OPAC should be displayed when there is no fee (1.97 KB, patch)
2023-04-17 18:41 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Janusz Kaczmarek 2023-03-13 14:06:32 UTC
After resolving Bug 31963 everything works as expected when there is hold fee defined (!= 0).  But in case when the fee for given patron category is set to 0.00, the user will always see the message "You will be charged a hold fee of 0,00 ...", which is obviously not intended.

This is because categories.reservefee is returned from database as '0.000000' and as such, without type casting, is interpreted as string in Perl. Prior to Bug 31963 the result was compared to 0 before sending anything to the template, so the casting was done, now it is not.
Comment 1 Janusz Kaczmarek 2023-03-13 14:11:05 UTC
Created attachment 148120 [details] [review]
Bug 33210: (Bug 31963 follow-up) No hold fee message on OPAC should be displayed when there is no fee

After resolving Bug 31963 everything works as expected when there is
hold fee defined (!= 0).  But in case when the fee for given patron
category is set to 0.00, the user will always see the message "You will
be charged a hold fee of 0,00 ...", which is obviously not intended.

This is because categories.reservefee is returned from database as
'0.000000' and as such, without type casting, is interpreted as string
in Perl. Prior to Bug 31963 the result was compared to 0 before sending
anything to the template, so the casting was done, now it is not.

To test:
========
1. Go to Administration -> Patron categories
2. Edit your patron category and give a hold fee of 0.
3. HoldFeeMode does not matter - you can set it to any value.
4. In another tab, open the OPAC.
5. Search the OPAC for any record with an item.
6. Go to place a hold on this record.  You will see "You will be
   charged a hold fee of 0,00" --> This is a bug.
7. Apply patch and restart services.
6. Repeat step 6.
8. You should NOT see the hold fee message.
Comment 2 David Nind 2023-03-13 20:28:02 UTC
Created attachment 148149 [details] [review]
Bug 33210: (Bug 31963 follow-up) No hold fee message on OPAC should be displayed when there is no fee

After resolving Bug 31963 everything works as expected when there is
hold fee defined (!= 0).  But in case when the fee for given patron
category is set to 0.00, the user will always see the message "You will
be charged a hold fee of 0,00 ...", which is obviously not intended.

This is because categories.reservefee is returned from database as
'0.000000' and as such, without type casting, is interpreted as string
in Perl. Prior to Bug 31963 the result was compared to 0 before sending
anything to the template, so the casting was done, now it is not.

To test:
========
1. Go to Administration -> Patron categories
2. Edit your patron category and give a hold fee of 0.
3. HoldFeeMode does not matter - you can set it to any value.
4. In another tab, open the OPAC.
5. Search the OPAC for any record with an item.
6. Go to place a hold on this record.  You will see "You will be
   charged a hold fee of 0,00" --> This is a bug.
7. Apply patch and restart services.
6. Repeat step 6.
8. You should NOT see the hold fee message.

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Marcel de Rooy 2023-03-24 08:19:53 UTC
Please add a small unit test.
Comment 4 Janusz Kaczmarek 2023-03-24 16:21:14 UTC
Created attachment 148676 [details] [review]
Bug 33210: Add unit test

I'm not an expert in writing unit tests--so please correct it if it didn't conform to the guidelines.
Comment 5 David Nind 2023-04-04 21:56:44 UTC
Created attachment 149144 [details] [review]
Bug 33210: (Bug 31963 follow-up) No hold fee message on OPAC should be displayed when there is no fee

After resolving Bug 31963 everything works as expected when there is
hold fee defined (!= 0).  But in case when the fee for given patron
category is set to 0.00, the user will always see the message "You will
be charged a hold fee of 0,00 ...", which is obviously not intended.

This is because categories.reservefee is returned from database as
'0.000000' and as such, without type casting, is interpreted as string
in Perl. Prior to Bug 31963 the result was compared to 0 before sending
anything to the template, so the casting was done, now it is not.

To test:
========
1. Go to Administration -> Patron categories
2. Edit your patron category and give a hold fee of 0.
3. HoldFeeMode does not matter - you can set it to any value.
4. In another tab, open the OPAC.
5. Search the OPAC for any record with an item.
6. Go to place a hold on this record.  You will see "You will be
   charged a hold fee of 0,00" --> This is a bug.
7. Apply patch and restart services.
6. Repeat step 6.
8. You should NOT see the hold fee message.

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2023-04-04 21:56:46 UTC
Created attachment 149145 [details] [review]
Bug 33210: Add unit test

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2023-04-04 21:57:43 UTC
Testing notes (using KTD):

1. Tests pass before and after the patches are applied: prove t/db_dependent/Reserves/GetReserveFee.t
Comment 8 Nick Clemens 2023-04-17 18:41:33 UTC
Created attachment 149769 [details] [review]
Bug 33210: Add unit test

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick <nick@bywatersolutions.com>
Comment 9 Nick Clemens 2023-04-17 18:41:35 UTC
Created attachment 149770 [details] [review]
Bug 33210: (Bug 31963 follow-up) No hold fee message on OPAC should be displayed when there is no fee

After resolving Bug 31963 everything works as expected when there is
hold fee defined (!= 0).  But in case when the fee for given patron
category is set to 0.00, the user will always see the message "You will
be charged a hold fee of 0,00 ...", which is obviously not intended.

This is because categories.reservefee is returned from database as
'0.000000' and as such, without type casting, is interpreted as string
in Perl. Prior to Bug 31963 the result was compared to 0 before sending
anything to the template, so the casting was done, now it is not.

To test:
========
1. Go to Administration -> Patron categories
2. Edit your patron category and give a hold fee of 0.
3. HoldFeeMode does not matter - you can set it to any value.
4. In another tab, open the OPAC.
5. Search the OPAC for any record with an item.
6. Go to place a hold on this record.  You will see "You will be
   charged a hold fee of 0,00" --> This is a bug.
7. Apply patch and restart services.
6. Repeat step 6.
8. You should NOT see the hold fee message.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick <nick@bywatersolutions.com>
Comment 10 Tomás Cohen Arazi 2023-04-21 13:37:29 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 11 Martin Renvoize 2023-05-11 10:32:28 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release
Comment 12 Lucas Gass 2023-05-15 21:38:14 UTC
I accidentally pushed this for 22.05 and then realized it is dependent on 31963 so I reverted. 

Missing dependencies for 22.05.x, no backport.