Bug 31095

Summary: Remove Koha::Patron::Debarment::GetDebarments and use $patron->restrictions in preference
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: Architecture, internals, and plumbingAssignee: Martin Renvoize <martin.renvoize>
Status: RESOLVED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: enhancement    
Priority: P5 - low CC: andrewfh, dcook, jonathan.druart, kyle, lucas, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32398
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29145
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
**Sponsored by** *PTFS Europe*
Version(s) released in:
23.05.00,22.11.03
Bug Depends on: 23681    
Bug Blocks: 23295, 29145, 31458, 33574    
Attachments: Bug 31095: Add Koha::Patron::Restriction(s)
Bug 31095: Add Koha::Patron::Restriction(s)
Bug 31095: Add restrictions accessor to Koha::Patron
Bug 31095: Add type relation accessor to Koha::Patron::Restriction
Bug 31095: Remove GetDebarments from circ/circulation.pl
Bug 31095: Remove GetDebarments from members/moremember.pl
Bug 31095: Remove GetDebarments from members/memberentry.pl
Bug 31095: Remove borrower_debarments and patron_restrictions includes
Bug 31095: Remove keyed_on_code
Bug 31095: Remove GetDebarments from tools/modborrowers.pl
Bug 31095: Remove GetDebarments from Circulation.pm
Bug 31095: Add Koha::Patron::Restriction(s)
Bug 31095: Add restrictions accessor to Koha::Patron
Bug 31095: Add type relation accessor to Koha::Patron::Restriction
Bug 31095: Remove GetDebarments from circ/circulation.pl
Bug 31095: Remove GetDebarments from members/moremember.pl
Bug 31095: Remove GetDebarments from members/memberentry.pl
Bug 31095: Remove borrower_debarments and patron_restrictions includes
Bug 31095: Remove keyed_on_code
Bug 31095: Remove GetDebarments from tools/modborrowers.pl
Bug 31095: Remove GetDebarments from Circulation.pm
Bug 31095: Remove GetDebarments from Circulation.t
Bug 31095: Remove GetDebarments from Patron::Import
Bug 31095: Remove GetDebarments from maxsuspensiondays.t
Bug 31095: Remove GetDebarments from MarkIssueReturned.t
Bug 31095: Remove GetDebarments from Restriction/Type.t
Bug 31095: Remove GetDebarments from Borrower_Debarments.t
Bug 31095: Remove GetDebarments from Koha::Patron::Debarments
Bug 31095: Add Koha::Patron::Restriction(s)
Bug 31095: Add restrictions accessor to Koha::Patron
Bug 31095: Add type relation accessor to Koha::Patron::Restriction
Bug 31095: Remove GetDebarments from circ/circulation.pl
Bug 31095: Remove GetDebarments from members/moremember.pl
Bug 31095: Remove GetDebarments from members/memberentry.pl
Bug 31095: Remove borrower_debarments and patron_restrictions includes
Bug 31095: Remove keyed_on_code
Bug 31095: Remove GetDebarments from tools/modborrowers.pl
Bug 31095: Remove GetDebarments from Circulation.pm
Bug 31095: Remove GetDebarments from Circulation.t
Bug 31095: Remove GetDebarments from Patron::Import
Bug 31095: Remove GetDebarments from maxsuspensiondays.t
Bug 31095: Remove GetDebarments from MarkIssueReturned.t
Bug 31095: Remove GetDebarments from Restriction/Type.t
Bug 31095: Remove GetDebarments from Borrower_Debarments.t
Bug 31095: Remove GetDebarments from Koha::Patron::Debarments
Bug 31095: (follow-up) Iterate debarments in batch mod
Bug 31095: Add Koha::Patron::Restriction(s)
Bug 31095: Add restrictions accessor to Koha::Patron
Bug 31095: Add type relation accessor to Koha::Patron::Restriction
Bug 31095: Remove GetDebarments from circ/circulation.pl
Bug 31095: Remove GetDebarments from members/moremember.pl
Bug 31095: Remove GetDebarments from members/memberentry.pl
Bug 31095: Remove borrower_debarments and patron_restrictions includes
Bug 31095: Remove keyed_on_code
Bug 31095: Remove GetDebarments from tools/modborrowers.pl
Bug 31095: Remove GetDebarments from Circulation.pm
Bug 31095: Remove GetDebarments from Circulation.t
Bug 31095: Remove GetDebarments from Patron::Import
Bug 31095: Remove GetDebarments from maxsuspensiondays.t
Bug 31095: Remove GetDebarments from MarkIssueReturned.t
Bug 31095: Remove GetDebarments from Restriction/Type.t
Bug 31095: Remove GetDebarments from Borrower_Debarments.t
Bug 31095: Remove GetDebarments from Koha::Patron::Debarments
Bug 31095: (QA follow-up) Fix after rebase
Bug 31095: Add Koha::Patron::Restriction(s)
Bug 31095: Add restrictions accessor to Koha::Patron
Bug 31095: Add type relation accessor to Koha::Patron::Restriction
Bug 31095: Remove GetDebarments from circ/circulation.pl
Bug 31095: Remove GetDebarments from members/moremember.pl
Bug 31095: Remove GetDebarments from members/memberentry.pl
Bug 31095: Remove borrower_debarments and patron_restrictions includes
Bug 31095: Remove keyed_on_code
Bug 31095: Remove GetDebarments from tools/modborrowers.pl
Bug 31095: Remove GetDebarments from Circulation.pm
Bug 31095: Remove GetDebarments from Circulation.t
Bug 31095: Remove GetDebarments from Patron::Import
Bug 31095: Remove GetDebarments from maxsuspensiondays.t
Bug 31095: Remove GetDebarments from MarkIssueReturned.t
Bug 31095: Remove GetDebarments from Restriction/Type.t
Bug 31095: Remove GetDebarments from Borrower_Debarments.t
Bug 31095: Remove GetDebarments from Koha::Patron::Debarments
Bug 31095: (QA follow-up) Fix after rebase
Bug 31095: (follow-up) Iterate debarments in batch mod
Bug 31095: Add Koha::Patron::Restriction(s)
Bug 31095: Add restrictions accessor to Koha::Patron
Bug 31095: Add type relation accessor to Koha::Patron::Restriction
Bug 31095: Remove GetDebarments from circ/circulation.pl
Bug 31095: Remove GetDebarments from members/moremember.pl
Bug 31095: Remove GetDebarments from members/memberentry.pl
Bug 31095: Remove borrower_debarments and patron_restrictions includes
Bug 31095: Remove keyed_on_code
Bug 31095: Remove GetDebarments from tools/modborrowers.pl
Bug 31095: Remove GetDebarments from Circulation.pm
Bug 31095: Remove GetDebarments from Circulation.t
Bug 31095: Remove GetDebarments from Patron::Import
Bug 31095: Remove GetDebarments from maxsuspensiondays.t
Bug 31095: Remove GetDebarments from MarkIssueReturned.t
Bug 31095: Remove GetDebarments from Restriction/Type.t
Bug 31095: Remove GetDebarments from Borrower_Debarments.t
Bug 31095: Remove GetDebarments from Koha::Patron::Debarments
Bug 31095: (QA follow-up) Fix after rebase
Bug 31095: (follow-up) Iterate debarments in batch mod
Bug 31095: (QA follow-up) Fix POD
Bug 31095: DBIC additions and relation name tweak
Bug 31095: (Rebase follow-up) Correct fine rules and AddIssue calls
Bug 31095: [TO SQUASH] Fix Circulation.t

