Bug 13590 - Add ability to charge fines at start of charge period
Summary: Add ability to charge fines at start of charge period
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 9768 (view as bug list)
Depends on:
Blocks: 9129
  Show dependency treegraph
 
Reported: 2015-01-16 14:10 UTC by Kyle M Hall
Modified: 2016-06-21 21:40 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 13590 - Add ability to charge fines at start of charge period (20.05 KB, patch)
2015-01-16 14:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13590 - Add ability to charge fines at start of charge period (20.93 KB, patch)
2015-01-16 14:14 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13590 - Add ability to charge fines at start of charge period (23.03 KB, patch)
2015-01-16 14:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13590 - Add ability to charge fines at start of charge period (23.61 KB, patch)
2015-01-16 14:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13590 - Add ability to charge fines at start of charge period (23.67 KB, patch)
2015-01-16 14:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13590 - Add ability to charge fines at start of charge period (23.69 KB, patch)
2015-01-20 19:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13590 - Add ability to charge fines at start of charge period (22.39 KB, patch)
2015-04-15 15:40 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13590 - Add ability to charge fines at start of charge period (22.01 KB, patch)
2015-05-01 12:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13590 - Add ability to charge fines at start of charge period (22.08 KB, patch)
2015-07-24 11:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13590 - Add ability to charge fines at start of charge period (22.08 KB, patch)
2015-07-24 11:09 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13590 - Add ability to charge fines at start of charge period (22.07 KB, patch)
2015-07-24 11:12 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13590 - Add ability to charge fines at start of charge period (17.31 KB, patch)
2015-10-14 17:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13590 - Add ability to charge fines at start of charge period (17.38 KB, patch)
2015-10-14 17:31 UTC, Nicole C. Engard
Details | Diff | Splinter Review
Bug 13590 - Add ability to charge fines at start of charge period (17.43 KB, patch)
2015-10-15 10:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13590: Add missing th in tfoot (1.14 KB, patch)
2015-10-15 10:37 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 Kyle M Hall 2015-01-16 14:10:30 UTC
Right now, Koha only charges fines at the end of a given charge period. For example, let us assume a circulation rule has a charge period of one week ( 7 days ) and a fine of $5. This means that an item can be overdue for 6 days without accruing a fine. Koha should allow circulation rules to be configured to place the charge at the start of the end of the charge period so the library can decide when the fine should accrue.
Comment 1 Kyle M Hall 2015-01-16 14:11:41 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2015-01-16 14:14:26 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2015-01-16 14:15:23 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2015-01-16 14:17:12 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2015-01-16 14:19:01 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2015-01-16 14:34:23 UTC
Hi Kyle,

this sounds very interesting - how is the grace period treated?

Currently we have something like, if the grace period is < the charging interval, it won't have any effect, but only if it's > (iirc).

We have libraries that would like something like:
due date
2 days grace period 
1st fine 
7 days interval
2nd fine
7 days interval
3rd fine
Comment 7 Kyle M Hall 2015-01-16 14:41:14 UTC
Grace period behavior remains unchanged, but this patch should allow you to get the behavior you are looking for.

