From 64fee166898c58c4e51f96120b9bade09eb71fd2 Mon Sep 17 00:00:00 2001 From: Stefan Berndtsson Date: Fri, 17 Feb 2017 15:50:45 +0100 Subject: [PATCH] Add borrowernumber to staged imports, and set it to the currently logged in user. SysPref to turn on filtering of staged import list so that only batches staged by logged in user is visible. SysPref to remove Cleaned/Imported batches from staged import list. Link near list to ignore filter, in case one needs to see all batches. https://bugs.koha-community.org/show_bug.cgi?id=18129 --- .gitignore | 3 ++ C4/ImportBatch.pm | 32 ++++++++++-- .../bug_18129-stage-with-username-syspref.sql | 2 + .../atomicupdate/bug_18129-stage-with-username.sql | 1 + .../en/modules/admin/preferences/cataloguing.pref | 12 +++++ .../prog/en/modules/tools/manage-marc-import.tt | 20 +++++++ t/ImportBatch.t | 61 +++++++++++++++++++++- tools/manage-marc-import.pl | 27 ++++++++-- 8 files changed, 148 insertions(+), 10 deletions(-) create mode 100644 .gitignore create mode 100644 installer/data/mysql/atomicupdate/bug_18129-stage-with-username-syspref.sql create mode 100644 installer/data/mysql/atomicupdate/bug_18129-stage-with-username.sql diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..e48f4f0 --- /dev/null +++ b/.gitignore @@ -0,0 +1,3 @@ +*~ +\#*# +.#* diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm index 88d670d..c41c673 100644 --- a/C4/ImportBatch.pm +++ b/C4/ImportBatch.pm @@ -237,9 +237,17 @@ sub GetImportRecordMarcXML { sub AddImportBatch { my ($params) = @_; + # Get ID of logged in user. if called from a batch job, + # no user session exists and C4::Context->userenv() returns + # the scalar '0'. + my $userenv = C4::Context->userenv(); + my $usernumber = (ref($userenv) eq 'HASH') ? $userenv->{'number'} : 0; + $usernumber ||= 0; + $params->{borrowernumber} = $usernumber; + my (@fields, @vals); foreach (qw( matcher_id template_id branchcode - overlay_action nomatch_action item_action + overlay_action nomatch_action item_action borrowernumber import_status batch_type file_name comments record_type )) { if (exists $params->{$_}) { push @fields, $_; @@ -1028,13 +1036,29 @@ start at the given offset. =cut sub GetImportBatchRangeDesc { - my ($offset, $results_per_group) = @_; + my ($offset, $results_per_group, $no_filter) = @_; my $dbh = C4::Context->dbh; my $query = "SELECT * FROM import_batches - WHERE batch_type IN ('batch', 'webservice') - ORDER BY import_batch_id DESC"; + WHERE batch_type IN ('batch', 'webservice')"; + my @params; + + if(C4::Context->preference("StageFilterByUser") && !$no_filter) { + my $userenv = C4::Context->userenv(); + my $usernumber = (ref($userenv) eq 'HASH') ? $userenv->{'number'} : 0; + $usernumber ||= 0; + + $query .= " AND borrowernumber = ?"; + push(@params, $usernumber); + } + + if(C4::Context->preference("StageHideCleanedImported") && !$no_filter) { + $query .= " AND import_status NOT IN ('cleaned', 'imported')"; + } + + $query .= " ORDER BY import_batch_id DESC"; + if ($results_per_group){ $query .= " LIMIT ?"; push(@params, $results_per_group); diff --git a/installer/data/mysql/atomicupdate/bug_18129-stage-with-username-syspref.sql b/installer/data/mysql/atomicupdate/bug_18129-stage-with-username-syspref.sql new file mode 100644 index 0000000..ce516aa --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_18129-stage-with-username-syspref.sql @@ -0,0 +1,2 @@ +INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type`) VALUES ('StageFilterByUser', '0', '', 'Filter staged batches by user', 'YesNo'); +INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type`) VALUES ('StageHideCleanedImported', '0', '', 'Hide staged batches when cleaned or imported', 'YesNo'); diff --git a/installer/data/mysql/atomicupdate/bug_18129-stage-with-username.sql b/installer/data/mysql/atomicupdate/bug_18129-stage-with-username.sql new file mode 100644 index 0000000..02b125f --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_18129-stage-with-username.sql @@ -0,0 +1 @@ +ALTER TABLE `import_batches` ADD `borrowernumber` int(11); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref index 1aeadf9..8fcfe88 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref @@ -231,6 +231,18 @@ Cataloging: yes: "do" no: "don't" - attempt to match aggressively by trying all variations of the ISSNs in the imported record as a phrase in the ISSN fields of already cataloged records. Note that this preference has no effect if UseQueryParser is on. + - + - In list of staged batches of MARC records, + - pref: StageFilterByUser + choices: + yes: "show only batches uploaded by logged in user." + no: "show all batches regardless of user." + - + - pref: StageHideCleanedImported + choices: + yes: "Hide" + no: "Do not hide" + - cleaned and imported batches of MARC records by default. Exporting: - diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt index 53ad39f..d7accd6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt @@ -1,3 +1,4 @@ +[% USE Koha %] [% BLOCK final_match_link %] [% IF ( record.record_type == 'biblio' ) %] [% record.final_match_id %] @@ -212,6 +213,13 @@ $(document).ready(function(){

