Bug 2720 - Overdues which debar automatically should undebar automatically when returned
Summary: Overdues which debar automatically should undebar automatically when returned
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: 3.14
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---
Assignee: Fridolin SOMERS
QA Contact: Jonathan Druart
URL:
Whiteboard:
Keywords: rel_3_14_candidate
: 1698 10353 (view as bug list)
Depends on:
Blocks: 10863 11283 11287 11846 12134 12292 12760
  Show dependency treegraph
 
Reported: 2008-10-27 09:22 UTC by Chris Cormack
Modified: 2015-06-04 23:33 UTC (History)
15 users (show)

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


Attachments
Bug 2720 - Overdues which debar automatically should undebar automatically when returned (49.18 KB, patch)
2013-02-04 20:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 - Overdues which debar automatically should undebar automatically when returned (49.49 KB, patch)
2013-02-05 12:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 - Overdues which debar automatically should undebar automatically when returned (49.37 KB, patch)
2013-02-05 12:33 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 - Overdues which debar automatically should undebar automatically when returned (51.99 KB, patch)
2013-02-05 13:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 - Overdues which debar automatically should undebar automatically when returned (51.98 KB, patch)
2013-03-21 16:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 - Overdues which debar automatically should undebar automatically when returned (52.42 KB, patch)
2013-05-17 17:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 - Overdues which debar automatically should undebar automatically when returned (56.67 KB, patch)
2013-07-05 11:33 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 - Overdues which debar automatically should undebar automatically when returned (56.76 KB, patch)
2013-07-05 14:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 - Overdues which debar automatically should undebar automatically when returned (56.72 KB, patch)
2013-07-05 16:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 2720 - Overdues which debar automatically should undebar automatically when returned (56.82 KB, patch)
2013-07-05 16:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 2720 [FOLLOW-UP] Overdues which debar automatically should undebar automatically (12.81 KB, patch)
2013-07-05 17:12 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 2720 [FOLLOW-UP] Overdues which debar automatically should undebar automatically (12.82 KB, patch)
2013-07-05 17:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720: Unit tests improvements (1.98 KB, patch)
2013-07-18 10:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 2720 - Overdues which debar automatically should undebar automatically when returned (57.73 KB, patch)
2013-08-27 17:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 [FOLLOW-UP] Overdues which debar automatically should undebar automatically (12.82 KB, patch)
2013-08-27 17:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720: Unit tests improvements (1.98 KB, patch)
2013-08-27 17:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 - QA Followup 2 (1.75 KB, patch)
2013-08-27 17:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 - Overdues which debar automatically should undebar automatically when returned (54.85 KB, patch)
2013-09-23 12:25 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 [FOLLOW-UP] Overdues which debar automatically should undebar automatically (12.82 KB, patch)
2013-09-23 12:25 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720: Unit tests improvements (1.98 KB, patch)
2013-09-23 12:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 - QA Followup 2 (1.75 KB, patch)
2013-09-23 12:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 2720 - Overdues which debar automatically should undebar automatically when returned (54.91 KB, patch)
2013-09-24 10:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 2720 [FOLLOW-UP] Overdues which debar automatically should undebar automatically (12.88 KB, patch)
2013-09-24 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 2720: Unit tests improvements (2.04 KB, patch)
2013-09-24 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 2720 - QA Followup 2 (1.81 KB, patch)
2013-09-24 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 2720: QA followup - change license version to GPLv3+ (5.46 KB, patch)
2013-09-24 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 2720: FIX the "remove" links (1.94 KB, patch)
2013-09-24 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 2720 - [Follow-up] Correct database update for debarments with 9999-12-31 (2.35 KB, patch)
2014-02-24 11:28 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Bug 2720 - [Follow-up] Correct database update for debarments with 9999-12-31 (2.35 KB, patch)
2014-02-24 13:01 UTC, Fridolin SOMERS
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 00:55:06 UTC


---- Reported by joe.atzberger@liblime.com 2008-10-27 09:22:56 ----

Client suggested enhancment:

I have noticed something that seems illogical.  If a person owes a fine above a certain amount, the circulation is restricted.  If the fines are paid, the restriction is still there and must be removed in a separate action.  It seems that if a fine is paid, the restriction should be automatically lifted....



---- Additional Comments From oleonard@myacpl.org 2008-10-27 09:38:26 ----

