Summary: | Batch patron deletion should not delete patrons with issues | ||
---|---|---|---|
Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
Component: | Tools | Assignee: | Galen Charlton <gmcharlt> |
Status: | CLOSED INVALID | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | jonathan.druart |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10612 | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Bug 15642 - Batch patron deletion should not delete patrons with issues
Bug 15642 - Batch patron deletion should not delete patrons with issues Bug 15642 - Batch patron deletion should not delete patrons with issues Bug 15642 - Batch patron deletion should not delete patrons with issues |
Description
Nick Clemens (kidclamp)
2016-01-22 01:37:38 UTC
Created attachment 47152 [details] [review] Bug 15642 - Batch patron deletion should not delete patrons with issues This patch changes _skip_borrowers_with_nonzero_balance to _skip_borrowers_with_nonzero_balance_or_issues and excludes borrowrs with issues from the batch deletion tool (this change brings tools/cleanborrowers.pl into line with misc/cronjobs/delete_patrons.pl with uses C4::Members GetBorrowersToExpunge which excludes patrons with current issues) Created attachment 47154 [details] [review] Bug 15642 - Batch patron deletion should not delete patrons with issues This patch changes _skip_borrowers_with_nonzero_balance to _skip_borrowers_with_nonzero_balance_or_issues and excludes borrowrs with issues from the batch deletion tool (this change brings tools/cleanborrowers.pl into line with misc/cronjobs/delete_patrons.pl with uses C4::Members GetBorrowersToExpunge which excludes patrons with current issues) Created attachment 47155 [details] [review] Bug 15642 - Batch patron deletion should not delete patrons with issues This patch changes _skip_borrowers_with_nonzero_balance to _skip_borrowers_with_nonzero_balance_or_issues and excludes borrowrs with issues from the batch deletion tool (this change brings tools/cleanborrowers.pl into line with misc/cronjobs/delete_patrons.pl with uses C4::Members GetBorrowersToExpunge which excludes patrons with current issues) Created attachment 47156 [details] [review] Bug 15642 - Batch patron deletion should not delete patrons with issues This patch changes _skip_borrowers_with_nonzero_balance to _skip_borrowers_with_nonzero_balance_or_issues and excludes borrowrs with issues from the batch deletion tool (this change brings tools/cleanborrowers.pl into line with misc/cronjobs/delete_patrons.pl with uses C4::Members GetBorrowersToExpunge which excludes patrons with current issues) Nick, tools/cleanborrowers.pl already call GetBorrowersToExpunge, so I am not sure this change is needed. What's the point of this _skip_borrowers_with_nonzero_balance subroutine actually? :) From my understanding, it removes the patrons with charges. In misc/cronjobs/delete_patrons.pl it's done with: 68 my $flags = C4::Members::patronflags( $member ); 69 if ( my $charges = $flags->{CHARGES}{amount} ) { 70 say "Failed to delete patron $borrowernumber: patron has $charges in fines"; 71 next; 72 } Should not we simply move this check to the GetBorrowersToExpunge subroutine? > Nick, tools/cleanborrowers.pl already call GetBorrowersToExpunge, so I am > not sure this change is needed. Agreed, not needed here, but needed on bug 10612, will move back over there (In reply to Jonathan Druart from comment #6) > In misc/cronjobs/delete_patrons.pl it's done with: > 68 my $flags = C4::Members::patronflags( $member ); > 69 if ( my $charges = $flags->{CHARGES}{amount} ) { > 70 say "Failed to delete patron $borrowernumber: patron has > $charges in fines"; > 71 next; > 72 } > Should not we simply move this check to the GetBorrowersToExpunge subroutine? FWIW I think I would rather see the checks done as above - so that the warning is visible that not all patrons in the category/list/etc were deleted. If the checks were moved into GetBorrowersToExpunge it would be nice to make it return patrons who wont be deleted because of issues/fines This is probably a separate enhancement though and I think this report can be closed. Agreed? |