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> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
parent
e5d5623ee2
commit
ad1d734e54
4 changed files with 63 additions and 36 deletions
|
@ -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');
|
||||
|
|
|
@ -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() {
|
||||
|
|
|
@ -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,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
@ -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(
|
||||
|
|
Loading…
Reference in a new issue