Bug 20815 - Add ability to choose if lost fee is refunded based on length of time item has been lost
Summary: Add ability to choose if lost fee is refunded based on length of time item ha...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 10 votes (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 25663
  Show dependency treegraph
 
Reported: 2018-05-24 17:16 UTC by Kyle M Hall
Modified: 2022-06-06 20:24 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/417
Text to go in the release notes:
Version(s) released in:
20.11.00


Attachments
Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost (15.77 KB, patch)
2020-02-12 19:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost (16.25 KB, patch)
2020-02-14 11:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost (16.32 KB, patch)
2020-02-14 20:58 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost (16.54 KB, patch)
2020-05-20 11:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20815: Do not use compare float with precision (3.28 KB, patch)
2020-07-06 11:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost (16.61 KB, patch)
2020-07-06 21:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20815: Do not use compare float with precision (3.32 KB, patch)
2020-07-06 21:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20815: (QA follow-up) Add message to database update / rephrase pref (1.89 KB, patch)
2020-07-06 21:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20815: Tests refactoring (15.97 KB, patch)
2020-07-21 16:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20815: Add unit tests for changes to AddIssue (8.16 KB, patch)
2020-07-22 12:54 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 Kyle M Hall 2018-05-24 17:16:18 UTC
1 - Add a enw system preference "Don'tForgiveLostAfter" which would allow specification of a range of time
2 - modify the check in routine to check the value of the itemlost_on field and only refund if before this time
3 - Add feedback for circulation staff to alert that item was lost but fee is not refunded
Comment 1 Katrin Fischer 2018-05-24 21:12:29 UTC
Hi Kyle, can you explain what FY18 means?
Comment 2 Kyle M Hall 2018-05-25 14:34:30 UTC
(In reply to Katrin Fischer from comment #1)
> Hi Kyle, can you explain what FY18 means?

I have no idea, maybe JesseM does ; )

I have updated the title to actually describe the requires.
Comment 3 Brian 2019-04-24 00:48:50 UTC
Any updates on bug 20815?

Carnegie team
Comment 4 Brian 2019-05-16 23:27:15 UTC
FY18 means Fiscal year 2018

Hi this bug is attached to our development New System Pref, “Don’t forgive lost after(set date), modify checkin routine to check the value of the itemlost_on field.

My superiors want this bug pushed, thank you
Comment 5 Joy Nelson 2019-05-17 12:22:38 UTC
Hi Brian,
We understand the urgency for your library.  This bug is currently assigned and in development.  When the feature is ready, tested and passed QA it will be pushed into a release of Koha.  

Thanks!
joy

(In reply to Brian from comment #4)
> FY18 means Fiscal year 2018
> 
> Hi this bug is attached to our development New System Pref, “Don’t forgive
> lost after(set date), modify checkin routine to check the value of the
> itemlost_on field.
> 
> My superiors want this bug pushed, thank you
Comment 6 Kyle M Hall 2020-02-12 19:31:28 UTC
Created attachment 98772 [details] [review]
Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost

This adds the ability to not refund lost item fees on return if the item
has been lost for more than a given number of days.

Test Plan:
1) Set the new system preference NoRefundOnLostReturnedItemsAge to a number of days
2) Find a lost item that has been lost longer than that NoRefundOnLostReturnedItemsAge days which would have otherwise been refunded
3) Return the item
4) Note no refund on the lost item fee was processed, the fee remains unchanged
5) prove t/db_dependent/Circulation.t
Comment 7 Kyle M Hall 2020-02-14 11:56:19 UTC
Created attachment 98929 [details] [review]
Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost

This adds the ability to not refund lost item fees on return if the item
has been lost for more than a given number of days.

Test Plan:
1) Set the new system preference NoRefundOnLostReturnedItemsAge to a number of days
2) Find a lost item that has been lost longer than that NoRefundOnLostReturnedItemsAge days which would have otherwise been refunded
3) Return the item
4) Note no refund on the lost item fee was processed, the fee remains unchanged
5) prove t/db_dependent/Circulation.t
Comment 8 ByWater Sandboxes 2020-02-14 20:58:20 UTC
Created attachment 99038 [details] [review]
Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost

This adds the ability to not refund lost item fees on return if the item
has been lost for more than a given number of days.

Test Plan:
1) Set the new system preference NoRefundOnLostReturnedItemsAge to a number of days
2) Find a lost item that has been lost longer than that NoRefundOnLostReturnedItemsAge days which would have otherwise been refunded
3) Return the item
4) Note no refund on the lost item fee was processed, the fee remains unchanged
5) prove t/db_dependent/Circulation.t