How can you tell that the fine was the only thing restricting the patron? Maybe they also have overdues? Maybe the librarian also put a note on their account saying "This patron is restricted because of bad behavior." 



---- Additional Comments From oleonard@myacpl.org 2008-11-12 09:21:40 ----

Duplicate of http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=1698?



---- Additional Comments From dianajweaver@gmail.com 2009-05-15 21:29:35 ----

I would say that there should be an automatic debarment lift for returned overdue items. Debarment for fines or misbehavior may very well warrant staff intervention, but it would be useful if the return of overdue
books automatically cleared a patron's account (if that was the only reason for
the restriction). 



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:55 UTC  ---

This bug was previously known as _bug_ 2720 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=2720

Actual time not defined. Setting to 0.0
The original reporter of this bug does not have
   an account here. Reassigning to the person who moved
   it here: chris@bigballofwax.co.nz.
   Previous reporter was joe.atzberger@liblime.com.
CC member dianajweaver@gmail.com does not have an account here

Comment 1 Liz Rea 2011-07-28 15:07:51 UTC
NEKLS is seeking cosponsors for this development. We will be working with ByWater Solutions on the development, please contact either nexpresshelp@nekls.org or ByWater Solutions at staff@bywatersolutions.com to express your interest in cosponsoring.

Be sure to put in your message "interested in cosponsoring bug 2720 - fines that debar should undebar automatically when paid."

