Bug 16850 - Move IsMemberBlocked to Koha::Patron
Summary: Move IsMemberBlocked to Koha::Patron
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 16849 16851
Blocks: 16846
  Show dependency treegraph
 
Reported: 2016-07-04 16:14 UTC by Jonathan Druart
Modified: 2017-06-14 22:15 UTC (History)
2 users (show)

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


Attachments
Bug 16850: Add test coverage for CanBookBeIssued (3.66 KB, patch)
2016-07-04 17:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16850: Remove C4::Members::IsMemberBlocked (4.52 KB, patch)
2016-07-04 17:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16850: Remove C4::Members::IsMemberBlocked (4.96 KB, patch)
2016-07-09 15:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16850: Add test coverage for CanBookBeIssued (3.66 KB, patch)
2016-07-20 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16850: Remove C4::Members::IsMemberBlocked (4.96 KB, patch)
2016-07-20 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16850: Add test coverage for CanBookBeIssued (3.76 KB, patch)
2016-08-04 16:18 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16850: Remove C4::Members::IsMemberBlocked (5.06 KB, patch)
2016-08-04 16:18 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 16850: Add test coverage for CanBookBeIssued (3.53 KB, patch)
2016-09-19 09:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16850: Remove C4::Members::IsMemberBlocked (4.93 KB, patch)
2016-09-19 09:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16850: Add test coverage for CanBookBeIssued (3.58 KB, patch)
2016-10-12 09:06 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16850: Remove C4::Members::IsMemberBlocked (4.99 KB, patch)
2016-10-12 09:06 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16850: Adjust number of tests (665 bytes, patch)
2016-10-24 11:47 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 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