Description Martin Renvoize 2022-07-04 09:22:27 UTC
The Koha::Patron::Debarment module is written in the C4 style when it should really be a Koha::Object based class now.

We should update it to the modern practices to enable simple addition of API's and more standard use codebase wide.
Comment 1 Tomás Cohen Arazi 2022-08-22 17:09:05 UTC
Created attachment 139614 [details] [review]
Bug 31095: Add Koha::Patron::Restriction(s)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Martin Renvoize 2022-08-24 15:24:27 UTC
Created attachment 139719 [details] [review]
Bug 31095: Add Koha::Patron::Restriction(s)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Martin Renvoize 2022-08-24 15:24:31 UTC
Created attachment 139720 [details] [review]
Bug 31095: Add restrictions accessor to Koha::Patron
Comment 4 Martin Renvoize 2022-08-24 15:24:34 UTC
Created attachment 139721 [details] [review]
Bug 31095: Add type relation accessor to Koha::Patron::Restriction
Comment 5 Martin Renvoize 2022-08-24 15:24:38 UTC
Created attachment 139722 [details] [review]
Bug 31095: Remove GetDebarments from circ/circulation.pl

This patch remove the use of GetDebarments from circ/circulation.pl,
replacing it with a reference to patron.restrictions in the template and
includes.
Comment 6 Martin Renvoize 2022-08-24 15:24:41 UTC
Created attachment 139723 [details] [review]
Bug 31095: Remove GetDebarments from members/moremember.pl

This patch removes the use of GetDebarments from members/moremember.pl
and replaces template references with patrons.restrictions and the new
includes introduced in the prior patch
Comment 7 Martin Renvoize 2022-08-24 15:24:44 UTC
Created attachment 139724 [details] [review]
Bug 31095: Remove GetDebarments from members/memberentry.pl

This patch removes the use of GetDebarments from members/memberentry.pl
and replaces the references in the templates with patron.restrictions.
Comment 8 Martin Renvoize 2022-08-24 15:24:48 UTC
Created attachment 139725 [details] [review]
Bug 31095: Remove borrower_debarments and patron_restrictions includes

This patch removes the aforementioned includes and drops the last
remaining reference to them.

We have replaces these includes with patron-restrictions-tab.inc and
restriction-types.inc.
Comment 9 Martin Renvoize 2022-08-24 15:24:52 UTC
Created attachment 139726 [details] [review]
Bug 31095: Remove keyed_on_code