(In reply to Katrin Fischer from comment #6)
> Hi Kyle,
> 
> this sounds very interesting - how is the grace period treated?
> 
> Currently we have something like, if the grace period is < the charging
> interval, it won't have any effect, but only if it's > (iirc).
> 
> We have libraries that would like something like:
> due date
> 2 days grace period 
> 1st fine 
> 7 days interval
> 2nd fine
> 7 days interval
> 3rd fine
Comment 8 Kyle M Hall 2015-01-20 19:16:16 UTC Comment hidden (obsolete)
Comment 9 Mirko Tietgen 2015-04-13 16:31:45 UTC
Patch needs a rebase.

Apply? [(y)es, (n)o, (i)nteractive] i
Applying: Bug 13590 - Add ability to charge fines at start of charge period
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Circulation_Issuingrule.t
CONFLICT (content): Merge conflict in t/db_dependent/Circulation_Issuingrule.t
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging admin/smart-rules.pl
CONFLICT (content): Merge conflict in admin/smart-rules.pl
Auto-merging Koha/Schema/Result/Issuingrule.pm
CONFLICT (content): Merge conflict in Koha/Schema/Result/Issuingrule.pm
Auto-merging C4/Overdues.pm
CONFLICT (content): Merge conflict in C4/Overdues.pm
Failed to merge in the changes.
Comment 10 Kyle M Hall 2015-04-15 15:40:07 UTC Comment hidden (obsolete)
Comment 11 Aleisha Amohia 2015-04-22 05:29:10 UTC
When running updatedatabase.pl - 
Can't locate C4/Context.pm in @INC (@INC contains: /etc/perl /usr/local/lib/perl/5.14.2 /usr/local/share/perl/5.14.2 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.14 /usr/share/perl/5.14 /usr/local/lib/site_perl .) at updatedatabase.pl line 37.
BEGIN failed--compilation aborted at updatedatabase.pl line 37.

Hoping this wasn't just me testing this wrong, setting as Failed QA
Comment 12 Katrin Fischer 2015-04-22 05:31:20 UTC
Hi Aleisha,
you have to make sure your environment variables are set correctly or that you are using the koha shell (depending on how your Koha is set up) - I have checked the updatedatabase, it looks ok. Could you try again?
Comment 13 Kyle M Hall 2015-05-01 12:10:55 UTC Comment hidden (obsolete)
Comment 14 Mirko Tietgen 2015-07-03 07:27:09 UTC
(In reply to Kyle M Hall from comment #13)

> 3) prove t/db_dependent/Circulation_Issuingrule.t

t/db_dependent/Circulation_Issuingrule.t .. ok   
All tests successful.
Files=1, Tests=9,  2 wallclock secs ( 0.06 usr  0.02 sys +  1.98 cusr  0.23 csys =  2.29 CPU)
Result: PASS

> 4) prove t/db_dependent/Circulation.t

t/db_dependent/Circulation.t .. 22/61 DBD::mysql::st execute failed: Duplicate entry '4' for key 'PRIMARY' at /home/mirko/koha/C4/Circulation.pm line 2100.
DBD::mysql::st execute failed: Duplicate entry '4' for key 'PRIMARY' at /home/mirko/koha/C4/Circulation.pm line 2100.
# Looks like you planned 61 tests but ran 43.
# Looks like your test exited with 255 just after 43.
t/db_dependent/Circulation.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 18/61 subtests 


Test Summary Report
-------------------
t/db_dependent/Circulation.t (Wstat: 65280 Tests: 43 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 61 tests but ran 43.
Files=1, Tests=43,  5 wallclock secs ( 0.07 usr  0.03 sys +  3.02 cusr  0.40 csys =  3.52 CPU)
Result: FAIL


> 5) prove t/db_dependent/Fines.t

t/db_dependent/Fines.t .. DBIx::Class::ResultSet::create(): No such column chargeperiod_charge_at on Koha::Schema::Result::Issuingrule at t/db_dependent/Fines.t line 21
# Looks like your test exited with 255 before it could output anything.
t/db_dependent/Fines.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 5/5 subtests 

Test Summary Report
-------------------
t/db_dependent/Fines.t (Wstat: 65280 Tests: 0 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 5 tests but ran 0.
Files=1, Tests=0,  3 wallclock secs ( 0.05 usr  0.03 sys +  1.84 cusr  0.23 csys =  2.15 CPU)
Result: FAIL

I ran updatedatabase before I tested, so I wonder if something is missing?
Comment 15 Kyle M Hall 2015-07-06 16:33:01 UTC
(In reply to Mirko Tietgen from comment #14)
> (In reply to Kyle M Hall from comment #13)
> 
> > 3) prove t/db_dependent/Circulation_Issuingrule.t
> 
> t/db_dependent/Circulation_Issuingrule.t .. ok   
> All tests successful.
> Files=1, Tests=9,  2 wallclock secs ( 0.06 usr  0.02 sys +  1.98 cusr  0.23
> csys =  2.29 CPU)
> Result: PASS
> 
> > 4) prove t/db_dependent/Circulation.t
> 
> t/db_dependent/Circulation.t .. 22/61 DBD::mysql::st execute failed:
> Duplicate entry '4' for key 'PRIMARY' at /home/mirko/koha/C4/Circulation.pm
> line 2100.
> DBD::mysql::st execute failed: Duplicate entry '4' for key 'PRIMARY' at
> /home/mirko/koha/C4/Circulation.pm line 2100.
> # Looks like you planned 61 tests but ran 43.
> # Looks like your test exited with 255 just after 43.
> t/db_dependent/Circulation.t .. Dubious, test returned 255 (wstat 65280,
> 0xff00)
> Failed 18/61 subtests 
> 
> 
> Test Summary Report
> -------------------
> t/db_dependent/Circulation.t (Wstat: 65280 Tests: 43 Failed: 0)
>   Non-zero exit status: 255
>   Parse errors: Bad plan.  You planned 61 tests but ran 43.
> Files=1, Tests=43,  5 wallclock secs ( 0.07 usr  0.03 sys +  3.02 cusr  0.40
> csys =  3.52 CPU)
> Result: FAIL
> 
> 
> > 5) prove t/db_dependent/Fines.t
> 
> t/db_dependent/Fines.t .. DBIx::Class::ResultSet::create(): No such column
> chargeperiod_charge_at on Koha::Schema::Result::Issuingrule at
> t/db_dependent/Fines.t line 21
> # Looks like your test exited with 255 before it could output anything.
> t/db_dependent/Fines.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
> Failed 5/5 subtests 
> 
> Test Summary Report
> -------------------
> t/db_dependent/Fines.t (Wstat: 65280 Tests: 0 Failed: 0)
>   Non-zero exit status: 255
>   Parse errors: Bad plan.  You planned 5 tests but ran 0.
> Files=1, Tests=0,  3 wallclock secs ( 0.05 usr  0.03 sys +  1.84 cusr  0.23
> csys =  2.15 CPU)
> Result: FAIL
> 
> I ran updatedatabase before I tested, so I wonder if something is missing?