No records have been staged.

Stage MARC records for import.

+ [% IF Koha.Preference( 'StageFilterByUser' ) %] +

+ [% UNLESS ( no_filter ) %] + Show unfiltered list. + [% END %] +

+ [% END %]
[% END %] [% END %] @@ -247,6 +255,7 @@ $(document).ready(function(){
  1. File name: [% file_name %]
  2. +
  3. User: [% user_name %]
  4. Comments: [% IF ( comments ) %][% comments %][% ELSE %](none)[% END %]
  5. Type: [% IF ( record_type == 'auth' ) %]Authority records[% ELSE %]Bibliographic records[% END %]
  6. Staged: [% upload_timestamp %]
  7. @@ -411,6 +420,15 @@ $(document).ready(function(){
    [% IF ( batch_list ) %] + [% IF Koha.Preference( 'StageFilterByUser' ) %] +

    + [% IF ( no_filter ) %] + Filter by logged in user + [% ELSE %] + Do not filter list + [% END %] +

    + [% END %] [% IF ( pages ) %]
    Page @@ -427,6 +445,7 @@ Page # File name + User Comments Type Status @@ -439,6 +458,7 @@ Page [% batch_lis.import_batch_id %] [% batch_lis.file_name %] + [% batch_lis.user_name %] [% batch_lis.comments %] [% IF ( batch_lis.record_type == 'auth' ) %]Authority[% ELSE %]Bibliographic[% END %] diff --git a/t/ImportBatch.t b/t/ImportBatch.t index 9db2c27..d7869a0 100644 --- a/t/ImportBatch.t +++ b/t/ImportBatch.t @@ -21,7 +21,7 @@ use File::Temp qw|tempfile|; use MARC::Field; use MARC::File::XML; use MARC::Record; -use Test::More tests => 3; +use Test::More tests => 4; use t::lib::Mocks; BEGIN { @@ -30,6 +30,54 @@ BEGIN { t::lib::Mocks::mock_preference('marcflavour', 'MARC21'); +subtest 'StageFilterByUser' => sub { + plan tests => 5; + + # Set current user to "user1" + C4::Context->_new_userenv('USER1-ENV'); + C4::Context->set_userenv('12345', 'user1', '12345', '', '', '', '', 1, '', ''); + C4::Context->set_preference( 'StageFilterByUser', '1'); + C4::Context->set_preference( 'StageHideCleanedImported', '1'); + + my $file = create_file({ two => 1, format => 'marc'}); + my ($errors, $recs) = C4::ImportBatch::RecordsFromISO2709File($file, 'biblio', 'UTF-8'); + + my ($batch_id) = C4::ImportBatch::BatchStageMarcRecords('biblio', 'UTF-8', $recs, + 'file1.mrc', undef, undef, '', + '', 1, 0, 0, undef); + + my $batch_list_user1 = C4::ImportBatch::GetImportBatchRangeDesc(0, 100000); + is( find_id_in_batch_list($batch_id, $batch_list_user1), 1, "Found one batch from user1"); + + # Set current user to "user2" + C4::Context->_new_userenv('USER2-ENV'); + C4::Context->set_userenv('23456', 'user2', '23456', '', '', '', '', 1, '', ''); + + my $batch_list_user2 = C4::ImportBatch::GetImportBatchRangeDesc(0, 100000); + is( find_id_in_batch_list($batch_id, $batch_list_user2), 0, "Did not see batches from user1 when logged in as user2"); + + $batch_list_user2 = C4::ImportBatch::GetImportBatchRangeDesc(0, 100000, 1); + is( find_id_in_batch_list($batch_id, $batch_list_user2), 1, "Can see batch from user1 as user2 when filter is disabled"); + + # Set current user back to "user1" + C4::Context->_new_userenv('USER1-ENV'); + C4::Context->set_userenv('12345', 'user1', '12345', '', '', '', '', 1, '', ''); + + C4::Context->set_preference( 'StageHideCleanedImported', '1'); + C4::ImportBatch::SetImportBatchStatus($batch_id, 'imported'); + + $batch_list_user1 = C4::ImportBatch::GetImportBatchRangeDesc(0, 100000); + is( find_id_in_batch_list($batch_id, $batch_list_user1), 0, "Did not see imported batch from user1 hen hidden"); + + C4::Context->set_preference( 'StageHideCleanedImported', '0'); + C4::ImportBatch::SetImportBatchStatus($batch_id, 'imported'); + + $batch_list_user1 = C4::ImportBatch::GetImportBatchRangeDesc(0, 100000); + is( find_id_in_batch_list($batch_id, $batch_list_user1), 1, "Did see imported batch from user1 when not hidden"); + + C4::ImportBatch::DeleteBatch($batch_id); +}; + subtest 'RecordsFromISO2709File' => sub { plan tests => 4; @@ -99,3 +147,14 @@ sub create_file { close $fh; return $name; } + +sub find_id_in_batch_list { + my ($batch_id, $batch_list) = @_; + my $found = 0; + foreach my $batch (@$batch_list) { + if($batch->{import_batch_id} == $batch_id) { + $found = 1; + } + } + return $found; +} diff --git a/tools/manage-marc-import.pl b/tools/manage-marc-import.pl index 249cabe..4822224 100755 --- a/tools/manage-marc-import.pl +++ b/tools/manage-marc-import.pl @@ -32,6 +32,7 @@ use C4::Auth; use C4::AuthoritiesMarc; use C4::Output; use C4::Biblio; +use C4::Members; use C4::ImportBatch; use C4::Matcher; use C4::BackgroundJob; @@ -42,6 +43,7 @@ my $script_name = "/cgi-bin/koha/tools/manage-marc-import.pl"; my $input = new CGI; my $op = $input->param('op') || ''; +my $no_filter = $input->param('no_filter') || 0; my $completedJobID = $input->param('completedJobID'); our $runinbackground = $input->param('runinbackground'); my $import_batch_id = $input->param('import_batch_id') || ''; @@ -91,7 +93,7 @@ if ($op) { if ($op eq "") { # displaying a list if ($import_batch_id eq '') { - import_batches_list($template, $offset, $results_per_page); + import_batches_list($template, $offset, $results_per_page, $no_filter); } else { import_records_list($template, $import_batch_id, $offset, $results_per_page); } @@ -112,14 +114,14 @@ if ($op eq "") { import_records_list($template, $import_batch_id, $offset, $results_per_page); } elsif ($op eq "clean-batch") { CleanBatch($import_batch_id); - import_batches_list($template, $offset, $results_per_page); + import_batches_list($template, $offset, $results_per_page, $no_filter); $template->param( did_clean => 1, import_batch_id => $import_batch_id, ); } elsif ($op eq "delete-batch") { DeleteBatch($import_batch_id); - import_batches_list($template, $offset, $results_per_page); + import_batches_list($template, $offset, $results_per_page, $no_filter); $template->param( did_delete => 1, ); @@ -203,11 +205,17 @@ sub create_labelbatch_from_importbatch { } sub import_batches_list { - my ($template, $offset, $results_per_page) = @_; - my $batches = GetImportBatchRangeDesc($offset, $results_per_page); + my ($template, $offset, $results_per_page, $no_filter) = @_; + my $batches = GetImportBatchRangeDesc($offset, $results_per_page, $no_filter); my @list = (); foreach my $batch (@$batches) { + my $borrower = GetMember(borrowernumber => $batch->{'borrowernumber'}); + my $user_name = ""; + if($borrower) { + $user_name = $borrower->{'userid'}; + } + push @list, { import_batch_id => $batch->{'import_batch_id'}, num_records => $batch->{'num_records'}, @@ -218,6 +226,7 @@ sub import_batches_list { comments => $batch->{'comments'}, can_clean => ($batch->{'import_status'} ne 'cleaned') ? 1 : 0, record_type => $batch->{'record_type'}, + user_name => $user_name, }; } $template->param(batch_list => \@list); @@ -227,6 +236,7 @@ sub import_batches_list { $template->param(range_top => $offset + $results_per_page - 1); $template->param(num_results => $num_batches); $template->param(results_per_page => $results_per_page); + $template->param(no_filter => $no_filter); } @@ -354,6 +364,13 @@ sub import_records_list { my $batch = GetImportBatch($import_batch_id); $template->param(import_batch_id => $import_batch_id); + my $borrower = GetMember(borrowernumber => $batch->{'borrowernumber'}); + my $user_name = ""; + if($borrower) { + $user_name = $borrower->{'userid'}; + } + $template->param("user_name" => $user_name); + my $overlay_action = GetImportBatchOverlayAction($import_batch_id); $template->param("overlay_action_${overlay_action}" => 1); $template->param(overlay_action => $overlay_action); -- 2.7.4