From 5e9b0b8f2e9780c84334a2176329365e1f56f41a 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 | 29 ++++ t/db_dependent/Koha/Import/Record/Matches.t | 14 +- t/db_dependent/api/v1/import_record_matches.t | 167 +++++++++++++++++++++ tools/batch_records_ajax.pl | 35 ++--- 17 files changed, 620 insertions(+), 71 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 2a5696d513..9e59a88efa 100644 --- a/C4/ImportBatch.pm +++ b/C4/ImportBatch.pm @@ -690,7 +690,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; @@ -1179,6 +1178,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(); @@ -1459,12 +1459,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(); @@ -1497,10 +1498,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 } } @@ -1717,41 +1720,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..33de3062f9 --- /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 02dbc95c01..bfe7a93982 100644 --- a/api/v1/swagger/definitions.json +++ b/api/v1/swagger/definitions.json @@ -47,6 +47,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 95464e1461..71374834ee 100644 --- a/api/v1/swagger/parameters.json +++ b/api/v1/swagger/parameters.json @@ -23,6 +23,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 ba0c1e8b0f..ad1a20b402 100644 --- a/api/v1/swagger/paths.json +++ b/api/v1/swagger/paths.json @@ -77,6 +77,9 @@ "/holds/{hold_id}/pickup_location": { "$ref": "paths/holds.json#/~1holds~1{hold_id}~1pickup_location" }, + "/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 491f8304ff..1388d5a873 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 4d100ecaa1..682830fb60 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -2902,6 +2902,7 @@ CREATE TABLE `import_record_matches` ( `import_record_id` int(11) NOT NULL COMMENT 'the id given to the imported bib record (import_records.import_record_id)', `candidate_match_id` int(11) NOT NULL COMMENT 'the biblio the imported record matches (biblio.biblionumber)', `score` int(11) NOT NULL DEFAULT 0 COMMENT 'the match score', + `chosen` tinyint(1) NULL DEFAULT NULL COMMENT 'whether this match has been allowed or denied', PRIMARY KEY (`import_record_id`,`candidate_match_id`), KEY `record_score` (`import_record_id`,`score`), 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 2a56b1a2d3..eeae4450ae 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 @@ @@ -483,8 +484,8 @@ { "mDataProp": "citation" }, { "mDataProp": "status" }, { "mDataProp": "overlay_status" }, - { "mDataProp": "match_citation" }, - { "mDataProp": "diff_url" }, + { "mDataProp": "" }, + { "mDataProp": "" }, { "mDataProp": "matched" } ], "fnServerData": function(sSource, aoData, fnCallback) { @@ -540,22 +541,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 da0c1899b2..00bcf77a5a 100755 --- a/t/db_dependent/ImportBatch.t +++ b/t/db_dependent/ImportBatch.t @@ -213,4 +213,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/Koha/Import/Record/Matches.t b/t/db_dependent/Koha/Import/Record/Matches.t index f37653b363..87c16d94ac 100755 --- a/t/db_dependent/Koha/Import/Record/Matches.t +++ b/t/db_dependent/Koha/Import/Record/Matches.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 5; +use Test::More tests => 7; use Koha::Import::Record::Matches; use Koha::Database; @@ -31,15 +31,23 @@ $schema->storage->txn_begin; my $builder = t::lib::TestBuilder->new; my $nb_of_matches = Koha::Import::Record::Matches->search->count; +my $nb_of_chosen_matches = Koha::Import::Record::Matches->search({ chosen => 1 })->count; -my $match_1 = $builder->build({ source => 'ImportRecordMatch', }); -my $match_2 = $builder->build({ source => 'ImportRecordMatch', value => { import_record_id => $match_1->{import_record_id} } }); +my $match_1 = $builder->build({ source => 'ImportRecordMatch', value => { chosen => 1 } }); +my $match_2 = $builder->build({ source => 'ImportRecordMatch', value => { chosen => 1, import_record_id => $match_1->{import_record_id} } }); is( Koha::Import::Record::Matches->search->count, $nb_of_matches + 2, 'The 2 matches should have been added' ); +is( Koha::Import::Record::Matches->search({chosen => 1 })->count, $nb_of_chosen_matches + 2, 'The 2 chosen matches should have been added' ); + my $retrieved_match_1 = Koha::Import::Record::Matches->search({ import_record_id => $match_1->{import_record_id}, candidate_match_id => $match_1->{candidate_match_id} })->next; is_deeply( $retrieved_match_1->unblessed, $match_1, 'Find a match by import record id and candidate should return the correct match' ); +my $added_chose = Koha::Import::Record::Matches->search({ import_record_id => $match_1->{import_record_id} }); +$added_chose->unset_chosen(); + +is( Koha::Import::Record::Matches->search({chosen => 1 })->count, $nb_of_chosen_matches, 'The 2 added chosen matches should have been unchosen' ); + $retrieved_match_1->delete; is( Koha::Import::Record::Matches->search->count, $nb_of_matches + 1, 'Delete should have deleted the match' ); 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