Did you run the unit tests with a fresh install of Koha with the default data?

Kyle
Comment 16 Kyle M Hall 2015-07-06 16:35:48 UTC
To follow up on that question, on my test server those unit tests often fail unless I'm using a fresh install with all the default data.
Comment 17 Kyle M Hall 2015-07-24 11:06:32 UTC Comment hidden (obsolete)
Comment 18 Kyle M Hall 2015-07-24 11:09:33 UTC Comment hidden (obsolete)
Comment 19 Kyle M Hall 2015-07-24 11:12:29 UTC Comment hidden (obsolete)
Comment 20 Nicole C. Engard 2015-10-14 16:49:17 UTC
11:48 ~/kohaclone (master % u=)$ git qa 13590
From git://git.koha-community.org/koha
 * branch            master     -> FETCH_HEAD
Branch bug13590-qa set up to track remote branch master from origin.
Switched to a new branch 'bug13590-qa'
Bug 13590 - Add ability to charge fines at start of charge period

41151 - Bug 13590 - Add ability to charge fines at start of charge period

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 13590 - Add ability to charge fines at start of charge period
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Circulation_Issuingrule.t
CONFLICT (content): Merge conflict in t/db_dependent/Circulation_Issuingrule.t
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging admin/smart-rules.pl
CONFLICT (content): Merge conflict in admin/smart-rules.pl
Auto-merging Koha/Schema/Result/Issuingrule.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 13590 - Add ability to charge fines at start of charge period
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-13590---Add-ability-to-charge-fines-at-start-o-yKwpQy.patch
11:48 ~/kohaclone (bug13590-qa *+%|AM u=)$
Comment 21 Kyle M Hall 2015-10-14 17:18:50 UTC Comment hidden (obsolete)
Comment 22 Nicole C. Engard 2015-10-14 17:31:57 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2015-10-15 10:37:31 UTC
Created attachment 43424 [details] [review]
Bug 13590 - Add ability to charge fines at start of charge period

Right now, Koha only charges fines at the end of a given charge period.
For example, let us assume a circulation rule has a charge period of one
week ( 7 days ) and a fine of $5. This means that an item can be overdue
for 6 days without accruing a fine. Koha should allow circulation rules
to be configured to place the charge at the start of the end of the
charge period so the library can decide when the fine should accrue.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) prove t/db_dependent/Circulation_Issuingrule.t
4) prove t/db_dependent/Circulation.t
5) prove t/db_dependent/Fines.t
6) Ensure you can still create/edit circulation rules

Signed-off-by: Daniel Grobani <dgrobani@samuelmerritt.edu>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 24 Jonathan Druart 2015-10-15 10:37:42 UTC
Created attachment 43425 [details] [review]
Bug 13590: Add missing th in tfoot

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 25 Katrin Fischer 2015-10-20 21:34:56 UTC
+1 for QA Jonathan - this was/is a wish list item for us!
Comment 26 Tomás Cohen Arazi 2015-10-23 12:56:11 UTC
Patches pushed to master.

Thanks Kyle!
Comment 27 Katrin Fischer 2015-10-23 13:06:03 UTC
*** Bug 9768 has been marked as a duplicate of this bug. ***