Looking forward to working with you!
Comment 2 Kyle M Hall 2012-06-14 13:28:01 UTC
Right now, Koha debars a borrower when borrowers.debarred is set to a date (
which is the date the borrower's debarment will expire ). An optional comment
is stored in borrowers.debarredocmment.

Since there are numerous reasons for debarment, not just fines, but overdues,
or a manual debarment, I imagine the best way to go about this is to add a
table borrower_debarment which would have the fields borrowernumber, reason,
date, comment. Reason would be an enumerated data type such as fines, overdues,
and manual.

When a borrowers fines go over the limit, a debarment for the borrower would go
into this table. The same for overdues and manual debarment. 

If a borrower has a least one row in this table, they will be debarred.

Paying fines will automatically remove a fines debarment, and returning overdue
books will automatically remove an overdues debarment.

This would require modifying all code that involves debarring, in particular
C4::Overdues::CheckBorrowerDebarred, the borrower editing page, and the
circulation pages.
Comment 3 Owen Leonard 2012-09-10 14:38:27 UTC
(In reply to comment #2)
> Since there are numerous reasons for debarment, not just fines, but overdues,
> or a manual debarment

Automatic debarment happens only through overdue triggers. Fines may block certain actions, but those checks are made based on the fine amount and fine-related system preferences ( OPACFineNoRenewals, maxoutstanding,  noissuescharge). Unless I'm missing something?

> If a borrower has a least one row in this table, they will be debarred.

Will this affect the suspension in days feature? Will the debarred column in the borrowers table stop being used, and all checks for patron debarment be referred to this new table? Is this going to significantly affect performance?

> This would require modifying all code that involves debarring, in particular
> C4::Overdues::CheckBorrowerDebarred, the borrower editing page, and the
> circulation pages.

If we keep borrowers.debarred, I suppose checkout performance doesn't have to be affected. I am concerned that this is going to slow down check-in. Upon each check-in Koha will have to query the borrower_debarment table to see if the patron should have the restriction removed?
Comment 4 Kyle M Hall 2012-09-10 18:32:03 UTC
> Automatic debarment happens only through overdue triggers. Fines may block
> certain actions, but those checks are made based on the fine amount and
> fine-related system preferences ( OPACFineNoRenewals, maxoutstanding, 
> noissuescharge). Unless I'm missing something?
> 
> Will this affect the suspension in days feature? Will the debarred column in
> the borrowers table stop being used, and all checks for patron debarment be
> referred to this new table? Is this going to significantly affect
> performance?
> 
> If we keep borrowers.debarred, I suppose checkout performance doesn't have
> to be affected. I am concerned that this is going to slow down check-in.
> Upon each check-in Koha will have to query the borrower_debarment table to
> see if the patron should have the restriction removed?

I would say we keep borrowers.debarred, but convert it to a boolean value. We can then have that value updated each time the debarments table is altered. I believe we will then need a nightly cronjob to remove expired debarments and update borrowers.debarred.
Comment 5 Owen Leonard 2012-09-10 19:00:07 UTC
(In reply to comment #4)
> I would say we keep borrowers.debarred, but convert it to a boolean value.

Convert it *back* to a boolean value! And move the date information into borrower_debarment?

> We can then have that value updated each time the debarments table is
> altered.

Which is when? Is there a line in borrower_debarment for each overdue item which is past the limit? And the table is reviewed each time an item is renewed or checked in?+

> I believe we will then need a nightly cronjob to remove expired
> debarments and update borrowers.debarred.

I don't understand. Are we talking about a cron job or is this something which updates each time something is checked in or renewed?
Comment 6 Kyle M Hall 2012-09-12 12:52:49 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > I would say we keep borrowers.debarred, but convert it to a boolean value.
> 
> Convert it *back* to a boolean value! And move the date information into
> borrower_debarment?

Yes. It will just be a quick check to know if the borrower is currently debarred without checking the debarments table.

> > We can then have that value updated each time the debarments table is
> > altered.
> 
> Which is when? Is there a line in borrower_debarment for each overdue item
> which is past the limit? And the table is reviewed each time an item is
> renewed or checked in?+

Each time a debarment is added to the borrower_debarments table, or a debarment is removed from the borrower_debarments table, a check will then be performed to determine if the borrower is currently debarred or not, and borrowers.debarred will be updated accordingly.

> > I believe we will then need a nightly cronjob to remove expired
> > debarments and update borrowers.debarred.
> 
> I don't understand. Are we talking about a cron job or is this something
> which updates each time something is checked in or renewed?

Right now, Koha checks the debarment field as a date to see if it is still valid. This will no longer be effective, as we will be able to have multiple debarments with different expiration dates. We can have a cronjob that runs nightly to remove any expired debarments, and reset the borrowers.debarment field to 0 if the borrower no longer has any active debarments.
Comment 7 Owen Leonard 2012-09-12 13:45:04 UTC
(In reply to comment #6)
> We can have a cronjob that runs
> nightly to remove any expired debarments, and reset the borrowers.debarment
> field to 0 if the borrower no longer has any active debarments.

If we're relying on a cron job to remove debarments, then I wonder why we bother with the extra table? Why not run a cron job which checks for borrowers with no overdues for whom borrowers.debarred = '9999-12-31' ? This would exclude patrons which were restricted under a suspension in days rule.

If we're trying to exclude patrons for whom borrowers.debarred was set manually, maybe we need to instead create a new flag for manual restriction?
Comment 8 Kyle M Hall 2012-09-12 14:23:48 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > We can have a cronjob that runs
> > nightly to remove any expired debarments, and reset the borrowers.debarment
> > field to 0 if the borrower no longer has any active debarments.
> 
> If we're relying on a cron job to remove debarments, then I wonder why we
> bother with the extra table? Why not run a cron job which checks for
> borrowers with no overdues for whom borrowers.debarred = '9999-12-31' ? This
> would exclude patrons which were restricted under a suspension in days rule.
> 
> If we're trying to exclude patrons for whom borrowers.debarred was set
> manually, maybe we need to instead create a new flag for manual restriction?

I guess my idea is to have a more flexible and extensible system. Having multiple debarment fields in the borrowers table seems kludgy to me. What if a borrower is debarred for different reasons at different branches? With my proposed design, we could allow patrons to be debarred at only specific libraries if we add a branch code to the debarments table. I guess I'm just making the case that it would be better to in the long run to develop a separate table rather than keep adding more fields to the borrowers table. If plack ever becomes the primary way to run Koha, we wouldn't even need a 'quick' flag in the borrowers table, as the result could be cached.
Comment 9 Kyle M Hall 2012-09-12 14:25:43 UTC
I should also note that if don't use a quick flag in the borrowers table, we wouldn't need a cronjob either. We could remove expired debarments when checking if the patron is currently debarred.
Comment 10 Kyle M Hall 2013-02-04 20:37:30 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2013-02-05 12:32:00 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall 2013-02-05 12:33:50 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2013-02-05 13:03:16 UTC Comment hidden (obsolete)
Comment 14 Owen Leonard 2013-02-08 17:19:03 UTC
*** Bug 1698 has been marked as a duplicate of this bug. ***
Comment 15 Owen Leonard 2013-03-06 20:32:14 UTC
Some test results:

- There is a typo in updatedatabase, a reversed end parenthesis.
- There is at least once instance of "debar" being used instead of "restrict" 
- I think the "X" icon isn't appropriate here. It would be simpler and more
  clear to use text ("Remove").
- The table of restrictions doesn't look correct if there are no
  restrictions--probably because there is nothing in <tbody>? Anyway
  I think it would be more consistent to separate the entry from from
  the table. Wrapping <form> around <tfoot> is invalid.
- onclick="return confirm(_('Remove debarment?'))" at line 24 of 
  borrower_debarments.inc is not translatable. The action should be moved
  to a function.
- borrower_debarments.inc has a couple of unescaped ampersands in line 24.

I don't think the "Restrictions" tab should be present on circulation.pl at all times. Being able to add a restriction to an account is certainly not something which happens so frequently that it needs to be only one click away. 

It's useful to be able to see information about all restrictions, but the full information could just as easily be displayed in the main block of "Cannot check out!" messages at the top. There's no reason to conserve space there if the patron is restricted anyway. If we mimicked the current method for displaying restrictions we could simply make it possible to display more than one line of restrictions, each with the same "lift restriction" button or link.

If we did this then we could move the form for adding a manual restriction either back into the patron edit form (where people still expect to find it) or onto its own page (not as good an option IMO).

One idea for an improvement: Perhaps the overdue notices job could set a restricted comment with the datetime of the job? "Restriction added by overdues process 2013-03-05 00:20:00"
Comment 16 Kyle M Hall 2013-03-21 16:00:14 UTC Comment hidden (obsolete)
Comment 17 Melia Meggs 2013-04-18 16:08:49 UTC
I'm just changing status back to "Needs Signoff" for the new patch that Kyle attached on 3/21.
Comment 18 Owen Leonard 2013-05-17 14:47:37 UTC
I'm sorry to say that this doesn't apply anymore.
Comment 19 Kyle M Hall 2013-05-17 17:05:09 UTC Comment hidden (obsolete)
Comment 20 Owen Leonard 2013-05-22 14:49:56 UTC
Is this a new patch? It doesn't seem to address any of the issues I raised.
Comment 21 Kyle M Hall 2013-05-22 14:59:05 UTC
(In reply to comment #20)
> Is this a new patch? It doesn't seem to address any of the issues I raised.

That issue? If you are referring to your question about the cronjob, that has been obviated and is no longer an issue, this patch doesn't have a cronjob.
Comment 22 Owen Leonard 2013-05-22 15:01:43 UTC
See comment #15.
Comment 23 Kyle M Hall 2013-07-05 11:33:44 UTC Comment hidden (obsolete)
Comment 24 Kyle M Hall 2013-07-05 11:40:47 UTC
(In reply to Owen Leonard from comment #15)
> - There is a typo in updatedatabase, a reversed end parenthesis.
Fixed!

> - There is at least once instance of "debar" being used instead of
> "restrict" 
Fixed!

> - I think the "X" icon isn't appropriate here. It would be simpler and more
>   clear to use text ("Remove").
Fixed!

> - The table of restrictions doesn't look correct if there are no
>   restrictions--probably because there is nothing in <tbody>? Anyway
>   I think it would be more consistent to separate the entry from from
>   the table. Wrapping <form> around <tfoot> is invalid.
> - onclick="return confirm(_('Remove debarment?'))" at line 24 of 
>   borrower_debarments.inc is not translatable. The action should be moved
>   to a function.
Fixed!

> - borrower_debarments.inc has a couple of unescaped ampersands in line 24.
Fixed!

> I don't think the "Restrictions" tab should be present on circulation.pl at
> all times. Being able to add a restriction to an account is certainly not
> something which happens so frequently that it needs to be only one click
> away. 
That is correct, it's there for removing restrictions primarily. However, I see no strong reason *not* to allow the ability to restrict a patron from here.

> It's useful to be able to see information about all restrictions, but the
> full information could just as easily be displayed in the main block of
> "Cannot check out!" messages at the top. There's no reason to conserve space
> there if the patron is restricted anyway. If we mimicked the current method
> for displaying restrictions we could simply make it possible to display more
> than one line of restrictions, each with the same "lift restriction" button
> or link.
As you write, it's useful, and this area appears to be more a matter of aesthetic preference. If you really dislike it, please submit a followup!
 
> If we did this then we could move the form for adding a manual restriction
> either back into the patron edit form (where people still expect to find it)
> or onto its own page (not as good an option IMO).
This latest edition of the patch adds the restrictions back to the patron editor to address this.

> One idea for an improvement: Perhaps the overdue notices job could set a
> restricted comment with the datetime of the job? "Restriction added by
> overdues process 2013-03-05 00:20:00"
Done!
Comment 25 Kyle M Hall 2013-07-05 14:05:33 UTC Comment hidden (obsolete)
Comment 26 Kyle M Hall 2013-07-05 16:32:36 UTC Comment hidden (obsolete)
Comment 27 Owen Leonard 2013-07-05 16:59:19 UTC Comment hidden (obsolete)
Comment 28 Owen Leonard 2013-07-05 17:12:01 UTC Comment hidden (obsolete)
Comment 29 Kyle M Hall 2013-07-05 17:50:47 UTC Comment hidden (obsolete)
Comment 30 Jonathan Druart 2013-07-18 10:10:31 UTC
QA comment:

1/ The unit tests should be improved: you suppose a borrower exists in the DB (and no borrower is added by sample data). Sql queries should be executed into a transaction.

2/ Step 8: The message concats 2 messages:

    Restricted: Patron's account is restricted until 04/08/2013 with the comment "im restricted im restricted2" View restrictions

message 1 is "im restricted" and message 2 is "im restricted2"

3/ Minor: double quote around sql queries

4/ There is no comment for suspension in the borrower_debarments table.

5/ Why don't you remove the debarred and debarredcomment field in the borrowers table?

I will provide a followup for unit tests

Marked as Failed QA for 2 and needs clarification for others points.
Comment 31 Jonathan Druart 2013-07-18 10:11:39 UTC Comment hidden (obsolete)
Comment 32 Kyle M Hall 2013-08-27 16:45:07 UTC
> 2/ Step 8: The message concats 2 messages:
> 
>     Restricted: Patron's account is restricted until 04/08/2013 with the
> comment "im restricted im restricted2" View restrictions
> 
> message 1 is "im restricted" and message 2 is "im restricted2"

Hmm, I think I should add a newline between the messages, and use the TT filter that will convert them to <br> tags. That should solve that.

> 4/ There is no comment for suspension in the borrower_debarments table.

The original functionality of suspensions did not have comments, so I did not add one to the suspension. Do you have a suggestion for the message?

> 5/ Why don't you remove the debarred and debarredcomment field in the
> borrowers table?

The first few comments on this bug explain it. TL;DR: By keeping them, we have to modify less code ( so fewer chances for bugs ) and it also makes everything faster as we don't have to recalculate everything each time we check for debarments, only when we modify debarments.

> I will provide a followup for unit tests

Thanks for the followup!
Comment 33 Kyle M Hall 2013-08-27 17:06:00 UTC Comment hidden (obsolete)
Comment 34 Kyle M Hall 2013-08-27 17:06:10 UTC Comment hidden (obsolete)
Comment 35 Kyle M Hall 2013-08-27 17:06:18 UTC Comment hidden (obsolete)
Comment 36 Kyle M Hall 2013-08-27 17:06:32 UTC Comment hidden (obsolete)
Comment 37 Kyle M Hall 2013-09-23 12:25:46 UTC Comment hidden (obsolete)
Comment 38 Kyle M Hall 2013-09-23 12:25:59 UTC Comment hidden (obsolete)
Comment 39 Kyle M Hall 2013-09-23 12:26:13 UTC Comment hidden (obsolete)
Comment 40 Kyle M Hall 2013-09-23 12:26:23 UTC Comment hidden (obsolete)
Comment 41 Jonathan Druart 2013-09-24 10:39:57 UTC
QA comment:
The feature works as expected.
I will provide 2 followups:
- fix GPL license
- fix a minor issue on the "remove" links

Note: I think fields debarred and debarredcomment should be removed later.

Marked as Passed QA.
Comment 42 Jonathan Druart 2013-09-24 10:40:56 UTC
Created attachment 21405 [details] [review]
Bug 2720 - Overdues which debar automatically should undebar automatically when returned

This patch adds a more extensible and flexible debarments system to Koha. The fields
borrowers.debarred and borrowers.debarredcomment are retained for compatibility and
speed.

This system supports having debarments for multiple reasons. There are currently
three types of debarments:
OVERDUES    - Generated by overdue_notices.pl if the notice should debar a patron
SUSPENSION  - A punative debarment generated on checkin via an issuing rule
MANUAL      - A debarment created manually by a librarian

OVERDUE debarments are cleared automatically when all overdue items have been returned,
if the new system preference AutoRemoveOverduesRestrictions is enabled. It is disabled
by default to retain current default functionality.

Whenever a borrowers debarments are modified, the system updates the borrowers debarment
fields with the highest expiration from all the borrowers debarments, and concatenates
the comments from the debarments together.

Test plan:
 1) Apply patch
 2) Run updatedatabase.pl
 3) Verify the borrower_debarments table has been created and
    populated with the pre-existing debarments
 4) Run t/db_dependent/Borrower_Debarments.t
 5) Manually debar a patron, with an expiration date
 6) Verify the patron cannot be issued to
 7) Add another manual debarment with a different expiration date
 8) Verify the 'restricted' message lists the date farthest into the future
 9) Add another manual debarment with no expiration date
