Bugzilla – Attachment 28129 Details for
Bug 11975
Tools: Batch patron deletion code should be improved
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[PASSED QA] Bug 11975: improve the batch patron deletion code
PASSED-QA-Bug-11975-improve-the-batch-patron-delet.patch (text/plain), 14.30 KB, created by
Kyle M Hall (khall)
on 2014-05-09 12:20:01 UTC
(
hide
)
Description:
[PASSED QA] Bug 11975: improve the batch patron deletion code
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2014-05-09 12:20:01 UTC
Size:
14.30 KB
patch
obsolete
>From d9be11d9f0ac79f470d2034a0493b828d8dbce2d Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >Date: Thu, 20 Mar 2014 16:41:29 +0100 >Subject: [PATCH] [PASSED QA] Bug 11975: improve the batch patron deletion code > >This patch tries to improve the code for the cleanborrowers.pl tool. > >- use KohaDates plugin and Koha::DateUtils for date management, > removing a dependency on C4::Dates >- replace variables step1, step2 and step3 with step >- add a JavaScript check if no action if checked > >Test plan: >- Backup your DB >- Play with the Patron deletion/anonymisation tool and try to find > something inconsistent. >If you don't find something different, it works! > >Signed-off-by: Galen Charlton <gmc@esilibrary.com> > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > .../prog/en/modules/tools/cleanborrowers.tt | 33 +++-- > tools/cleanborrowers.pl | 132 ++++++++++++++------ > 2 files changed, 110 insertions(+), 55 deletions(-) > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt >index 8a14187..bd505fb 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt >@@ -1,5 +1,6 @@ >+[% USE KohaDates %] > [% INCLUDE 'doc-head-open.inc' %] >-<title>Koha › Tools › Batch patron deletion/anonymization [% IF ( step2 ) %]› Confirm[% END %][% IF ( step3 ) %]› Finished[% END %]</title> >+<title>Koha › Tools › Batch patron deletion/anonymization [% IF step == 2 %]› Confirm[% END %][% IF step == 3 %]› Finished[% END %]</title> > [% INCLUDE 'doc-head-close.inc' %] > [% INCLUDE 'calendar.inc' %] > <script type="text/javascript"> >@@ -21,9 +22,13 @@ > return false; > } > } >+ if(!form.checkbox[0].checked && !form.checkbox[1].checked) { >+ alert( _("Please check at least one action") ); >+ return false; >+ } > return true; > } >- >+ > /** > * checkForm2(form) > * This function check the form2 is correctly filled. >@@ -39,7 +44,7 @@ > [% INCLUDE 'header.inc' %] > [% INCLUDE 'cat-search.inc' %] > >-<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/tools/tools-home.pl">Tools</a> › [% IF ( step1 ) %]Clean Patron Records[% ELSE %]<a href="/cgi-bin/koha/tools/cleanborrowers.pl">Clean patron records</a> › [% END %][% IF ( step2 ) %]Confirm[% END %][% IF ( step3 ) %]Finished[% END %]</div> >+<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/tools/tools-home.pl">Tools</a> › [% IF step == 1 %]Clean Patron Records[% ELSE %]<a href="/cgi-bin/koha/tools/cleanborrowers.pl">Clean patron records</a> › [% END %][% IF step == 2 %]Confirm[% END %][% IF step == 3 %]Finished[% END %]</div> > > <div id="doc3" class="yui-t2"> > >@@ -51,7 +56,7 @@ > <div class="help"> > <p>This tool allows you to delete patrons and anonymize checkout history. For deleting patrons, any combination of limits can be used.</p> > </div> >-[% IF ( step1 ) %] >+[% IF step == 1 %] > <!-- step 1 START --> > <div id="step1"> > <form name="f1" onsubmit="return checkForm(this);" action="/cgi-bin/koha/tools/cleanborrowers.pl" method="post"> >@@ -63,7 +68,7 @@ > <ul> > <li> > <label for="date1">who have not borrowed since:</label> >- <input size="10" id="date1" name="filterdate1" type="text" class="datepicker" /> >+ <input size="10" id="date1" name="not_borrowed_since" type="text" class="datepicker" /> > <span class="hint">[% INCLUDE 'date-format.inc' %]</span> > </li> > <li> >@@ -91,13 +96,13 @@ > <ul> > <li> > <label for="date2">Permanently delete checkout history older than</label> >- <input size="10" id="date2" name="filterdate2" type="text" class="datepicker" /> >+ <input size="10" id="date2" name="last_issue_date" type="text" class="datepicker" /> > <span class="hint">[% INCLUDE 'date-format.inc' %]</span> > </li> > </ul> > > <!-- hidden here --> >- <input type="hidden" name="step2" value="1" /> >+ <input type="hidden" name="step" value="2" /> > </fieldset> > <fieldset class="action"><input type="submit" value="Next >>" /></fieldset> > </form> >@@ -105,7 +110,7 @@ > <!-- step 1 END --> > [% END %] > >-[% IF ( step2 ) %] >+[% IF step == 2 %] > <!-- STEP 2 START --> > <div id="step2"> > <form name="f2" action="/cgi-bin/koha/tools/cleanborrowers.pl" method="post" onsubmit="return checkForm2(this);"> >@@ -135,10 +140,10 @@ > <input type="hidden" name="do_anonym" value="[% totalToAnonymize %]" /> > [% END %] > >- <input type="hidden" name="step3" value="1" /> >- <input type="hidden" name="filterdate1" value="[% filterdate1 %]" /> >- <input type="hidden" name="filterdate2" value="[% filterdate2 %]" /> >- <input type="hidden" name="borrower_dateexpiry" value="[% borrower_dateexpiry %]" /> >+ <input type="hidden" name="step" value="3" /> >+ <input type="hidden" name="not_borrowed_since" value="[% not_borrowed_since | $KohaDates %]" /> >+ <input type="hidden" name="last_issue_date" value="[% last_issue_date | $KohaDates %]" /> >+ <input type="hidden" name="borrower_dateexpiry" value="[% borrower_dateexpiry | $KohaDates %]" /> > <input type="hidden" name="borrower_categorycode" value="[% borrower_categorycode %]" /> > </fieldset> > <fieldset class="action"><input type="submit" value="Finish" /> <a class="cancel" href="/cgi-bin/koha/tools/cleanborrowers.pl">Cancel</a></fieldset> >@@ -147,7 +152,7 @@ > <!-- STEP 2 END --> > [% END %] > >-[% IF ( step3 ) %] >+[% IF step == 3 %] > <!-- Step 3 START --> > > <div id="step3"> >@@ -166,7 +171,7 @@ > [% END %] > [% END %] > [% IF ( do_anonym ) %] >- <h4>All patrons with checkouts older than [% filterdate1 %] have been anonymized</h4> >+ <h4>All patrons with checkouts older than [% last_issue_date | $KohaDates %] have been anonymized</h4> > [% ELSE %] > <h4>No patron records have been anonymized</h4> > [% END %] >diff --git a/tools/cleanborrowers.pl b/tools/cleanborrowers.pl >index 9c1977b..f6ebcc2 100755 >--- a/tools/cleanborrowers.pl >+++ b/tools/cleanborrowers.pl >@@ -37,10 +37,10 @@ use strict; > use CGI; > use C4::Auth; > use C4::Output; >-use C4::Dates qw/format_date format_date_in_iso/; > use C4::Members; # GetBorrowersWhoHavexxxBorrowed. > use C4::Circulation; # AnonymiseIssueHistory. > use C4::VirtualShelves (); #no import >+use Koha::DateUtils qw( dt_from_string output_pref ); > use Date::Calc qw/Today Add_Delta_YM/; > > my $cgi = new CGI; >@@ -51,10 +51,21 @@ my $cgi = new CGI; > # * multivalued CGI paramaters are returned as a packaged string separated by "\0" (null) > my $params = $cgi->Vars; > >-my $filterdate1; # the date which filter on issue history. >-my $filterdate2; # the date which filter on borrowers last issue. >-my $borrower_dateexpiry; >-my $borrower_categorycode; >+my $step = $params->{step} || 1; >+my $not_borrowered_since = # the date which filter on issue history. >+ $params->{not_borrowered_since} >+ ? dt_from_string $params->{not_borrowered_since} >+ : undef; >+my $last_issue_date = # the date which filter on borrowers last issue. >+ $params->{last_issue_date} >+ ? dt_from_string $params->{last_issue_date} >+ : undef; >+my $borrower_dateexpiry = >+ $params->{borrower_dateexpiry} >+ ? dt_from_string $params->{borrower_dateexpiry} >+ : undef; >+ >+my $borrower_categorycode = $params->{'borrower_categorycode'} || q{}; > > # getting the template > my ( $template, $loggedinuser, $cookie ) = get_template_and_user( >@@ -66,19 +77,48 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( > } > ); > >-if ( $params->{'step2'} ) { >- $filterdate1 = format_date_in_iso( $params->{'filterdate1'} ); >- $filterdate2 = format_date_in_iso( $params->{'filterdate2'} ); >- $borrower_dateexpiry = format_date_in_iso( $params->{'borrower_dateexpiry'} ); >- $borrower_categorycode = $params->{'borrower_categorycode'}; >+if ( $step == 2 ) { > > my %checkboxes = map { $_ => 1 } split /\0/, $params->{'checkbox'}; > > my $totalDel; > my $membersToDelete; > if ( $checkboxes{borrower} ) { >- $membersToDelete = >- GetBorrowersToExpunge( { not_borrowered_since => $filterdate1, expired_before => $borrower_dateexpiry, category_code => $borrower_categorycode } ); >+ $membersToDelete = GetBorrowersToExpunge( >+ { >+ ( >+ $not_borrowered_since >+ ? ( >+ not_borrowered_since => output_pref( >+ { >+ dt => $not_borrowered_since, >+ dateformat => 'iso', >+ dateonly => 1 >+ } >+ ) >+ ) >+ : () >+ ), >+ ( >+ $borrower_dateexpiry >+ ? ( >+ expired_before => output_pref( >+ { >+ dt => $borrower_dateexpiry, >+ dateformat => 'iso', >+ dateonly => 1 >+ } >+ ) >+ ) >+ : () >+ ), >+ ( >+ $borrower_categorycode >+ ? ( category_code => $borrower_categorycode ) >+ : () >+ ), >+ } >+ ); > _skip_borrowers_with_nonzero_balance( $membersToDelete ); > $totalDel = scalar @$membersToDelete; > >@@ -86,33 +126,19 @@ if ( $params->{'step2'} ) { > my $totalAno; > my $membersToAnonymize; > if ( $checkboxes{issue} ) { >- $membersToAnonymize = GetBorrowersWithIssuesHistoryOlderThan($filterdate2); >+ $membersToAnonymize = GetBorrowersWithIssuesHistoryOlderThan($last_issue_date); > $totalAno = scalar @$membersToAnonymize; > } > > $template->param( >- step2 => 1, > totalToDelete => $totalDel, > totalToAnonymize => $totalAno, > memberstodelete_list => $membersToDelete, > memberstoanonymize_list => $membersToAnonymize, >- filterdate1 => format_date($filterdate1), >- filterdate2 => format_date($filterdate2), >- borrower_dateexpiry => format_date($borrower_dateexpiry), >- borrower_categorycode => $borrower_categorycode, > ); >- >- ### TODO : Use GetBorrowersNamesAndLatestIssue function in order to get the borrowers to delete or anonymize. >- output_html_with_http_headers $cgi, $cookie, $template->output; >- exit; > } > >-if ( $params->{'step3'} ) { >- $filterdate1 = format_date_in_iso( $params->{'filterdate1'} ); >- $filterdate2 = format_date_in_iso( $params->{'filterdate2'} ); >- $borrower_dateexpiry = format_date_in_iso( $params->{'borrower_dateexpiry'} ); >- $borrower_categorycode = $params->{'borrower_categorycode'}; >- >+elsif ( $step == 3 ) { > my $do_delete = $params->{'do_delete'}; > my $do_anonym = $params->{'do_anonym'}; > >@@ -120,8 +146,36 @@ if ( $params->{'step3'} ) { > > # delete members > if ($do_delete) { >- my $membersToDelete = >- GetBorrowersToExpunge( { not_borrowered_since => $filterdate1, expired_before => $borrower_dateexpiry, category_code => $borrower_categorycode } ); >+ my $membersToDelete = GetBorrowersToExpunge( >+ { >+ ( >+ $not_borrowered_since >+ ? ( >+ not_borrowered_since => output_pref( >+ { >+ dt => $not_borrowered_since, >+ dateformat => 'iso' >+ } >+ ) >+ ) >+ : () >+ ), >+ ( >+ $borrower_dateexpiry >+ ? ( >+ expired_before => output_pref( >+ { dt => $borrower_dateexpiry, dateformat => 'iso' } >+ ) >+ ) >+ : () >+ ), >+ ( >+ $borrower_categorycode >+ ? ( category_code => $borrower_categorycode ) >+ : () >+ ), >+ } >+ ); > _skip_borrowers_with_nonzero_balance( $membersToDelete ); > $totalDel = scalar(@$membersToDelete); > $radio = $params->{'radio'}; >@@ -141,29 +195,25 @@ if ( $params->{'step3'} ) { > # Anonymising all members > if ($do_anonym) { > #FIXME: anonymisation errors are not handled >- ($totalAno,my $anonymisation_error) = AnonymiseIssueHistory($filterdate2); >+ ($totalAno,my $anonymisation_error) = AnonymiseIssueHistory($last_issue_date); > $template->param( >- filterdate1 => $filterdate2, > do_anonym => '1', > ); > } > > $template->param( >- step3 => '1', > trash => ( $radio eq "trash" ) ? (1) : (0), > testrun => ( $radio eq "testrun" ) ? 1: 0, > ); >- >- #writing the template >- output_html_with_http_headers $cgi, $cookie, $template->output; >- exit; > } > > $template->param( >- step1 => '1', >- filterdate1 => $filterdate1, >- filterdate2 => $filterdate2, >- borrower_categorycodes => GetBorrowercategoryList(), >+ step => $step, >+ not_borrowered_since => $not_borrowered_since, >+ borrower_dateexpiry => $borrower_dateexpiry, >+ last_issue_date => $last_issue_date, >+ borrower_categorycodes => GetBorrowercategoryList(), >+ borrower_categorycode => $borrower_categorycode, > ); > > #writing the template >-- >1.7.2.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 11975
:
26489
|
28104
|
28105
| 28129 |
28130