From 5f92411d938ce475d5253f14ff6f1e9d15fc2aa3 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Mon, 31 Aug 2020 17:01:24 +0000 Subject: [PATCH] Bug 22785: Allow option to choose which record match is applied during import This patchset adds the display of all matches found during import to the import management screen A staff member with the permission to manage batches will be able to select for any individual record which match, or none, should be used during import To test: 1 - Import a batch of records or export existing records from your catalog 2 - Import the file (again) and select a matching rule that will find matches 3 - Note that you now have radio buttons allowing you to select a record, or none 4 - Test scenarios: I - When 'Action if matching record found' is 'Ignore' a - Imported record ignored if match is selected b - 'Action if no match found' followed if no match is selected (Ignore matches) II - When 'Action if matching record found' is 'Replace' a - The chosen record is the one overlayed (you can edit the chosen record before importing to confirm) b - 'Action if no match found' followed if no match is selected (Ignore matches) III - When 'Action if matching record found' is 'Add incoming record' a - Record is added regardless of matches 5 - Confirm 'Diff' 'View' links work as expected 6 - Confirm that after records are imported the radio buttons to choose are disabled --- C4/ImportBatch.pm | 58 +++---- Koha/REST/V1/ImportRecordMatches.pm | 106 +++++++++++++ api/v1/swagger/definitions.json | 3 + .../swagger/definitions/import_record_match.json | 21 +++ api/v1/swagger/parameters.json | 3 + api/v1/swagger/parameters/import_record_match.json | 16 ++ api/v1/swagger/paths.json | 3 + api/v1/swagger/paths/import_record_matches.json | 144 ++++++++++++++++++ api/v1/swagger/x-primitives.json | 8 + installer/data/mysql/atomicupdate/bug_22785.perl | 8 + installer/data/mysql/kohastructure.sql | 1 + .../prog/en/modules/tools/manage-marc-import.tt | 74 ++++++--- reports/acquisitions_stats.pl | 1 + t/db_dependent/ImportBatch.t | 31 +++- t/db_dependent/api/v1/import_record_matches.t | 167 +++++++++++++++++++++ tools/batch_records_ajax.pl | 35 ++--- 16 files changed, 610 insertions(+), 69 deletions(-) create mode 100644 Koha/REST/V1/ImportRecordMatches.pm create mode 100644 api/v1/swagger/definitions/import_record_match.json create mode 100644 api/v1/swagger/parameters/import_record_match.json create mode 100644 api/v1/swagger/paths/import_record_matches.json create mode 100644 installer/data/mysql/atomicupdate/bug_22785.perl create mode 100755 t/db_dependent/api/v1/import_record_matches.t diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm index 7064f94728..210a802b17 100644 --- a/C4/ImportBatch.pm +++ b/C4/ImportBatch.pm @@ -689,7 +689,6 @@ sub BatchCommitRecords { } elsif ($record_result eq 'ignore') { $recordid = $record_match; $num_ignored++; - $recordid = $record_match; if ($record_type eq 'biblio' and defined $recordid and ( $item_result eq 'create_new' || $item_result eq 'replace' ) ) { my ($bib_items_added, $bib_items_replaced, $bib_items_errored) = BatchCommitItems($rowref->{'import_record_id'}, $recordid, $item_result); $num_items_added += $bib_items_added; @@ -1178,6 +1177,7 @@ sub GetBestRecordMatch { WHERE import_record_matches.import_record_id = ? AND ( (import_records.record_type = 'biblio' AND biblio.biblionumber IS NOT NULL) OR (import_records.record_type = 'auth' AND auth_header.authid IS NOT NULL) ) + AND chosen = 1 ORDER BY score DESC, candidate_match_id DESC"); $sth->execute($import_record_id); my ($record_id) = $sth->fetchrow_array(); @@ -1442,12 +1442,13 @@ sub GetImportRecordMatches { my $dbh = C4::Context->dbh; # FIXME currently biblio only my $sth = $dbh->prepare_cached("SELECT title, author, biblionumber, - candidate_match_id, score, record_type + candidate_match_id, score, record_type, + chosen FROM import_records JOIN import_record_matches USING (import_record_id) LEFT JOIN biblio ON (biblionumber = candidate_match_id) WHERE import_record_id = ? - ORDER BY score DESC, biblionumber DESC"); + ORDER BY score DESC, chosen DESC, biblionumber DESC"); $sth->bind_param(1, $import_record_id); my $results = []; $sth->execute(); @@ -1480,10 +1481,12 @@ sub SetImportRecordMatches { $delsth->execute($import_record_id); $delsth->finish(); - my $sth = $dbh->prepare("INSERT INTO import_record_matches (import_record_id, candidate_match_id, score) - VALUES (?, ?, ?)"); + my $sth = $dbh->prepare("INSERT INTO import_record_matches (import_record_id, candidate_match_id, score, chosen) + VALUES (?, ?, ?, ?)"); + my $chosen = 1; #The first match is defaulted to be chosen foreach my $match (@matches) { - $sth->execute($import_record_id, $match->{'record_id'}, $match->{'score'}); + $sth->execute($import_record_id, $match->{'record_id'}, $match->{'score'}, $chosen); + $chosen = 0; #After the first we do not default to other matches } } @@ -1700,41 +1703,30 @@ sub _get_commit_action { if ($record_type eq 'biblio') { my ($bib_result, $bib_match, $item_result); - if ($overlay_status ne 'no_match') { - $bib_match = GetBestRecordMatch($import_record_id); - if ($overlay_action eq 'replace') { - $bib_result = defined($bib_match) ? 'replace' : 'create_new'; - } elsif ($overlay_action eq 'create_new') { - $bib_result = 'create_new'; - } elsif ($overlay_action eq 'ignore') { - $bib_result = 'ignore'; - } - if($item_action eq 'always_add' or $item_action eq 'add_only_for_matches'){ + $bib_match = GetBestRecordMatch($import_record_id); + if ($overlay_status ne 'no_match' && defined($bib_match)) { + + $bib_result = $overlay_action; + + if($item_action eq 'always_add' or $item_action eq 'add_only_for_matches'){ $item_result = 'create_new'; - } - elsif($item_action eq 'replace'){ - $item_result = 'replace'; - } - else { - $item_result = 'ignore'; - } + } elsif($item_action eq 'replace'){ + $item_result = 'replace'; + } else { + $item_result = 'ignore'; + } + } else { $bib_result = $nomatch_action; - $item_result = ($item_action eq 'always_add' or $item_action eq 'add_only_for_new') ? 'create_new' : 'ignore'; + $item_result = ($item_action eq 'always_add' or $item_action eq 'add_only_for_new') ? 'create_new' : 'ignore'; } return ($bib_result, $item_result, $bib_match); } else { # must be auths my ($auth_result, $auth_match); - if ($overlay_status ne 'no_match') { - $auth_match = GetBestRecordMatch($import_record_id); - if ($overlay_action eq 'replace') { - $auth_result = defined($auth_match) ? 'replace' : 'create_new'; - } elsif ($overlay_action eq 'create_new') { - $auth_result = 'create_new'; - } elsif ($overlay_action eq 'ignore') { - $auth_result = 'ignore'; - } + $auth_match = GetBestRecordMatch($import_record_id); + if ($overlay_status ne 'no_match' && defined($auth_match)) { + $auth_result = $overlay_action; } else { $auth_result = $nomatch_action; } diff --git a/Koha/REST/V1/ImportRecordMatches.pm b/Koha/REST/V1/ImportRecordMatches.pm new file mode 100644 index 0000000000..cac579b53a --- /dev/null +++ b/Koha/REST/V1/ImportRecordMatches.pm @@ -0,0 +1,106 @@ +package Koha::REST::V1::ImportRecordMatches; + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Mojo::Base 'Mojolicious::Controller'; + +use Koha::Import::Record::Matches; + +use Try::Tiny; + +=head1 API + +=head2 Methods + +=cut + +=head3 unset_chosen + +Method that handles unselecting all chosen matches for an import record + +DELETE /api/v1/import/{import_batch_id}/records/{import_record_id}/matches/chosen + +=cut + +sub unset_chosen { + my $c = shift->openapi->valid_input or return; + + my $import_record_id = $c->validation->param('import_record_id'); + my $matches = Koha::Import::Record::Matches->search({ + import_record_id => $import_record_id, + }); + unless ($matches) { + return $c->render( + status => 404, + openapi => { error => "No matches not found" } + ); + } + return try { + $matches->unset_chosen; + return $c->render( status => 204, openapi => $matches ); + } + catch { + $c->unhandled_exception($_); + }; +} + +=head3 set_chosen + +Method that handles modifying if a Koha::Import::Record::Match object has been chosen for overlay + +PUT /api/v1/import/{import_batch_id}/records/{import_record_id}/matches/chosen + +Body should contain the condidate_match_id to chose + +=cut + +sub set_chosen { + my $c = shift->openapi->valid_input or return; + + my $import_record_id = $c->validation->param('import_record_id'); + my $body = $c->validation->param('body'); + my $candidate_match_id = $body->{'candidate_match_id'}; + + my $match = Koha::Import::Record::Matches->find({ + import_record_id => $import_record_id, + candidate_match_id => $candidate_match_id + }); + + unless ($match) { + return $c->render( + status => 404, + openapi => { error => "Match not found" } + ); + } + + return try { + my $matches = Koha::Import::Record::Matches->search({ + import_record_id => $import_record_id, + chosen => 1 + }); + $matches->unset_chosen if $matches; + $match->set_from_api({ chosen => JSON::true }); + $match->store; + return $c->render( status => 200, openapi => $match ); + } + catch { + $c->unhandled_exception($_); + }; +} + +1; diff --git a/api/v1/swagger/definitions.json b/api/v1/swagger/definitions.json index e35054a456..41b2e3aa1e 100644 --- a/api/v1/swagger/definitions.json +++ b/api/v1/swagger/definitions.json @@ -44,6 +44,9 @@ "import_batch_profiles": { "$ref": "definitions/import_batch_profiles.json" }, + "import_record_match": { + "$ref": "definitions/import_record_match.json" + }, "library": { "$ref": "definitions/library.json" }, diff --git a/api/v1/swagger/definitions/import_record_match.json b/api/v1/swagger/definitions/import_record_match.json new file mode 100644 index 0000000000..abde7365b0 --- /dev/null +++ b/api/v1/swagger/definitions/import_record_match.json @@ -0,0 +1,21 @@ +{ + "type": "object", + "properties": { + "import_record_id": { + "type": "integer", + "description": "Internal import record identifier" + }, + "candidate_match_id": { + "type": "integer", + "description": "Internal import record match candidate identifier" + }, + "chosen": { + "type": "boolean", + "description": "Whether match has been chosen for overlay" + }, + "score": { + "type": "integer", + "description": "Ranking value for this match calculated by the matching rules" + } + } +} diff --git a/api/v1/swagger/parameters.json b/api/v1/swagger/parameters.json index ddced814d0..8a052baddb 100644 --- a/api/v1/swagger/parameters.json +++ b/api/v1/swagger/parameters.json @@ -20,6 +20,9 @@ "hold_id_pp": { "$ref": "parameters/hold.json#/hold_id_pp" }, + "import_record_id_pp": { + "$ref": "parameters/import_record_match.json#/import_record_id_pp" + }, "club_id_pp": { "$ref": "parameters/club.json#/club_id_pp" }, diff --git a/api/v1/swagger/parameters/import_record_match.json b/api/v1/swagger/parameters/import_record_match.json new file mode 100644 index 0000000000..5268c2a2dc --- /dev/null +++ b/api/v1/swagger/parameters/import_record_match.json @@ -0,0 +1,16 @@ +{ + "import_record_id_pp": { + "name": "import_record_id", + "in": "path", + "description": "Internal import_record identifier", + "required": true, + "type": "integer" + }, + "candidate_match_id_pp": { + "name": "candidate_match_id", + "in": "path", + "description": "Internal import_record_match_candidate identifier", + "required": true, + "type": "integer" + } +} diff --git a/api/v1/swagger/paths.json b/api/v1/swagger/paths.json index 3b5dc352d5..a0c5911f0d 100644 --- a/api/v1/swagger/paths.json +++ b/api/v1/swagger/paths.json @@ -62,6 +62,9 @@ "/holds/{hold_id}/pickup_locations": { "$ref": "paths/holds.json#/~1holds~1{hold_id}~1pickup_locations" }, + "/import/{import_batch_id}/records/{import_record_id}/matches/chosen": { + "$ref": "paths/import_record_matches.json#/~1import~1{import_batch_id}~1records~1{import_record_id}~1matches~1chosen" + }, "/items": { "$ref": "paths/items.json#/~1items" }, diff --git a/api/v1/swagger/paths/import_record_matches.json b/api/v1/swagger/paths/import_record_matches.json new file mode 100644 index 0000000000..3c0be19d97 --- /dev/null +++ b/api/v1/swagger/paths/import_record_matches.json @@ -0,0 +1,144 @@ +{ + "/import/{import_batch_id}/records/{import_record_id}/matches/chosen": { + "put": { + "x-mojo-to": "ImportRecordMatches#set_chosen", + "operationId": "setChosen", + "tags": ["import_record_matches"], + "parameters": [{ + "name": "import_batch_id", + "in": "path", + "required": true, + "description": "An import_batch ID", + "type": "integer" + }, { + "name": "import_record_id", + "in": "path", + "required": true, + "description": "An import_record ID", + "type": "integer" + }, { + "name": "body", + "in": "body", + "description": "A JSON object containing fields to modify", + "required": true, + "schema": { + "type": "object", + "properties": { + "candidate_match_id": { + "description": "Candudate match to choose", + "type": "integer" + } + } + } + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "responses": { + "200": { + "description": "Match updated" + }, + "400": { + "description": "Missing or wrong parameters", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "401": { + "description": "Authentication required", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "403": { + "description": "Match management not allowed", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "404": { + "description": "Import record match not found", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "500": { + "description": "Internal server error", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "503": { + "description": "Under maintenance", + "schema": { + "$ref": "../definitions.json#/error" + } + } + }, + "x-koha-authorization": { + "permissions": { + "tools": "manage_staged_marc" + } + } + }, + "delete": { + "x-mojo-to": "ImportRecordMatches#unset_chosen", + "operationId": "unsetChosen", + "tags": ["import_record_matches"], + "parameters": [{ + "name": "import_batch_id", + "in": "path", + "required": true, + "description": "An import_batch ID", + "type": "integer" + }, { + "name": "import_record_id", + "in": "path", + "required": true, + "description": "An import_record ID", + "type": "integer" + }], + "produces": ["application/json"], + "responses": { + "204": { + "description": "Matches unchosen" + }, + "401": { + "description": "Authentication required", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "403": { + "description": "Match management not allowed", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "404": { + "description": "Import record matches not found", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "500": { + "description": "Internal server error", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "503": { + "description": "Under maintenance", + "schema": { + "$ref": "../definitions.json#/error" + } + } + }, + "x-koha-authorization": { + "permissions": { + "tools": "manage_staged_marc" + } + } + } + } +} diff --git a/api/v1/swagger/x-primitives.json b/api/v1/swagger/x-primitives.json index 0594264be9..f7399b4cc2 100644 --- a/api/v1/swagger/x-primitives.json +++ b/api/v1/swagger/x-primitives.json @@ -3,6 +3,14 @@ "type": "integer", "description": "Internal biblio identifier" }, + "import_record_id": { + "type": "integer", + "description": "Internal import record identifier" + }, + "candidate_match_id": { + "type": "integer", + "description": "Internal import record match identifier" + }, "advancededitormacro_id": { "type": "integer", "description": "Internal advanced editor macro identifier", diff --git a/installer/data/mysql/atomicupdate/bug_22785.perl b/installer/data/mysql/atomicupdate/bug_22785.perl new file mode 100644 index 0000000000..9630904b87 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_22785.perl @@ -0,0 +1,8 @@ +$DBversion = 'XXX'; +if( CheckVersion( $DBversion ) ) { + + unless( column_exists('import_record_matches','chosen') ){ + $dbh->do( "ALTER TABLE import_record_matches ADD COLUMN chosen TINYINT null DEFAULT null AFTER score" ); + } + NewVersion( $DBversion, 22785, "Add chosen column to miport_record_matches"); +} diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 2cd118c444..662adfca34 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -787,6 +787,7 @@ CREATE TABLE `import_record_matches` ( -- matches found when importing a batch o `import_record_id` int(11) NOT NULL, -- the id given to the imported bib record (import_records.import_record_id) `candidate_match_id` int(11) NOT NULL, -- the biblio the imported record matches (biblio.biblionumber) `score` int(11) NOT NULL default 0, -- the match score + `chosen` TINYINT(1) NULL default NULL, -- whether this match has been allowed or denied PRIMARY KEY (`import_record_id`,`candidate_match_id`), CONSTRAINT `import_record_matches_ibfk_1` FOREIGN KEY (`import_record_id`) REFERENCES `import_records` (`import_record_id`) ON DELETE CASCADE ON UPDATE CASCADE, 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 824354064a..1388a2f1c8 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 @@ -13,6 +13,7 @@ @@ -466,8 +467,8 @@ { "mDataProp": "citation" }, { "mDataProp": "status" }, { "mDataProp": "overlay_status" }, - { "mDataProp": "match_citation" }, - { "mDataProp": "diff_url" }, + { "mDataProp": "" }, + { "mDataProp": "" }, { "mDataProp": "matched" } ], "fnServerData": function(sSource, aoData, fnCallback) { @@ -523,22 +524,45 @@ aData['overlay_status'] ); - if (aData['match_id']) { - [% IF(record_type == 'auth') -%] - var matching_msg = _("Matches authority %s (score=%s):%s"); - [%- ELSE -%] - var matching_msg = _("Matches bibliographic record %s (score=%s):%s"); - [%- END %] - $('td:eq(4)', nRow).html( - matching_msg.format(aData['match_id'], aData['score'], - '' + aData['match_citation'] + '') - ); - } - if (aData['diff_url']) { - $('td:eq(5)', nRow).html( - '' + _("View") + '' - ); + if ( aData['matches'].length > 0 ) { + + var any_checked = 0; + $('td:eq(4)', nRow).html(''); + $('td:eq(5)', nRow).html(''); + var checked = ""; + var disabled = ""; + if( aData['status'] == "imported" || aData['status'] == "ignored" ){ + disabled = ' disabled '; + } + aData['matches'].forEach(function(item,index){ + if( item.chosen == 1 ){ + checked = 'checked="checked"'; + any_checked = 1; + } + [% IF(item.record_type == 'auth') -%] + var matching_msg = _("Matches authority %s (score=%s):%s"); + [%- ELSE -%] + var matching_msg = _("Matches bibliographic record %s (score=%s):%s"); + [%- END %] + var match_option = ""; + match_option = ' '; + + var diff_url = '/cgi-bin/koha/tools/showdiffmarc.pl?batchid=%s&importid=%s&id=%s&type=%s'; + var match_citation = ''; + if( item.title ){ match_citation += item.title + ' ' } + if( item.author ){ match_citation += item.author } + $('td:eq(4) ul', nRow).append('
    • ') + ); + $('td:eq(5) ul', nRow).append('
  • ' + _("View") + '
    • '); + }); + checked = ""; + if( !any_checked ){ checked = 'checked="checked"'; } + $('td:eq(4) ul', nRow).prepend('
    • '); + $('td:eq(5) ul', nRow).prepend('
  •  
    • '); } $('td:eq(6)', nRow).html( 'prepare($strcalc); $dbcalc->execute; diff --git a/t/db_dependent/ImportBatch.t b/t/db_dependent/ImportBatch.t index 863c903728..916790fbab 100755 --- a/t/db_dependent/ImportBatch.t +++ b/t/db_dependent/ImportBatch.t @@ -1,7 +1,7 @@ #!/usr/bin/perl use Modern::Perl; -use Test::More tests => 15; +use Test::More tests => 16; use utf8; use File::Basename; use File::Temp qw/tempfile/; @@ -208,4 +208,33 @@ subtest "RecordsFromMarcPlugin" => sub { 'Checked one field in second record' ); }; +subtest "_get_commit_action" => sub { + plan tests => 24; + my $mock_import = Test::MockModule->new("C4::ImportBatch"); + + $mock_import->mock( GetBestRecordMatch => sub { return 5; } ); + foreach my $record_type ( ('biblio','authority') ){ + foreach my $match_action ( ('replace','create_new','ignore') ){ + foreach my $no_match_action ( ('create_new','ignore') ){ + my ($result, $match, $item_result) = + C4::ImportBatch::_get_commit_action($match_action, $no_match_action, 'always_add', 'auto_match', 42, $record_type); + is( $result, $match_action, "When match found amd chosen we return the match_action for $record_type records with match action $match_action and no match action $no_match_action"); + } + } + } + + $mock_import->mock( GetBestRecordMatch => sub { my $matches = undef; return $matches; } ); + foreach my $record_type ( ('biblio','authority') ){ + foreach my $match_action ( ('replace','create_new','ignore') ){ + foreach my $no_match_action ( ('create_new','ignore') ){ + my ($result, $match, $item_result) = + C4::ImportBatch::_get_commit_action($match_action, $no_match_action, 'always_add', 'auto_match', 42, $record_type); + is( $result, $no_match_action, "When no match found or chosen we return the match_action for $record_type records with match action $match_action and no match action $no_match_action"); + } + } + } + +}; + + $schema->storage->txn_rollback; diff --git a/t/db_dependent/api/v1/import_record_matches.t b/t/db_dependent/api/v1/import_record_matches.t new file mode 100755 index 0000000000..9e79e4a85c --- /dev/null +++ b/t/db_dependent/api/v1/import_record_matches.t @@ -0,0 +1,167 @@ +#!/usr/bin/env perl + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 1; +use Test::Mojo; +use Test::Warn; + +use t::lib::TestBuilder; +use t::lib::Mocks; + +use C4::Auth; +use Koha::Import::Record::Matches; + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + +# FIXME: sessionStorage defaults to mysql, but it seems to break transaction handling +# this affects the other REST api tests +t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' ); + +my $remote_address = '127.0.0.1'; +my $t = Test::Mojo->new('Koha::REST::V1'); + +subtest 'import record matches tests' => sub { + + plan tests => 13; + + $schema->storage->txn_begin; + + my ( $unauthorized_borrowernumber, $unauthorized_session_id ) = + create_user_and_session( { authorized => 0 } ); + my ( $authorized_borrowernumber, $authorized_session_id ) = + create_user_and_session( { authorized => 1 } ); + + my $match_1 = $builder->build_object({ + class => 'Koha::Import::Record::Matches', + value => { + chosen => 0, + } + }); + my $match_2 = $builder->build_object({ + class => 'Koha::Import::Record::Matches', + value => { + import_record_id => $match_1->import_record_id, + chosen => 1, + } + }); + my $del_match = $builder->build_object({ class => 'Koha::Import::Record::Matches' }); + my $del_import_batch_id = $del_match->import_record->import_batch_id; + my $del_match_id = $del_match->import_record_id; + + # Unauthorized attempt to update + my $tx = $t->ua->build_tx( + PUT => "/api/v1/import/".$match_1->import_record->import_batch_id."/records/".$match_1->import_record_id."/matches/chosen"=> + json => { + candidate_match_id => $match_1->candidate_match_id + } + ); + $tx->req->cookies( + { name => 'CGISESSID', value => $unauthorized_session_id } ); + $tx->req->env( { REMOTE_ADDR => $remote_address } ); + $t->request_ok($tx)->status_is(403); + + # Invalid attempt to allow match on a non-existent record + $tx = $t->ua->build_tx( + PUT => "/api/v1/import/".$del_import_batch_id."/records/".$del_match_id."/matches/chosen" => + json => { + candidate_match_id => $match_1->candidate_match_id + } + ); + + $tx->req->cookies( + { name => 'CGISESSID', value => $authorized_session_id } ); + $tx->req->env( { REMOTE_ADDR => $remote_address } ); + $del_match->delete(); + $t->request_ok($tx)->status_is(404) + ->json_is( '/error' => "Match not found" ); + + # Valid, authorised update + $tx = $t->ua->build_tx( + PUT => "/api/v1/import/".$match_1->import_record->import_batch_id."/records/".$match_1->import_record_id."/matches/chosen" => + json => { + candidate_match_id => $match_1->candidate_match_id + } + ); + $tx->req->cookies( + { name => 'CGISESSID', value => $authorized_session_id } ); + $tx->req->env( { REMOTE_ADDR => $remote_address } ); + $t->request_ok($tx)->status_is(200); + + $match_1->discard_changes; + $match_2->discard_changes; + ok( $match_1->chosen,"Match 1 is correctly set to chosen"); + ok( !$match_2->chosen,"Match 2 correctly unset when match 1 is set"); + + # Valid unsetting + $tx = $t->ua->build_tx( + DELETE => "/api/v1/import/".$match_1->import_record->import_batch_id."/records/".$match_1->import_record_id."/matches/chosen" => + json => { + } + ); + $tx->req->cookies( + { name => 'CGISESSID', value => $authorized_session_id } ); + $tx->req->env( { REMOTE_ADDR => $remote_address } ); + $t->request_ok($tx)->status_is(204); + + $match_1->discard_changes; + $match_2->discard_changes; + ok( !$match_1->chosen,"Match 1 is correctly unset to chosen"); + ok( !$match_2->chosen,"Match 2 is correctly unset to chosen"); + + $schema->storage->txn_rollback; +}; + +sub create_user_and_session { + + my $args = shift; + my $dbh = C4::Context->dbh; + + my $user = $builder->build( + { + source => 'Borrower', + value => { + flags => 0 + } + } + ); + + # Create a session for the authorized user + my $session = C4::Auth::get_session(''); + $session->param( 'number', $user->{borrowernumber} ); + $session->param( 'id', $user->{userid} ); + $session->param( 'ip', '127.0.0.1' ); + $session->param( 'lasttime', time() ); + $session->flush; + + if ( $args->{authorized} ) { + $builder->build({ + source => 'UserPermission', + value => { + borrowernumber => $user->{borrowernumber}, + module_bit => 13, + code => 'manage_staged_marc', + } + }); + } + + return ( $user->{borrowernumber}, $session->id ); +} + +1; diff --git a/tools/batch_records_ajax.pl b/tools/batch_records_ajax.pl index 4ddb0f5369..d1ef2c5733 100755 --- a/tools/batch_records_ajax.pl +++ b/tools/batch_records_ajax.pl @@ -69,21 +69,23 @@ my @list = (); foreach my $record (@$records) { my $citation = $record->{'title'} || $record->{'authorized_heading'}; - my $match = GetImportRecordMatches( $record->{'import_record_id'}, 1 ); - my $match_citation = ''; + my $matches = GetImportRecordMatches( $record->{'import_record_id'} ); my $match_id; - if ( $#$match > -1 ) { - if ( $match->[0]->{'record_type'} eq 'biblio' ) { - $match_citation .= $match->[0]->{'title'} - if defined( $match->[0]->{'title'} ); - $match_citation .= ' ' . $match->[0]->{'author'} - if defined( $match->[0]->{'author'} ); - $match_id = $match->[0]->{'biblionumber'}; - } - elsif ( $match->[0]->{'record_type'} eq 'auth' ) { - if ( defined( $match->[0]->{'authorized_heading'} ) ) { - $match_citation .= $match->[0]->{'authorized_heading'}; - $match_id = $match->[0]->{'candidate_match_id'}; + if ( scalar @$matches > 0 ) { + foreach my $match (@$matches){ + my $match_citation = ''; + if ( $match->{'record_type'} eq 'biblio' ) { + $match_citation .= $match->{'title'} + if defined( $match->{'title'} ); + $match_citation .= ' ' . $match->{'author'} + if defined( $match->{'author'} ); + $match->{'match_citation'} = $match_citation; + } + elsif ( $match->{'record_type'} eq 'auth' ) { + if ( defined( $match->{'authorized_heading'} ) ) { + $match_citation .= $match->{'authorized_heading'}; + $match->{'match_citation'} = $match_citation; + } } } } @@ -98,12 +100,11 @@ foreach my $record (@$records) { isbn => $record->{'isbn'}, status => $record->{'status'}, overlay_status => $record->{'overlay_status'}, - match_citation => $match_citation, matched => $record->{'matched_biblionumber'} || $record->{'matched_authid'} || q{}, - score => $#$match > -1 ? $match->[0]->{'score'} : 0, - match_id => $match_id, + score => scalar @$matches > 0 ? $matches->[0]->{'score'} : 0, + matches => $matches, diff_url => $match_id ? "/cgi-bin/koha/tools/showdiffmarc.pl?batchid=$import_batch_id&importid=$record->{import_record_id}&id=$match_id&type=$record->{record_type}" : undef }; } -- 2.11.0