10) Verify the borrower is now debarred indefinitely
11) Delete the indefinite debarment
12) Verify the debarment message lists an expiration date dagain
13) Enable the new system preference AutoRemoveOverduesRestrictions
14) Set an overdue notice to debar after 1 day of being overdue
15) Check out an item to a patron and backdate the due date to yesterday
16) Run overdue_notices.pl
17) Verify the OVERDUES debarment was created
18) Return the item
19) Verify the OVERDUES debarment was removed
20) Disable AutoRemoveOverduesRestrictions
21) Repeat steps 15 though 18, verify the OVERDUES debarment was *not* removed
22) Add issuing rules so that an overdue item causes a temporary debarment
23) Check out an item to a patron and backdate the due date by a year
24) Return the item
25) Verify the SUSPENSION debarment was added to the patron

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

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 43 Jonathan Druart 2013-09-24 10:41:01 UTC
Created attachment 21406 [details] [review]
Bug 2720 [FOLLOW-UP] Overdues which debar automatically should undebar automatically

This follow-up to Bug 2720 makes some template changes which I think
make the interface a little more consistent and streamlined.

Instead of incorporating the manual entry form into the table it is
broken out and follows standard form structure. This lets the table be
hidden altogether if there are no existing restrictions.

The manual entry form is hidden by default and shown when you click a
link to add a manual restriction.