Signed-off-by: Deb Stephenson <DStephen@dubuque.lib.ia.us>
Comment 9 Jonathan Druart 2020-04-24 10:10:02 UTC
CONFLICT (content): Merge conflict in t/db_dependent/Circulation.t
CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql
Comment 10 Kyle M Hall 2020-05-20 11:10:15 UTC
Created attachment 105141 [details] [review]
Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost

This adds the ability to not refund lost item fees on return if the item
has been lost for more than a given number of days.

Test Plan:
1) Set the new system preference NoRefundOnLostReturnedItemsAge to a number of days
2) Find a lost item that has been lost longer than that NoRefundOnLostReturnedItemsAge days which would have otherwise been refunded
3) Return the item
4) Note no refund on the lost item fee was processed, the fee remains unchanged
5) prove t/db_dependent/Circulation.t

Signed-off-by: Deb Stephenson <DStephen@dubuque.lib.ia.us>
Comment 11 Katrin Fischer 2020-07-05 01:13:47 UTC
Please fix!

 FAIL	t/db_dependent/Circulation.t
   OK	  critic
   FAIL	  forbidden patterns
		forbidden pattern: Do not use compare float with precision (bug 24408). Ignore this if you know what you are doing. (line 3998)
		forbidden pattern: Do not use compare float with precision (bug 24408). Ignore this if you know what you are doing. (line 4001)
		forbidden pattern: Do not use compare float with precision (bug 24408). Ignore this if you know what you are doing. (line 4073)
		forbidden pattern: Do not use compare float with precision (bug 24408). Ignore this if you know what you are doing. (line 4076)
		forbidden pattern: Do not use compare float with precision (bug 24408). Ignore this if you know what you are doing. (line 4148)
		forbidden pattern: Do not use compare float with precision (bug 24408). Ignore this if you know what you are doing. (line 4151)
		forbidden pattern: Do not use compare float with precision (bug 24408). Ignore this if you know what you are doing. (line 4223)
		forbidden pattern: Do not use compare float with precision (bug 24408). Ignore this if you know what you are doing. (line 4226)
Comment 12 Katrin Fischer 2020-07-05 01:14:13 UTC
Well, or check at least :)
Comment 13 Kyle M Hall 2020-07-06 11:06:28 UTC
Created attachment 106575 [details] [review]
Bug 20815: Do not use compare float with precision
Comment 14 Katrin Fischer 2020-07-06 21:04:35 UTC
Created attachment 106606 [details] [review]
Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost

This adds the ability to not refund lost item fees on return if the item
has been lost for more than a given number of days.

Test Plan:
1) Set the new system preference NoRefundOnLostReturnedItemsAge to a number of days
2) Find a lost item that has been lost longer than that NoRefundOnLostReturnedItemsAge days which would have otherwise been refunded
3) Return the item
4) Note no refund on the lost item fee was processed, the fee remains unchanged
5) prove t/db_dependent/Circulation.t

Signed-off-by: Deb Stephenson <DStephen@dubuque.lib.ia.us>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2020-07-06 21:04:39 UTC
Created attachment 106607 [details] [review]
Bug 20815: Do not use compare float with precision

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Katrin Fischer 2020-07-06 21:04:43 UTC
Created attachment 106608 [details] [review]
Bug 20815: (QA follow-up) Add message to database update / rephrase pref

- Make pref use "checked in" instead of "returned".
- Add message to database update

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Jonathan Druart 2020-07-21 16:46:56 UTC
Created attachment 107165 [details] [review]
Bug 20815: Tests refactoring

Some code were duplicated.
Comment 18 Jonathan Druart 2020-07-22 08:17:44 UTC
Hi Kyle,
In my understanding the tests don't cover the changes in AddIssue, can you provide some please?
Comment 19 Kyle M Hall 2020-07-22 12:54:51 UTC
Created attachment 107177 [details] [review]
Bug 20815: Add unit tests for changes to AddIssue
Comment 20 Jonathan Druart 2020-07-23 09:49:09 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 21 Lucas Gass 2020-07-28 15:43:06 UTC
enhancement will not be backported to 20.05.x series
Comment 22 Caroline Cyr La Rose 2020-08-20 20:43:32 UTC
I changed the component from "Architecture, internals and plumbing" to "System administration" so that it appears under Administration in the release notes. I hope this is ok. If not, feel free to change it back :)