We no longer require keyed_on_code from Koha::Patron::Restrictoin::Types
as we have relation accessors for the relevent use cases and no longer
reference the method anywhere in the codebase!
Comment 10 Martin Renvoize 2022-08-24 15:31:20 UTC
Created attachment 139727 [details] [review]
Bug 31095: Remove GetDebarments from tools/modborrowers.pl
Comment 11 Martin Renvoize 2022-08-24 15:48:27 UTC
Created attachment 139728 [details] [review]
Bug 31095: Remove GetDebarments from Circulation.pm
Comment 12 Martin Renvoize 2022-08-24 15:49:08 UTC
Needs a thorough test plan.. I'm considering breaking the removal up.. one method at a time as this bug is already growing.
Comment 13 Tomás Cohen Arazi 2022-08-24 15:52:08 UTC
(In reply to Martin Renvoize from comment #12)
> Needs a thorough test plan.. I'm considering breaking the removal up.. one
> method at a time as this bug is already growing.

Go for it
Comment 14 Martin Renvoize 2022-08-25 09:31:23 UTC
Created attachment 139742 [details] [review]
Bug 31095: Add Koha::Patron::Restriction(s)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Martin Renvoize 2022-08-25 09:31:27 UTC
Created attachment 139743 [details] [review]
Bug 31095: Add restrictions accessor to Koha::Patron
Comment 16 Martin Renvoize 2022-08-25 09:31:30 UTC
Created attachment 139744 [details] [review]
Bug 31095: Add type relation accessor to Koha::Patron::Restriction
Comment 17 Martin Renvoize 2022-08-25 09:31:35 UTC
Created attachment 139745 [details] [review]
Bug 31095: Remove GetDebarments from circ/circulation.pl

This patch remove the use of GetDebarments from circ/circulation.pl,
replacing it with a reference to patron.restrictions in the template and
includes.

Test plan
1. Confirm that the 'Restrictions (x)' tab still appears on the checkout
   page for a user.
2. Confirm that the 'Restrictions (x)' tab count is correct
3. Confirm that the 'Restrictions (x)' tab table functions
4. Confirm that the 'Restrictions (x)' tab 'Add manual restriction' form
   works as expected
Comment 18 Martin Renvoize 2022-08-25 09:31:39 UTC
Created attachment 139746 [details] [review]
Bug 31095: Remove GetDebarments from members/moremember.pl

This patch removes the use of GetDebarments from members/moremember.pl
and replaces template references with patrons.restrictions and the new
includes introduced in the prior patch

Test plan
1. Confirm that the 'Restrictions (x)' tab still appears on the patron
   details page.
2. Confirm that the 'Restrictions (x)' tab count is correct
3. Confirm that the 'Restrictions (x)' tab table functions
4. Confirm that the 'Restrictions (x)' tab 'Add manual restriction' form
   works as expected
Comment 19 Martin Renvoize 2022-08-25 09:31:43 UTC
Created attachment 139747 [details] [review]
Bug 31095: Remove GetDebarments from members/memberentry.pl

This patch removes the use of GetDebarments from members/memberentry.pl
and replaces the references in the templates with patron.restrictions.

Test plan
1. Add a new user and confirm that the patron restrictions section does
   not appear on the form
2. Edit the user and confirm the patron restrictions section now appears
3. Add a manual restriction using the patron edit form
4. Confirm the restriction appears on the patron edit form
5. Confirm you can remove the restriction usine the patron edit form
Comment 20 Martin Renvoize 2022-08-25 09:31:46 UTC
Created attachment 139748 [details] [review]
Bug 31095: Remove borrower_debarments and patron_restrictions includes

This patch removes the aforementioned includes and drops the last
remaining reference to them.

We have replaces these includes with patron-restrictions-tab.inc and
restriction-types.inc.

Test plan
1. Confirm the includes are no longer referenced anywhere in the
   codebase. 'git grep borrower_debarments.inc', 'git grep
   patron_restrictions.inc'
Comment 21 Martin Renvoize 2022-08-25 09:31:50 UTC
Created attachment 139749 [details] [review]
Bug 31095: Remove keyed_on_code

We no longer require keyed_on_code from Koha::Patron::Restrictoin::Types
as we have relation accessors for the relevent use cases and no longer
reference the method anywhere in the codebase!

Test plan
1. Confirm 'keyed_on_code' is no longer referenced anywhere in the
   codebase.
   `git grep keyed_on_code`
Comment 22 Martin Renvoize 2022-08-25 09:31:54 UTC
Created attachment 139750 [details] [review]
Bug 31095: Remove GetDebarments from tools/modborrowers.pl

This patch removes GetDebarments from the modborrowers tool controller
and replaces it with $patron->restrictions as appropriate.

Test plan
1. Confirm that the 'Batch patron modification' tool pages still load
   and perform as expected.
Comment 23 Martin Renvoize 2022-08-25 09:31:58 UTC
Created attachment 139751 [details] [review]
Bug 31095: Remove GetDebarments from Circulation.pm

This patch removes GetDebarments from Circulation.pm replacing them with
calls to $patron->restrictions and filtering using a chained search
call.

Test plan
1. Confirm that t/db_dependant/Circulation.t continues to pass
Comment 24 Martin Renvoize 2022-08-25 09:32:02 UTC
Created attachment 139752 [details] [review]
Bug 31095: Remove GetDebarments from Circulation.t

Replace GetDebarments with $patron->restrictions.

Test plan
1. Run test prior to patch - all should pass
2. Run test after patch - all should pass
3. Confirm no tests were removed as part of the patch
Comment 25 Martin Renvoize 2022-08-25 09:32:07 UTC
Created attachment 139753 [details] [review]
Bug 31095: Remove GetDebarments from Patron::Import

This patch replaced GetDebarments with $patron->restrictions->search in
Koha::Patron::Import.

Test plan
1. Confirm that importing patrons with debarred and debarredcomment
   fields in the import still works as expected
Comment 26 Martin Renvoize 2022-08-25 09:32:11 UTC
Created attachment 139754 [details] [review]
Bug 31095: Remove GetDebarments from maxsuspensiondays.t

This patch replaces GetDebarments in maxsuspensiondays.t with calls to
the restrictions accessor.

Test plan
1. Confirm t/db_dependent/Circulation/maxsuspensiondays.t passes prior
   to this patch
2. Confirm t/db_dependent/Circulation/maxsuspensiondays.t passes after
   this patch
3. Confirm no tests have been removed in this patch
Comment 27 Martin Renvoize 2022-08-25 09:32:15 UTC
Created attachment 139755 [details] [review]
Bug 31095: Remove GetDebarments from MarkIssueReturned.t

This patch removes GetDebarments from Ciculation/MarkIssueReturned.t

Test plan
1. Confirm t/db_dependent/Circulation/MarkIssueReturned.t passes prior
   to the patch
2. Confirm t/db_dependent/Circulation/MarkIssueReturned.t passes after
   the patch
3. Confirm no tests have been removed
Comment 28 Martin Renvoize 2022-08-25 09:32:19 UTC
Created attachment 139756 [details] [review]
Bug 31095: Remove GetDebarments from Restriction/Type.t

This patch replaces GetDebarments with $patron->restrictions in
t/db_dependent/Koha/Patron/Restriction/Type.t

Test plan
1. Confirm t/db_dependent/Koha/Patron/Restriction/Type.t passes prior to
   the patch
2. Confirm t/db_dependent/Koha/Patron/Restriction/Type.t passes after
   the patch
3. Confirm no tests were removed
Comment 29 Martin Renvoize 2022-08-25 09:32:23 UTC
Created attachment 139757 [details] [review]
Bug 31095: Remove GetDebarments from Borrower_Debarments.t

This patch replaces calls to GetDebarments with teh
$patron->restritions->search() equivilents.

Test plan
1. Run the test prior to the patch and confirm it passes
2. Run the test after the patch and confirm it passes
3. Confirm no tests were removed
Comment 30 Martin Renvoize 2022-08-25 09:32:27 UTC
Created attachment 139758 [details] [review]
Bug 31095: Remove GetDebarments from Koha::Patron::Debarments

This patch finally removes GetDebarments from the codebase.

Test plan
1. Confirm GetDebarments is no longer mentioned in the codebase. `git
   grep GetDebarments`
2. Confirm t/db_dependent/Patron/Borrower_Debarments.t continues to pass
Comment 31 Martin Renvoize 2022-08-25 09:40:38 UTC
So I opted to break this into a series of smaller bugs rather than attack it all at once after realising how involved it was becoming.

Each patch here should be simple to read and simple to test in the hopes we can move quickly.
Comment 32 Nick Clemens 2022-09-16 16:08:49 UTC
Created attachment 140714 [details] [review]
Bug 31095: Add Koha::Patron::Restriction(s)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

bsoletes: 139747 - Bug 31095: Remove GetDebarments from members/memberentry.pl
Comment 33 Nick Clemens 2022-09-16 16:08:52 UTC
Created attachment 140715 [details] [review]
Bug 31095: Add restrictions accessor to Koha::Patron

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 34 Nick Clemens 2022-09-16 16:08:56 UTC
Created attachment 140716 [details] [review]
Bug 31095: Add type relation accessor to Koha::Patron::Restriction

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 35 Nick Clemens 2022-09-16 16:09:00 UTC
Created attachment 140717 [details] [review]
Bug 31095: Remove GetDebarments from circ/circulation.pl

This patch remove the use of GetDebarments from circ/circulation.pl,
replacing it with a reference to patron.restrictions in the template and
includes.

Test plan
1. Confirm that the 'Restrictions (x)' tab still appears on the checkout
   page for a user.
2. Confirm that the 'Restrictions (x)' tab count is correct
3. Confirm that the 'Restrictions (x)' tab table functions
4. Confirm that the 'Restrictions (x)' tab 'Add manual restriction' form
   works as expected

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 36 Nick Clemens 2022-09-16 16:09:04 UTC
Created attachment 140718 [details] [review]
Bug 31095: Remove GetDebarments from members/moremember.pl

This patch removes the use of GetDebarments from members/moremember.pl
and replaces template references with patrons.restrictions and the new
includes introduced in the prior patch

Test plan
1. Confirm that the 'Restrictions (x)' tab still appears on the patron
   details page.
2. Confirm that the 'Restrictions (x)' tab count is correct
3. Confirm that the 'Restrictions (x)' tab table functions
4. Confirm that the 'Restrictions (x)' tab 'Add manual restriction' form
   works as expected

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 37 Nick Clemens 2022-09-16 16:09:08 UTC
Created attachment 140719 [details] [review]
Bug 31095: Remove GetDebarments from members/memberentry.pl

This patch removes the use of GetDebarments from members/memberentry.pl
and replaces the references in the templates with patron.restrictions.

Test plan
1. Add a new user and confirm that the patron restrictions section does
   not appear on the form
2. Edit the user and confirm the patron restrictions section now appears
3. Add a manual restriction using the patron edit form
4. Confirm the restriction appears on the patron edit form
5. Confirm you can remove the restriction usine the patron edit form

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 38 Nick Clemens 2022-09-16 16:09:12 UTC
Created attachment 140720 [details] [review]
Bug 31095: Remove borrower_debarments and patron_restrictions includes

This patch removes the aforementioned includes and drops the last
remaining reference to them.

We have replaces these includes with patron-restrictions-tab.inc and
restriction-types.inc.

Test plan
1. Confirm the includes are no longer referenced anywhere in the
   codebase. 'git grep borrower_debarments.inc', 'git grep
   patron_restrictions.inc'

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 39 Nick Clemens 2022-09-16 16:09:16 UTC
Created attachment 140721 [details] [review]
Bug 31095: Remove keyed_on_code

We no longer require keyed_on_code from Koha::Patron::Restrictoin::Types
as we have relation accessors for the relevent use cases and no longer
reference the method anywhere in the codebase!

Test plan
1. Confirm 'keyed_on_code' is no longer referenced anywhere in the
   codebase.
   `git grep keyed_on_code`

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 40 Nick Clemens 2022-09-16 16:09:20 UTC
Created attachment 140722 [details] [review]
Bug 31095: Remove GetDebarments from tools/modborrowers.pl

This patch removes GetDebarments from the modborrowers tool controller
and replaces it with $patron->restrictions as appropriate.

Test plan
1. Confirm that the 'Batch patron modification' tool pages still load
   and perform as expected.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 41 Nick Clemens 2022-09-16 16:09:24 UTC
Created attachment 140723 [details] [review]
Bug 31095: Remove GetDebarments from Circulation.pm

This patch removes GetDebarments from Circulation.pm replacing them with
calls to $patron->restrictions and filtering using a chained search
call.

Test plan
1. Confirm that t/db_dependant/Circulation.t continues to pass

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 42 Nick Clemens 2022-09-16 16:09:28 UTC
Created attachment 140724 [details] [review]
Bug 31095: Remove GetDebarments from Circulation.t

Replace GetDebarments with $patron->restrictions.

Test plan
1. Run test prior to patch - all should pass
2. Run test after patch - all should pass
3. Confirm no tests were removed as part of the patch

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 43 Nick Clemens 2022-09-16 16:09:32 UTC
Created attachment 140725 [details] [review]
Bug 31095: Remove GetDebarments from Patron::Import

This patch replaced GetDebarments with $patron->restrictions->search in
Koha::Patron::Import.

Test plan
1. Confirm that importing patrons with debarred and debarredcomment
   fields in the import still works as expected

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 44 Nick Clemens 2022-09-16 16:09:37 UTC
Created attachment 140726 [details] [review]
Bug 31095: Remove GetDebarments from maxsuspensiondays.t

This patch replaces GetDebarments in maxsuspensiondays.t with calls to
the restrictions accessor.

Test plan
1. Confirm t/db_dependent/Circulation/maxsuspensiondays.t passes prior
   to this patch
2. Confirm t/db_dependent/Circulation/maxsuspensiondays.t passes after
   this patch
3. Confirm no tests have been removed in this patch

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 45 Nick Clemens 2022-09-16 16:09:41 UTC
Created attachment 140727 [details] [review]
Bug 31095: Remove GetDebarments from MarkIssueReturned.t

This patch removes GetDebarments from Ciculation/MarkIssueReturned.t

Test plan
1. Confirm t/db_dependent/Circulation/MarkIssueReturned.t passes prior
   to the patch
2. Confirm t/db_dependent/Circulation/MarkIssueReturned.t passes after
   the patch
3. Confirm no tests have been removed

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 46 Nick Clemens 2022-09-16 16:09:45 UTC
Created attachment 140728 [details] [review]
Bug 31095: Remove GetDebarments from Restriction/Type.t

This patch replaces GetDebarments with $patron->restrictions in
t/db_dependent/Koha/Patron/Restriction/Type.t

Test plan
1. Confirm t/db_dependent/Koha/Patron/Restriction/Type.t passes prior to
   the patch
2. Confirm t/db_dependent/Koha/Patron/Restriction/Type.t passes after
   the patch
3. Confirm no tests were removed

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 47 Nick Clemens 2022-09-16 16:09:50 UTC
Created attachment 140729 [details] [review]
Bug 31095: Remove GetDebarments from Borrower_Debarments.t

This patch replaces calls to GetDebarments with teh
$patron->restritions->search() equivilents.

Test plan
1. Run the test prior to the patch and confirm it passes
2. Run the test after the patch and confirm it passes
3. Confirm no tests were removed

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 48 Nick Clemens 2022-09-16 16:09:54 UTC
Created attachment 140730 [details] [review]
Bug 31095: Remove GetDebarments from Koha::Patron::Debarments

This patch finally removes GetDebarments from the codebase.

Test plan
1. Confirm GetDebarments is no longer mentioned in the codebase. `git
   grep GetDebarments`
2. Confirm t/db_dependent/Patron/Borrower_Debarments.t continues to pass

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 49 Nick Clemens 2022-09-16 16:09:58 UTC
Created attachment 140731 [details] [review]
Bug 31095: (follow-up) Iterate debarments in batch mod

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 50 Katrin Fischer 2022-10-07 14:44:09 UTC
Could you please check on the first fail especially? 

 FAIL	Koha/Patron/Restriction.pm
   FAIL	  forbidden patterns
		forbidden pattern: Warning: The 'sub type' may be wrong is declared in a Koha::* package (see bug 15446) (line 43)
   FAIL	  pod
		*** WARNING: empty section in previous paragraph  in file Koha/Patron/Restriction.pm

 OK	Koha/Patron/Restriction/Types.pm
 FAIL	Koha/Patron/Restrictions.pm
   FAIL	  pod
		*** WARNING: empty section in previous paragraph  in file Koha/Patron/Restrictions.pm
Comment 51 Katrin Fischer 2022-10-07 15:28:27 UTC
Created nice SUSPENSION and OVERDUE restrictions, but then got stuck on creating my own restriction types:

Template process failed: file error - restriction_type_description: not found at /kohadevbox/koha/C4/Templates.pm line 127
Comment 52 Martin Renvoize 2022-10-18 14:08:23 UTC Comment hidden (obsolete)
Comment 53 Martin Renvoize 2022-10-18 14:08:29 UTC Comment hidden (obsolete)
Comment 54 Martin Renvoize 2022-10-18 14:08:35 UTC Comment hidden (obsolete)
Comment 55 Martin Renvoize 2022-10-18 14:08:40 UTC Comment hidden (obsolete)
Comment 56 Martin Renvoize 2022-10-18 14:08:46 UTC Comment hidden (obsolete)
Comment 57 Martin Renvoize 2022-10-18 14:08:51 UTC Comment hidden (obsolete)
Comment 58 Martin Renvoize 2022-10-18 14:08:57 UTC Comment hidden (obsolete)
Comment 59 Martin Renvoize 2022-10-18 14:09:03 UTC Comment hidden (obsolete)
Comment 60 Martin Renvoize 2022-10-18 14:09:08 UTC Comment hidden (obsolete)
Comment 61 Martin Renvoize 2022-10-18 14:09:13 UTC Comment hidden (obsolete)
Comment 62 Martin Renvoize 2022-10-18 14:09:19 UTC Comment hidden (obsolete)
Comment 63 Martin Renvoize 2022-10-18 14:09:27 UTC Comment hidden (obsolete)
Comment 64 Martin Renvoize 2022-10-18 14:09:32 UTC Comment hidden (obsolete)
Comment 65 Martin Renvoize 2022-10-18 14:09:38 UTC Comment hidden (obsolete)
Comment 66 Martin Renvoize 2022-10-18 14:09:43 UTC Comment hidden (obsolete)
Comment 67 Martin Renvoize 2022-10-18 14:09:48 UTC Comment hidden (obsolete)
Comment 68 Martin Renvoize 2022-10-18 14:09:53 UTC Comment hidden (obsolete)
Comment 69 Martin Renvoize 2022-10-18 14:09:59 UTC Comment hidden (obsolete)
Comment 70 Martin Renvoize 2022-10-18 14:13:27 UTC
(In reply to Katrin Fischer from comment #51)
> Created nice SUSPENSION and OVERDUE restrictions, but then got stuck on
> creating my own restriction types:
> 
> Template process failed: file error - restriction_type_description: not
> found at /kohadevbox/koha/C4/Templates.pm line 127

Fixed by the latest rebase (see final commit.. we lost some things along the way somewhere)
Comment 71 Martin Renvoize 2022-10-18 14:14:02 UTC
(In reply to Katrin Fischer from comment #50)
> Could you please check on the first fail especially? 
> 
>  FAIL	Koha/Patron/Restriction.pm
>    FAIL	  forbidden patterns
> 		forbidden pattern: Warning: The 'sub type' may be wrong is declared in a
> Koha::* package (see bug 15446) (line 43)
>    FAIL	  pod
> 		*** WARNING: empty section in previous paragraph  in file
> Koha/Patron/Restriction.pm
> 
>  OK	Koha/Patron/Restriction/Types.pm
>  FAIL	Koha/Patron/Restrictions.pm
>    FAIL	  pod
> 		*** WARNING: empty section in previous paragraph  in file
> Koha/Patron/Restrictions.pm

These are false negatives.. the method name is correct here as we introduce a new 'type' relation.
Comment 72 Katrin Fischer 2022-10-27 21:40:32 UTC
Removing myself from QA contact to encourage someone else to take this while I might be too busy tomorrow.
Comment 73 Katrin Fischer 2022-10-30 10:16:40 UTC
I am sorry :(

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 31095: (follow-up) Iterate debarments in batch mod
error: sha1 information is lacking or useless (tools/modborrowers.pl).
error: could not build fake ancestor
Patch failed at 0001 Bug 31095: (follow-up) Iterate debarments in batch mod
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-31095-follow-up-Iterate-debarments-in-batch-mo-J4_1zl.patch
Comment 74 Martin Renvoize 2022-11-11 08:34:01 UTC Comment hidden (obsolete)
Comment 75 Martin Renvoize 2022-11-11 08:34:06 UTC Comment hidden (obsolete)
Comment 76 Martin Renvoize 2022-11-11 08:34:12 UTC Comment hidden (obsolete)
Comment 77 Martin Renvoize 2022-11-11 08:34:18 UTC Comment hidden (obsolete)
Comment 78 Martin Renvoize 2022-11-11 08:34:24 UTC Comment hidden (obsolete)
Comment 79 Martin Renvoize 2022-11-11 08:34:29 UTC Comment hidden (obsolete)
Comment 80 Martin Renvoize 2022-11-11 08:34:35 UTC Comment hidden (obsolete)
Comment 81 Martin Renvoize 2022-11-11 08:34:41 UTC Comment hidden (obsolete)
Comment 82 Martin Renvoize 2022-11-11 08:34:46 UTC Comment hidden (obsolete)
Comment 83 Martin Renvoize 2022-11-11 08:34:52 UTC Comment hidden (obsolete)
Comment 84 Martin Renvoize 2022-11-11 08:34:57 UTC Comment hidden (obsolete)
Comment 85 Martin Renvoize 2022-11-11 08:35:02 UTC Comment hidden (obsolete)
Comment 86 Martin Renvoize 2022-11-11 08:35:09 UTC Comment hidden (obsolete)
Comment 87 Martin Renvoize 2022-11-11 08:35:15 UTC Comment hidden (obsolete)
Comment 88 Martin Renvoize 2022-11-11 08:35:21 UTC Comment hidden (obsolete)
Comment 89 Martin Renvoize 2022-11-11 08:35:28 UTC Comment hidden (obsolete)
Comment 90 Martin Renvoize 2022-11-11 08:35:33 UTC Comment hidden (obsolete)
Comment 91 Martin Renvoize 2022-11-11 08:35:39 UTC Comment hidden (obsolete)
Comment 92 Martin Renvoize 2022-11-11 08:35:45 UTC Comment hidden (obsolete)
Comment 93 Martin Renvoize 2022-11-11 09:27:48 UTC
Patches were in the wrong order on bugzilla somehow.. fix that and done a minor rebase for the page-section bugs.
Comment 94 Martin Renvoize 2022-12-20 08:07:42 UTC
Any movement here.. it would be nice to get this off the ever increasing list of things I need to regularly rebase :|
Comment 95 Jonathan Druart 2023-01-02 14:57:59 UTC
Created attachment 144903 [details] [review]
Bug 31095: Add Koha::Patron::Restriction(s)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 96 Jonathan Druart 2023-01-02 14:58:06 UTC
Created attachment 144904 [details] [review]
Bug 31095: Add restrictions accessor to Koha::Patron

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 97 Jonathan Druart 2023-01-02 14:58:13 UTC
Created attachment 144905 [details] [review]
Bug 31095: Add type relation accessor to Koha::Patron::Restriction

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 98 Jonathan Druart 2023-01-02 14:58:20 UTC
Created attachment 144906 [details] [review]
Bug 31095: Remove GetDebarments from circ/circulation.pl

This patch remove the use of GetDebarments from circ/circulation.pl,
replacing it with a reference to patron.restrictions in the template and
includes.

Test plan
1. Confirm that the 'Restrictions (x)' tab still appears on the checkout
   page for a user.
2. Confirm that the 'Restrictions (x)' tab count is correct
3. Confirm that the 'Restrictions (x)' tab table functions
4. Confirm that the 'Restrictions (x)' tab 'Add manual restriction' form
   works as expected

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 99 Jonathan Druart 2023-01-02 14:58:27 UTC
Created attachment 144907 [details] [review]
Bug 31095: Remove GetDebarments from members/moremember.pl

This patch removes the use of GetDebarments from members/moremember.pl
and replaces template references with patrons.restrictions and the new
includes introduced in the prior patch

Test plan
1. Confirm that the 'Restrictions (x)' tab still appears on the patron
   details page.
2. Confirm that the 'Restrictions (x)' tab count is correct
3. Confirm that the 'Restrictions (x)' tab table functions
4. Confirm that the 'Restrictions (x)' tab 'Add manual restriction' form
   works as expected

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 100 Jonathan Druart 2023-01-02 14:58:33 UTC
Created attachment 144908 [details] [review]
Bug 31095: Remove GetDebarments from members/memberentry.pl

This patch removes the use of GetDebarments from members/memberentry.pl
and replaces the references in the templates with patron.restrictions.

Test plan
1. Add a new user and confirm that the patron restrictions section does
   not appear on the form
2. Edit the user and confirm the patron restrictions section now appears
3. Add a manual restriction using the patron edit form
4. Confirm the restriction appears on the patron edit form
5. Confirm you can remove the restriction usine the patron edit form

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 101 Jonathan Druart 2023-01-02 14:58:41 UTC
Created attachment 144909 [details] [review]
Bug 31095: Remove borrower_debarments and patron_restrictions includes

This patch removes the aforementioned includes and drops the last
remaining reference to them.

We have replaces these includes with patron-restrictions-tab.inc and
restriction-types.inc.

Test plan
1. Confirm the includes are no longer referenced anywhere in the
   codebase. 'git grep borrower_debarments.inc', 'git grep
   patron_restrictions.inc'

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 102 Jonathan Druart 2023-01-02 14:58:49 UTC
Created attachment 144910 [details] [review]
Bug 31095: Remove keyed_on_code

We no longer require keyed_on_code from Koha::Patron::Restrictoin::Types
as we have relation accessors for the relevent use cases and no longer
reference the method anywhere in the codebase!

Test plan
1. Confirm 'keyed_on_code' is no longer referenced anywhere in the
   codebase.
   `git grep keyed_on_code`

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 103 Jonathan Druart 2023-01-02 14:58:59 UTC
Created attachment 144911 [details] [review]
Bug 31095: Remove GetDebarments from tools/modborrowers.pl

This patch removes GetDebarments from the modborrowers tool controller
and replaces it with $patron->restrictions as appropriate.

Test plan
1. Confirm that the 'Batch patron modification' tool pages still load
   and perform as expected.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 104 Jonathan Druart 2023-01-02 14:59:07 UTC
Created attachment 144912 [details] [review]
Bug 31095: Remove GetDebarments from Circulation.pm

This patch removes GetDebarments from Circulation.pm replacing them with
calls to $patron->restrictions and filtering using a chained search
call.

Test plan
1. Confirm that t/db_dependant/Circulation.t continues to pass

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 105 Jonathan Druart 2023-01-02 14:59:14 UTC
Created attachment 144913 [details] [review]
Bug 31095: Remove GetDebarments from Circulation.t

Replace GetDebarments with $patron->restrictions.

Test plan
1. Run test prior to patch - all should pass
2. Run test after patch - all should pass
3. Confirm no tests were removed as part of the patch

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 106 Jonathan Druart 2023-01-02 14:59:21 UTC
Created attachment 144914 [details] [review]
Bug 31095: Remove GetDebarments from Patron::Import

This patch replaced GetDebarments with $patron->restrictions->search in
Koha::Patron::Import.

Test plan
1. Confirm that importing patrons with debarred and debarredcomment
   fields in the import still works as expected

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 107 Jonathan Druart 2023-01-02 14:59:27 UTC
Created attachment 144915 [details] [review]
Bug 31095: Remove GetDebarments from maxsuspensiondays.t

This patch replaces GetDebarments in maxsuspensiondays.t with calls to
the restrictions accessor.

Test plan
1. Confirm t/db_dependent/Circulation/maxsuspensiondays.t passes prior
   to this patch
2. Confirm t/db_dependent/Circulation/maxsuspensiondays.t passes after
   this patch
3. Confirm no tests have been removed in this patch

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 108 Jonathan Druart 2023-01-02 14:59:35 UTC
Created attachment 144916 [details] [review]
Bug 31095: Remove GetDebarments from MarkIssueReturned.t

This patch removes GetDebarments from Ciculation/MarkIssueReturned.t

Test plan
1. Confirm t/db_dependent/Circulation/MarkIssueReturned.t passes prior
   to the patch
2. Confirm t/db_dependent/Circulation/MarkIssueReturned.t passes after
   the patch
3. Confirm no tests have been removed

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 109 Jonathan Druart 2023-01-02 14:59:41 UTC
Created attachment 144917 [details] [review]
Bug 31095: Remove GetDebarments from Restriction/Type.t

This patch replaces GetDebarments with $patron->restrictions in
t/db_dependent/Koha/Patron/Restriction/Type.t

Test plan
1. Confirm t/db_dependent/Koha/Patron/Restriction/Type.t passes prior to
   the patch
2. Confirm t/db_dependent/Koha/Patron/Restriction/Type.t passes after
   the patch
3. Confirm no tests were removed

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 110 Jonathan Druart 2023-01-02 14:59:48 UTC
Created attachment 144918 [details] [review]
Bug 31095: Remove GetDebarments from Borrower_Debarments.t

This patch replaces calls to GetDebarments with teh
$patron->restritions->search() equivilents.

Test plan
1. Run the test prior to the patch and confirm it passes
2. Run the test after the patch and confirm it passes
3. Confirm no tests were removed

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 111 Jonathan Druart 2023-01-02 14:59:54 UTC
Created attachment 144919 [details] [review]
Bug 31095: Remove GetDebarments from Koha::Patron::Debarments

This patch finally removes GetDebarments from the codebase.

Test plan
1. Confirm GetDebarments is no longer mentioned in the codebase. `git
   grep GetDebarments`
2. Confirm t/db_dependent/Patron/Borrower_Debarments.t continues to pass

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 112 Jonathan Druart 2023-01-02 15:00:01 UTC
Created attachment 144920 [details] [review]
Bug 31095: (QA follow-up) Fix after rebase

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 113 Jonathan Druart 2023-01-02 15:00:08 UTC
Created attachment 144921 [details] [review]
Bug 31095: (follow-up) Iterate debarments in batch mod

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 114 Jonathan Druart 2023-01-02 16:25:00 UTC
1.
 FAIL   Koha/Patron/Restriction.pm
   FAIL   pod
                *** WARNING: empty section in previous paragraph  in file Koha/Patron/Restriction.pm

 FAIL   Koha/Patron/Restrictions.pm
   FAIL   pod
                *** WARNING: empty section in previous paragraph  in file Koha/Patron/Restrictions.pm

2. Koha/Patron/Restriction.pm:=head2 Relation accessors

Why? This is not used anywhere else. We use "Class methods".

3. 64                                    <option value="[% code | html %]">[% PROCESS restriction_type_description %]</option>

I think it's always better to pass the parameter, even if not needed
=> [% PROCESS restriction_type_description restriction_type => restriction_type %]

Nothing considering blocker, passing QA. Please go ahead with bug 31458 and friends, I will be happy to QA them!
Comment 115 Tomás Cohen Arazi 2023-01-30 15:59:24 UTC
Created attachment 145800 [details] [review]
Bug 31095: (QA follow-up) Fix POD

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 116 Tomás Cohen Arazi 2023-01-30 15:59:38 UTC
Created attachment 145801 [details] [review]
Bug 31095: DBIC additions and relation name tweak

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 117 Martin Renvoize 2023-01-30 17:29:56 UTC
Created attachment 145803 [details] [review]
Bug 31095: (Rebase follow-up) Correct fine rules and AddIssue calls

This patch corrects the errant call to AddIssue that passes a patron
object instread of a patron hash to AddIssue in the tests and also adds
lengthunit to squash a pair of warnings.

However, it doesn't resolve the failing test.. still somthing going on
there.
Comment 118 Jonathan Druart 2023-01-31 11:32:08 UTC
Created attachment 145845 [details] [review]
Bug 31095: [TO SQUASH] Fix Circulation.t
Comment 119 Jonathan Druart 2023-01-31 11:32:37 UTC
Please squash the last 2 patches.
Comment 120 Martin Renvoize 2023-01-31 12:07:26 UTC
Ha, thanks Jonathan.. my brain had hit melt point last night.. can't believe I missed those calls being the issue!
Comment 121 Tomás Cohen Arazi 2023-01-31 12:24:10 UTC
(In reply to Jonathan Druart from comment #118)
> Created attachment 145845 [details] [review] [review]
> Bug 31095: [TO SQUASH] Fix Circulation.t

Thanks, Jonathan.
Comment 122 Tomás Cohen Arazi 2023-01-31 13:23:22 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 123 Matt Blenkinsop 2023-02-01 15:41:36 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 124 Lucas Gass 2023-02-03 22:27:30 UTC
Missing dependencies for 22.05.x, no backport