These changes have been applied to both the include file used for
circulation and patron detail and to the patron entry form template.

To test, add and remove manual restrictions from the circulation page,
the patron detail page, and the patron edit page. All operations should
work correctly.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 44 Jonathan Druart 2013-09-24 10:41:08 UTC
Created attachment 21407 [details] [review]
Bug 2720: Unit tests improvements

Create the patron.
Use a transaction.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 45 Jonathan Druart 2013-09-24 10:41:13 UTC
Created attachment 21408 [details] [review]
Bug 2720 - QA Followup 2

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 46 Jonathan Druart 2013-09-24 10:41:19 UTC
Created attachment 21409 [details] [review]
Bug 2720: QA followup - change license version to GPLv3+

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 47 Jonathan Druart 2013-09-24 10:41:25 UTC
Created attachment 21410 [details] [review]
Bug 2720: FIX the "remove" links

If there are more than one restriction, the "remove" links don't display
the js alert.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 48 Galen Charlton 2013-10-31 02:01:56 UTC
Pushed to master.  Thanks, Kyle!
Comment 49 Fridolin SOMERS 2014-02-24 11:28:21 UTC Comment hidden (obsolete)
Comment 50 Fridolin SOMERS 2014-02-24 13:01:34 UTC Comment hidden (obsolete)
Comment 51 Fridolin SOMERS 2014-02-26 10:48:19 UTC
I move the follow-up "Correct database update for debarments with 9999-12-31" to Bug 11846 in order to ease its management.
Comment 52 Tomás Cohen Arazi 2014-11-03 14:34:32 UTC
*** Bug 10353 has been marked as a duplicate of this bug. ***