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.
Created attachment 139614 [details] [review] Bug 31095: Add Koha::Patron::Restriction(s) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 139719 [details] [review] Bug 31095: Add Koha::Patron::Restriction(s) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 139720 [details] [review] Bug 31095: Add restrictions accessor to Koha::Patron
Created attachment 139721 [details] [review] Bug 31095: Add type relation accessor to Koha::Patron::Restriction
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.
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
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.
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.
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!
Created attachment 139727 [details] [review] Bug 31095: Remove GetDebarments from tools/modborrowers.pl
Created attachment 139728 [details] [review] Bug 31095: Remove GetDebarments from Circulation.pm
Needs a thorough test plan.. I'm considering breaking the removal up.. one method at a time as this bug is already growing.
(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
Created attachment 139742 [details] [review] Bug 31095: Add Koha::Patron::Restriction(s) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 139743 [details] [review] Bug 31095: Add restrictions accessor to Koha::Patron
Created attachment 139744 [details] [review] Bug 31095: Add type relation accessor to Koha::Patron::Restriction
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
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
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
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'
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`
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.
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
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
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
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
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
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
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
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
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.
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
Created attachment 140715 [details] [review] Bug 31095: Add restrictions accessor to Koha::Patron Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 140716 [details] [review] Bug 31095: Add type relation accessor to Koha::Patron::Restriction Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Created attachment 140731 [details] [review] Bug 31095: (follow-up) Iterate debarments in batch mod Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
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
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
Created attachment 142059 [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>
Created attachment 142060 [details] [review] Bug 31095: Add restrictions accessor to Koha::Patron Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 142061 [details] [review] Bug 31095: Add type relation accessor to Koha::Patron::Restriction Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 142062 [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>
Created attachment 142063 [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>
Created attachment 142064 [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>
Created attachment 142065 [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>
Created attachment 142066 [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>
Created attachment 142067 [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>
Created attachment 142068 [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>
Created attachment 142069 [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>
Created attachment 142070 [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>
Created attachment 142071 [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>
Created attachment 142072 [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>
Created attachment 142073 [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>
Created attachment 142074 [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>
Created attachment 142075 [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>
Created attachment 142076 [details] [review] Bug 31095: (QA follow-up) Fix after rebase Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(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)
(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.
Removing myself from QA contact to encourage someone else to take this while I might be too busy tomorrow.
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
Created attachment 143745 [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>
Created attachment 143746 [details] [review] Bug 31095: Add restrictions accessor to Koha::Patron Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 143747 [details] [review] Bug 31095: Add type relation accessor to Koha::Patron::Restriction Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 143748 [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>
Created attachment 143749 [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>
Created attachment 143750 [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>
Created attachment 143751 [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>
Created attachment 143752 [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>
Created attachment 143753 [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>
Created attachment 143754 [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>
Created attachment 143755 [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>
Created attachment 143756 [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>
Created attachment 143757 [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>
Created attachment 143758 [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>
Created attachment 143759 [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>
Created attachment 143760 [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>
Created attachment 143761 [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>
Created attachment 143762 [details] [review] Bug 31095: (QA follow-up) Fix after rebase Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 143763 [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>
Patches were in the wrong order on bugzilla somehow.. fix that and done a minor rebase for the page-section bugs.
Any movement here.. it would be nice to get this off the ever increasing list of things I need to regularly rebase :|
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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!
Created attachment 145800 [details] [review] Bug 31095: (QA follow-up) Fix POD Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 145801 [details] [review] Bug 31095: DBIC additions and relation name tweak Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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.
Created attachment 145845 [details] [review] Bug 31095: [TO SQUASH] Fix Circulation.t
Please squash the last 2 patches.
Ha, thanks Jonathan.. my brain had hit melt point last night.. can't believe I missed those calls being the issue!
(In reply to Jonathan Druart from comment #118) > Created attachment 145845 [details] [review] [review] > Bug 31095: [TO SQUASH] Fix Circulation.t Thanks, Jonathan.
Pushed to master for 23.05. Nice work everyone, thanks!
Nice work everyone! Pushed to stable for 22.11.x
Missing dependencies for 22.05.x, no backport