Bug 9076 - Add ability to delete borrowers by expiration date and category code to cleanborrowers.pl
Summary: Add ability to delete borrowers by expiration date and category code to clean...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Jonathan Druart
URL:
Keywords:
: 6404 (view as bug list)
Depends on: 7919
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-14 16:39 UTC by Kyle M Hall
Modified: 2016-01-05 15:46 UTC (History)
5 users (show)

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


Attachments
Bug 9076 - Add ability to delete borrowers by expiration date and category code to cleanborrowers.pl (11.12 KB, patch)
2012-11-14 16:40 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9076 - Followup - Perltidy cleanborrowers.pl (7.92 KB, patch)
2012-11-14 16:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9076 - Add ability to delete borrowers by expiration date and category code to cleanborrowers.pl (11.19 KB, patch)
2012-11-16 16:40 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9076 - Followup - Perltidy cleanborrowers.pl (7.92 KB, patch)
2012-11-16 16:40 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9076 - Add ability to delete borrowers by expiration date and category code to cleanborrowers.pl (11.28 KB, patch)
2012-11-20 02:01 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 9076 - Followup - Perltidy cleanborrowers.pl (7.92 KB, patch)
2012-11-20 02:04 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 9076: Followup: FIX ergonomic issues (2.75 KB, patch)
2012-11-20 13:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9076: Followup: FIX ergonomic issues (2.83 KB, patch)
2012-11-20 13:46 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9076 - QA Followup - Rename GetBorrowersWhoHaveNotBorrowedSince to GetBorrowersToExpunge (3.31 KB, patch)
2012-12-11 19:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9076 - Add ability to delete borrowers by expiration date and category code to cleanborrowers.pl (11.34 KB, patch)
2013-01-04 08:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9076 - Followup - Perltidy cleanborrowers.pl (7.98 KB, patch)
2013-01-04 08:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9076: Followup: FIX ergonomic issues (2.86 KB, patch)
2013-01-04 08:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9076 - QA Followup - Rename GetBorrowersWhoHaveNotBorrowedSince to GetBorrowersToExpunge (3.37 KB, patch)
2013-01-04 08:40 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2012-11-14 16:39:47 UTC

    
Comment 1 Kyle M Hall 2012-11-14 16:40:09 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2012-11-14 16:41:15 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2012-11-14 16:45:41 UTC
Test plan:
1) Apply patch
2) Attempt to delete borrowers using each combination of criteria:
  a) Hasn't borrowed since
  b) Expiration date
  c) Category code
  d) a & b
  e) a & c
  f) b & c
  g) a & b & c
Comment 4 Liz Rea 2012-11-16 02:34:06 UTC
With the sample data, there are 5 staff patrons defined (or so - I had 6 but I believe I added one more). This tool, when asked to delete staff patrons, indicated there were no patrons that met the criteria of "category: staff." The other categories worked as expected.

Can this tool not delete staff patrons? Is that by design? If it is by design, then that categorycode needs to be removed from the list.
Comment 5 Kyle M Hall 2012-11-16 13:23:42 UTC
(In reply to comment #4)
> With the sample data, there are 5 staff patrons defined (or so - I had 6 but
> I believe I added one more). This tool, when asked to delete staff patrons,
> indicated there were no patrons that met the criteria of "category: staff."
> The other categories worked as expected.
> 
> Can this tool not delete staff patrons? Is that by design? If it is by
> design, then that categorycode needs to be removed from the list.

That is correct. It has never been able to delete staff patrons. I would assume that was a 'safety' feature.

I'll add a followup to remove staff from the list.
Comment 6 Kyle M Hall 2012-11-16 16:40:09 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2012-11-16 16:40:21 UTC Comment hidden (obsolete)
Comment 8 Kyle M Hall 2012-11-16 16:40:37 UTC
Fixed.
Comment 9 Liz Rea 2012-11-20 02:01:22 UTC Comment hidden (obsolete)
Comment 10 Liz Rea 2012-11-20 02:04:07 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2012-11-20 13:37:17 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall 2012-11-20 13:46:49 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2012-11-20 14:04:32 UTC
QA Comment:

Great improvements for the clean borrowers tool.
Patch clean.

Marked as Passed QA.
Comment 14 Jared Camins-Esakov 2012-11-26 00:11:13 UTC
I have a question about your changes to the GetBorrowersWhoHaveNotBorrowedSince routine. It looks to me like you have changed the routine so that it will use a number of different criteria to decide whether a borrower should be deleted. Is that correct? If so, shouldn't we change the name to reflect what the routine actually does? Perhaps something like "GetBorrowersToExpunge" would more accurately describe what it does now?
Comment 15 Kyle M Hall 2012-12-03 17:11:26 UTC
(In reply to comment #14)
> I have a question about your changes to the
> GetBorrowersWhoHaveNotBorrowedSince routine. It looks to me like you have
> changed the routine so that it will use a number of different criteria to
> decide whether a borrower should be deleted. Is that correct? If so,
> shouldn't we change the name to reflect what the routine actually does?
> Perhaps something like "GetBorrowersToExpunge" would more accurately
> describe what it does now?

I totally agree. I'll submit a followup to fix this unless someone else beats me to it.
Comment 16 Kyle M Hall 2012-12-11 19:08:19 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2012-12-27 10:07:04 UTC
This patch is broken by Bug 7919 (categorycode is null so we cannot filter by patron category)
Comment 18 Kyle M Hall 2013-01-03 13:45:16 UTC
I'm seeing the category pulldown populated correctly. Can you give me a bit more detail about the problem?

(In reply to comment #17)
> This patch is broken by Bug 7919 (categorycode is null so we cannot filter
> by patron category)
Comment 19 Jonathan Druart 2013-01-03 14:01:13 UTC
(In reply to comment #18)
> I'm seeing the category pulldown populated correctly. Can you give me a bit
> more detail about the problem?

The fix has been pushed so the problem does not exist anymore.
Comment 20 Liz Rea 2013-01-04 02:58:30 UTC
so this still needs a QA look please. :)
Comment 21 Jonathan Druart 2013-01-04 08:38:22 UTC
Last patch passes QA.
The feature still works.
Comment 22 Jonathan Druart 2013-01-04 08:39:39 UTC
Created attachment 14406 [details] [review]
Bug 9076 - Add ability to delete borrowers by expiration date and category code to cleanborrowers.pl

Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Tested per plan, works. Staff category is not shown, and patch passes automated testing.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 23 Jonathan Druart 2013-01-04 08:39:54 UTC
Created attachment 14407 [details] [review]
Bug 9076 - Followup - Perltidy cleanborrowers.pl

Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 24 Jonathan Druart 2013-01-04 08:40:02 UTC
Created attachment 14408 [details] [review]
Bug 9076: Followup: FIX ergonomic issues

If no criteria is given, the form was submitted.
Labels were wrong

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 25 Jonathan Druart 2013-01-04 08:40:08 UTC
Created attachment 14409 [details] [review]
Bug 9076 - QA Followup - Rename GetBorrowersWhoHaveNotBorrowedSince to GetBorrowersToExpunge

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 26 Jared Camins-Esakov 2013-01-08 03:04:18 UTC
This patch has been pushed to master, though not without some concern about the layout. Liz has promised a speedy follow-up, so I will open another bug about it.
Comment 27 Jacek Ablewicz 2014-02-24 12:15:08 UTC
*** Bug 6404 has been marked as a duplicate of this bug. ***