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

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16849: Move IsDebarred to Koha::Patron->is_debarred (13.94 KB, patch)
2016-07-04 16:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16849: Move IsDebarred to Koha::Patron->is_debarred (14.10 KB, patch)
2016-07-06 14:52 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 16849: Move IsDebarred to Koha::Patron->is_debarred (14.16 KB, patch)
2016-07-09 13:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16849: Move IsDebarred to Koha::Patron->is_debarred (14.22 KB, patch)
2016-07-15 13:29 UTC, Tomás Cohen Arazi
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 15:56:19 UTC

    
Comment 1 Jonathan Druart 2016-07-04 16:11:37 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2016-07-05 12:04:52 UTC
Testing /4 Put a hold and make sure you get "Patron has restrictions":
Works on top of Bug 16854 - request.tt: Logic to display messages broken
Comment 3 Marc Véron 2016-07-05 14:08:10 UTC
Regarding /3 Make sure he cannot get a discharge

I added a manual restriction to a patron, and I can discharge her with and without patch.
Should discharges be blocked by restrictions?
Comment 4 Jonathan Druart 2016-07-06 13:54:50 UTC
(In reply to Marc Véron from comment #3)
> Regarding /3 Make sure he cannot get a discharge
> 
> I added a manual restriction to a patron, and I can discharge her with and
> without patch.
> Should discharges be blocked by restrictions?

Actually it's a bit tricky :)
The expected behavior is:
Discharge a patron for the first time will debar the patron and generate a discharge.
Click on discharge again: no new discharge will be generated (the pdf will, but no new entries in the discharges table).
Remove the debarment and generate a new discharge will create a new entry in the discharges table.
Comment 5 Marc Véron 2016-07-06 14:47:16 UTC
(In reply to Jonathan Druart from comment #4)
> (In reply to Marc Véron from comment #3)
> > Regarding /3 Make sure he cannot get a discharge
> > 
> > I added a manual restriction to a patron, and I can discharge her with and
> > without patch.
> > Should discharges be blocked by restrictions?
> 
> Actually it's a bit tricky :)
> The expected behavior is:
> Discharge a patron for the first time will debar the patron and generate a
> discharge.
> Click on discharge again: no new discharge will be generated (the pdf will,
> but no new entries in the discharges table).
> Remove the debarment and generate a new discharge will create a new entry in
> the discharges table.

Thanks, Jonathan, for the explanation.

Patch works as described and I will sign off.

For the records: 
IMO the behavior (with and without patch) is not optimal: 
- As far as I understand a patron who has got a discharge should not be able to check out items again (adding a restriction that never expires).
- However, if I put a manual restriction to this patron that expires let's say tomorrow, discharging will not add it's own restriction. The day after tomorrow, the patron will be able to check out again (but she has got a discharge).

What do you think about? Is that intended or is it a Bug?
Comment 6 Marc Véron 2016-07-06 14:52:12 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-07-06 15:45:24 UTC
(In reply to Marc Véron from comment #5)
> What do you think about? Is that intended or is it a Bug?

I think it's intended: Even if a patron got a discharge, (s)he could come back to the library (next year, or 5 years after).
Comment 8 Jonathan Druart 2016-07-09 13:11:28 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2016-07-09 13:11:50 UTC
Patch rebased against master.
Comment 10 Tomás Cohen Arazi 2016-07-15 13:29:21 UTC
Created attachment 53431 [details] [review]
Bug 16849: Move IsDebarred to Koha::Patron->is_debarred

In order to move IsMemberBlocked to Koha::Patron it makes sense to move
the code from Koha::Patron::Debarments::IsDebarred to
Koha::Patron->is_debarred.

Test plan:
1/ Add a restriction to a patron
2/ make sure he is not able to checkout items any more
3/ Make sure he cannot get a discharge
4/ Put a hold and make sure you get "Patron has restrictions"

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 11 Kyle M Hall 2016-07-15 18:08:43 UTC
Pushed to master for 16.11, thanks Jonathan!