@@ -, +, @@ borrowernumber Cardnumber already in list Cardnumber not in list Non-existent cardnumber --- 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(-) --- a/Koha/List/Patron.pm +++ a/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'); --- a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt +++ a/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 @@

[% list.name | html %]

[% IF ( not_found.size > 0 ) %] -

Warning, the following cardnumbers were not found:

+

Warning, the following [% id_column | html %] were not found:

- + [% FOREACH nf IN not_found %] @@ -72,10 +72,10 @@ [% END %] [% IF ( existed.size > 0 ) %] -

Warning, the following cardnumbers were already in this list:

+

Warning, the following [% id_column | html %] were already in this list:

Cardnumbers not found
[% id_column | html FILTER ucfirst %] not found
- + [% FOREACH ed IN existed %] @@ -96,16 +96,29 @@ -
  • +
  •   - Enter multiple card numbers
  • -
  • - - + Add multiple patrons
  • +
  • +
    + Choose type of ID to enter + + + + +
    + +
  • -
    +
    @@ -249,14 +262,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() { --- a/patron_lists/list.pl +++ a/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, ); } --- a/t/db_dependent/PatronLists.t +++ a/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( --
    Cardnumbers already in list
    [% id_column | html FILTER ucfirst %] already in list