---- 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
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!
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.
(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?
> 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.
(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?
(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.
(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?
(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.
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.
Created attachment 15045 [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. 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) Run t/db_dependent/Borrower_Debarments.t 4) Manually debar a patron, with an expiration date 5) Verify the patron cannot be issued to 6) Add another manual debarment with a different expiration date 7) Verify the 'restricted' message lists the date farthest into the future 8) Add another manual debarment with no expiration date 9) Verify the borrower is now debarred indefinitely 10) Delete the indefinite debarment 11) Verify the debarment message lists an expiration date dagain 12) Set an overdue notice to debar after 1 day of being overdue 13) Check out an item to a patron and backdate the due date to yesterday 14) Run overdue_notices.pl 15) Verify the OVERDUES debarment was created 16) Return the item 17) Verify the OVERDUES debarment was removed 18) Add issuing rules so that an overdue item causes a temporary debarment 19) Check out an item to a patron and backdate the due date by a year 20) Return the item 21) Verify the SUSPENSION debarment was added to the patron
Created attachment 15050 [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. 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) Set an overdue notice to debar after 1 day of being overdue 14) Check out an item to a patron and backdate the due date to yesterday 15) Run overdue_notices.pl 16) Verify the OVERDUES debarment was created 17) Return the item 18) Verify the OVERDUES debarment was removed 19) Add issuing rules so that an overdue item causes a temporary debarment 20) Check out an item to a patron and backdate the due date by a year 21) Return the item 22) Verify the SUSPENSION debarment was added to the patron
Created attachment 15051 [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. 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) Set an overdue notice to debar after 1 day of being overdue 14) Check out an item to a patron and backdate the due date to yesterday 15) Run overdue_notices.pl 16) Verify the OVERDUES debarment was created 17) Return the item 18) Verify the OVERDUES debarment was removed 19) Add issuing rules so that an overdue item causes a temporary debarment 20) Check out an item to a patron and backdate the due date by a year 21) Return the item 22) Verify the SUSPENSION debarment was added to the patron
Created attachment 15052 [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
*** Bug 1698 has been marked as a duplicate of this bug. ***
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"
Created attachment 16669 [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
I'm just changing status back to "Needs Signoff" for the new patch that Kyle attached on 3/21.
I'm sorry to say that this doesn't apply anymore.
Created attachment 18213 [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
Is this a new patch? It doesn't seem to address any of the issues I raised.
(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.
See comment #15.
Created attachment 19401 [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
(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!
Created attachment 19414 [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
Created attachment 19418 [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
Created attachment 19419 [details] [review] [SIGNED-OFF] 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>
Created attachment 19420 [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.
Created attachment 19421 [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>
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.
Created attachment 19755 [details] [review] Bug 2720: Unit tests improvements Create the patron. Use 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" 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!
Created attachment 20655 [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>
Created attachment 20656 [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>
Created attachment 20657 [details] [review] Bug 2720: Unit tests improvements Create the patron. Use a transaction.
Created attachment 20658 [details] [review] Bug 2720 - QA Followup 2
Created attachment 21366 [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>
Created attachment 21367 [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>
Created attachment 21368 [details] [review] Bug 2720: Unit tests improvements Create the patron. Use a transaction.
Created attachment 21369 [details] [review] Bug 2720 - QA Followup 2
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.
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>
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>
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>
Created attachment 21408 [details] [review] Bug 2720 - QA Followup 2 Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 21409 [details] [review] Bug 2720: QA followup - change license version to GPLv3+ Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
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>
Pushed to master. Thanks, Kyle!
Created attachment 25575 [details] [review] Bug 2720 - [Follow-up] Correct database update for debarments with 9999-12-31 The borrowers with infinite debarments have borrowers.debarred with '9999-12-31'. Database update for this bug contained : INSERT INTO borrower_debarments ( borrowernumber, expiration, comment ) SELECT borrowernumber, debarred, debarredcomment FROM borrowers WHERE debarred IS NOT NULL For borrowers where borrowers.debarred is '9999-12-31', this value is copied into borrower_debarments.expiration. This is not correct because borrower_debarments.expiration must be NULL for infinite debarments. This patch corrects update 3.13.00.035 and also adds an update for databases already updated.
Created attachment 25578 [details] [review] Bug 2720 - [Follow-up] Correct database update for debarments with 9999-12-31 Small correction in new update
I move the follow-up "Correct database update for debarments with 9999-12-31" to Bug 11846 in order to ease its management.
*** Bug 10353 has been marked as a duplicate of this bug. ***