Bugzilla – Attachment 126623 Details for
Bug 16446
Allow librarians to add borrowers to patron lists by borrowernumber
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 16446: Add ability to add patrons to list by borrowernumber
Bug-16446-Add-ability-to-add-patrons-to-list-by-bo.patch (text/plain), 10.00 KB, created by
Katrin Fischer
on 2021-10-20 21:29:54 UTC
(
hide
)
Description:
Bug 16446: Add ability to add patrons to list by borrowernumber
Filename:
MIME Type:
Creator:
Katrin Fischer
Created:
2021-10-20 21:29:54 UTC
Size:
10.00 KB
patch
obsolete
>From e4fa2b2ea79d19cee6c786e264bb29bd5a1c45bb Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Fri, 27 Aug 2021 12:39:49 +0000 >Subject: [PATCH] Bug 16446: Add ability to add patrons to list by > borrowernumber > >AddPatronsToList already took a borrowernumber parameter, however, it >did not check if those were valid numbers. This patch expands the search >to apply to cardnumber or borrowernumbers in the subroutine. > >Template and script are adjusted to allow choosing borrowernumbers or cardnumbers > >To test: >1 - Apply patch >2 - Browse to Tools->Patron lists >3 - Create a list, or choose Actions->Add patrons for an existing list >4 - Click 'Add multiple patrons' >5 - Cardnumbers is preselected >6 - Enter a list of cardnumbers, ensure you test: > Cardnumber already in list > Cardnumber not in list > Non-existent cardnumber >7 - Patrons should be added/errors reported correctly >8 - Repeat with borowernumbers > >Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> > >Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> >--- > Koha/List/Patron.pm | 18 +++++---- > .../prog/en/modules/patron_lists/list.tt | 47 ++++++++++++++-------- > patron_lists/list.pl | 25 +++++++----- > t/db_dependent/PatronLists.t | 9 ++++- > 4 files changed, 63 insertions(+), 36 deletions(-) > >diff --git a/Koha/List/Patron.pm b/Koha/List/Patron.pm >index 7ff95a13e0..cb91275d85 100644 >--- a/Koha/List/Patron.pm >+++ b/Koha/List/Patron.pm >@@ -173,17 +173,19 @@ sub AddPatronsToList { > > my @borrowernumbers; > >+ my %search_param; > if ($cardnumbers) { >- @borrowernumbers = >- Koha::Database->new()->schema()->resultset('Borrower')->search( >- { cardnumber => { 'IN' => $cardnumbers } }, >- { columns => [qw/ borrowernumber /] } >- )->get_column('borrowernumber')->all(); >- } >- else { >- @borrowernumbers = @$borrowernumbers; >+ $search_param{cardnumber} = { 'IN' => $cardnumbers }; >+ } else { >+ $search_param{borrowernumber} = { 'IN' => $borrowernumbers }; > } > >+ @borrowernumbers = >+ Koha::Database->new()->schema()->resultset('Borrower')->search( >+ \%search_param, >+ { columns => [qw/ borrowernumber /] } >+ )->get_column('borrowernumber')->all(); >+ > my $patron_list_id = $list->patron_list_id(); > > my $plp_rs = Koha::Database->new()->schema()->resultset('PatronListPatron'); >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt >index a9f74cf211..d06a9726ff 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt >@@ -12,10 +12,10 @@ > #add_patrons_by_search { > display: none; > } >- #patron_barcodes_line { >+ #patron_ids_line { > display: none; > } >- #patron_barcodes_submit { >+ #patron_ids_submit { > display: none; > } > #searchheader { >@@ -58,10 +58,10 @@ > <h1>[% list.name | html %]</h1> > > [% IF ( not_found.size > 0 ) %] >- <div class="dialog alert"><p>Warning, the following cardnumbers were not found:</p></div> >+ <div class="dialog alert"><p>Warning, the following [% id_column | html %] were not found:</p></div> > <table style="margin:auto;"> > <thead> >- <tr><th>Cardnumbers not found</th></tr> >+ <tr><th>[% id_column | html FILTER ucfirst %] not found</th></tr> > </thead> > <tbody> > [% FOREACH nf IN not_found %] >@@ -72,10 +72,10 @@ > [% END %] > > [% IF ( existed.size > 0 ) %] >- <div class="dialog alert"><p>Warning, the following cardnumbers were already in this list:</p></div> >+ <div class="dialog alert"><p>Warning, the following [% id_column | html %] were already in this list:</p></div> > <table style="margin:auto;"> > <thead> >- <tr><th>Cardnumbers already in list</th></tr> >+ <tr><th>[% id_column | html FILTER ucfirst %] already in list</th></tr> > </thead> > <tbody> > [% FOREACH ed IN existed %] >@@ -96,16 +96,29 @@ > <li id="add_patrons_by_search"><a href="#"> > <span class="label"> </span> > <i class="fa fa-plus"></i> Search for patrons</a></li> >- <li id="add_patrons_by_barcode"><a href="#"> >+ <li id="add_patrons_by_id"><a href="#"> > <span class="label"> </span> >- <i class="fa fa-plus"></i> Enter multiple card numbers</a></li> >- <li id="patron_barcodes_line"> >- <label for="patrons_by_barcode">Card number list (one barcode per line):</label> >- <textarea id="patrons_by_barcode" name="patrons_by_barcode" id="" cols="30" rows="10"></textarea> >+ <i class="fa fa-plus"></i> Add multiple patrons</a></li> >+ <li id="patron_ids_line"> >+ <fieldset class="rows"> >+ <legend>Choose type of ID to enter</legend> >+ <span class="radio"> >+ <label for "cardnumbers"> >+ <input id="add_by_cardnumbers" type="radio" name="id_column" value="cardnumbers" checked="checked"> >+ <span class="add_by_cardnumbers">Cardnumbers</span> >+ </label> >+ <label for "borrowernumbers"> >+ <input id="add_by_borrwernumbers" type="radio" name="id_column" value="borrowernumbers"> >+ <span class="add_by_borrowernumbers">Borrowernumbers</span> >+ </label> >+ </span> >+ </fieldset> >+ <label for="patrons_by_id">List (one barcode per line):</label> >+ <textarea id="patrons_by_id" name="patrons_by_id" id="" cols="30" rows="10"></textarea> > </li> > </ol> > </fieldset> >- <fieldset id="patron_barcodes_submit" class="action"> >+ <fieldset id="patron_ids_submit" class="action"> > <input type="submit" value="Submit" /> > </fieldset> > >@@ -247,14 +260,14 @@ > } > }); > >- $("#add_patrons_by_barcode a").on("click", function(){ >- $("#add_patrons_by_barcode, #patron_search_line").hide(); >- $("#add_patrons_by_search, #patron_barcodes_line, #patron_barcodes_submit").show(); >+ $("#add_patrons_by_id a").on("click", function(){ >+ $("#add_patrons_by_id, #patron_search_line").hide(); >+ $("#add_patrons_by_search, #patron_ids_line, #patron_ids_submit").show(); > }); > > $("#add_patrons_by_search a").on("click", function(){ >- $("#add_patrons_by_barcode, #patron_search_line").show(); >- $("#add_patrons_by_search, #patron_barcodes_line, #patron_barcodes_submit").hide(); >+ $("#add_patrons_by_id, #patron_search_line").show(); >+ $("#add_patrons_by_search, #patron_ids_line, #patron_ids_submit").hide(); > }); > > $('.merge-patrons').on('click', function() { >diff --git a/patron_lists/list.pl b/patron_lists/list.pl >index 7076b20c9f..45b11a1667 100755 >--- a/patron_lists/list.pl >+++ b/patron_lists/list.pl >@@ -46,22 +46,27 @@ my ($list) = > > my @existing = $list->patron_list_patrons; > >-my $cardnumbers = $cgi->param('patrons_by_barcode'); >-my @patrons_by_barcode; >+my $patrons_by_id = $cgi->param('patrons_by_id'); >+my $id_column = $cgi->param('id_column'); > >-if ( $cardnumbers ){ >- push my @patrons_by_barcode, uniq( split(/\s\n/, $cardnumbers) ); >- my @results = AddPatronsToList( { list => $list, cardnumbers => \@patrons_by_barcode } ); >- my %found = map { $_->borrowernumber->cardnumber => 1 } @results; >- my %exist = map { $_->borrowernumber->cardnumber => 1 } @existing; >+if ( $patrons_by_id ){ >+ push my @patrons_list, uniq( split(/\s\n/, $patrons_by_id) ); >+ my %add_params; >+ $add_params{list} = $list; >+ $add_params{ $id_column } = \@patrons_list; >+ my @results = AddPatronsToList(\%add_params); >+ my $id = $id_column eq 'borrowernumbers' ? 'borrowernumber' : 'cardnumber'; >+ my %found = map { $_->borrowernumber->$id => 1 } @results; >+ my %exist = map { $_->borrowernumber->$id => 1 } @existing; > my (@not_found, @existed); >- foreach my $barcode ( @patrons_by_barcode ){ >- push (@not_found, $barcode) unless defined $found{$barcode}; >- push (@existed, $barcode) if defined $exist{$barcode}; >+ foreach my $patron ( @patrons_list ){ >+ push (@not_found, $patron) unless defined $found{$patron}; >+ push (@existed, $patron) if defined $exist{$patron}; > } > $template->param( > not_found => \@not_found, > existed => \@existed, >+ id_column => $id_column, > ); > } > >diff --git a/t/db_dependent/PatronLists.t b/t/db_dependent/PatronLists.t >index 6e4f5f8efa..82bab67c85 100755 >--- a/t/db_dependent/PatronLists.t >+++ b/t/db_dependent/PatronLists.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 9; >+use Test::More tests => 11; > use t::lib::TestBuilder; > use t::lib::Mocks; > >@@ -80,6 +80,13 @@ is( > 'AddPatronsToList works for borrowernumbers' > ); > >+my $deleted_patron = $builder->build_object({ class => 'Koha::Patrons' }); >+$deleted_patron->delete; >+my @result = AddPatronsToList({list => $list2,borrowernumbers => [ $deleted_patron->borrowernumber ]}); >+is( scalar @result, 0,'Invalid borrowernumber not added'); >+@result = AddPatronsToList({list => $list2,cardnumbers => [ $deleted_patron->cardnumber ]}); >+is( scalar @result, 0,'Invalid cardnumber not added'); >+ > my @ids = > $list1->patron_list_patrons()->get_column('patron_list_patron_id')->all(); > DelPatronsFromList( >-- >2.11.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=126623">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 16446
:
124177
|
124182
|
124242
| 126623 |
126624