Bug 16850

Summary: Move IsMemberBlocked to Koha::Patron
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: brendan, katrin.fischer
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 16849, 16851    
Bug Blocks: 16846    
Attachments: Bug 16850: Add test coverage for CanBookBeIssued
Bug 16850: Remove C4::Members::IsMemberBlocked
Bug 16850: Remove C4::Members::IsMemberBlocked
Bug 16850: Add test coverage for CanBookBeIssued
Bug 16850: Remove C4::Members::IsMemberBlocked
[SIGNED-OFF] Bug 16850: Add test coverage for CanBookBeIssued
[SIGNED-OFF] Bug 16850: Remove C4::Members::IsMemberBlocked
Bug 16850: Add test coverage for CanBookBeIssued
Bug 16850: Remove C4::Members::IsMemberBlocked
Bug 16850: Add test coverage for CanBookBeIssued
Bug 16850: Remove C4::Members::IsMemberBlocked
Bug 16850: Adjust number of tests

Description Jonathan Druart 2016-07-04 16:14:27 UTC
The C4::Members::IsMemberBlocked subroutine can be replaced by a call to Koha::Patron->is_debarred and Koha::Patron->has_overdues
Comment 1 Jonathan Druart 2016-07-04 17:06:52 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-07-04 17:06:55 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-07-09 15:08:17 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-07-20 13:45:42 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2016-07-20 13:45:45 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2016-08-04 16:18:06 UTC
Created attachment 54005 [details] [review]
[SIGNED-OFF] Bug 16850: Add test coverage for CanBookBeIssued

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 7 Owen Leonard 2016-08-04 16:18:14 UTC
Created attachment 54006 [details] [review]
[SIGNED-OFF] Bug 16850: Remove C4::Members::IsMemberBlocked

This subroutine is only called once and can be replaced with a call to
is_debarred and has_overdues.
Note that prior to this patch, IsMemberBlocked copy/paste code from
HasOverdues, which did not make sense.

Test plan:
Debar a patron and make sure he is not able to checkout (the librarian
is asked to overwrite if OverduesBlockCirc is set to 'confirmation')
Remove the debarment and add overdues to this patron, same as
previously, the checkout should be blocked

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 8 Katrin Fischer 2016-09-18 16:56:31 UTC
Sorry, patch doesn't apply:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 16850: Add test coverage for CanBookBeIssued
Using index info to reconstruct a base tree...
M	t/db_dependent/Circulation.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Circulation.t
Applying: Bug 16850: Remove C4::Members::IsMemberBlocked
fatal: sha1 information is lacking or useless (C4/Circulation.pm).
error: could not build fake ancestor

Also a question: We stopped using debarred/debarment in the GUI in favor of restricted/restrictions - should we not be using is_restricted?
Comment 9 Jonathan Druart 2016-09-19 09:01:37 UTC
Created attachment 55686 [details] [review]
Bug 16850: Add test coverage for CanBookBeIssued

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 10 Jonathan Druart 2016-09-19 09:01:44 UTC
Created attachment 55687 [details] [review]
Bug 16850: Remove C4::Members::IsMemberBlocked

This subroutine is only called once and can be replaced with a call to
is_debarred and has_overdues.
Note that prior to this patch, IsMemberBlocked copy/paste code from
HasOverdues, which did not make sense.

Test plan:
Debar a patron and make sure he is not able to checkout (the librarian
is asked to overwrite if OverduesBlockCirc is set to 'confirmation')
Remove the debarment and add overdues to this patron, same as
previously, the checkout should be blocked

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 11 Jonathan Druart 2016-09-19 09:04:12 UTC
(In reply to Katrin Fischer from comment #8)
> Also a question: We stopped using debarred/debarment in the GUI in favor of
> restricted/restrictions - should we not be using is_restricted?

IMO we should keep debar/debarment/debarred in the code, restrict is too vague. But I can be wrong.
Comment 12 Tomás Cohen Arazi 2016-10-12 09:06:48 UTC
Created attachment 56235 [details] [review]
Bug 16850: Add test coverage for CanBookBeIssued

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2016-10-12 09:06:54 UTC
Created attachment 56236 [details] [review]
Bug 16850: Remove C4::Members::IsMemberBlocked

This subroutine is only called once and can be replaced with a call to
is_debarred and has_overdues.
Note that prior to this patch, IsMemberBlocked copy/paste code from
HasOverdues, which did not make sense.

Test plan:
Debar a patron and make sure he is not able to checkout (the librarian
is asked to overwrite if OverduesBlockCirc is set to 'confirmation')
Remove the debarment and add overdues to this patron, same as
previously, the checkout should be blocked

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Brendan Gallagher 2016-10-13 15:32:48 UTC
Pushed to Master - Should be in the November 16.11 release.  Thanks!
Comment 15 Jonathan Druart 2016-10-24 11:47:43 UTC
Created attachment 56743 [details] [review]
Bug 16850: Adjust number of tests