From bfb2bf20fd562029b8d29713c98a160099d47024 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 Signed-off-by: Andrew Fuerste-Henry Bug 22785: API files Signed-off-by: Ben Daeuber Signed-off-by: Katrin Fischer Signed-off-by: Martin Renvoize --- C4/ImportBatch.pm | 58 +++--- Koha/REST/V1/ImportRecordMatches.pm | 106 +++++++++++ .../definitions/import_record_match.json | 21 +++ .../definitions/import_record_match.yaml | 16 ++ .../parameters/import_record_match.json | 16 ++ .../parameters/import_record_match.yaml | 13 ++ .../swagger/paths/import_record_matches.json | 144 +++++++++++++++ .../swagger/paths/import_record_matches.yaml | 106 +++++++++++ api/v1/swagger/swagger.yaml | 16 ++ .../data/mysql/atomicupdate/bug_22785.pl | 16 ++ installer/data/mysql/kohastructure.sql | 1 + .../en/modules/tools/manage-marc-import.tt | 69 ++++++-- t/db_dependent/ImportBatch.t | 30 +++- t/db_dependent/Koha/Import/Record/Matches.t | 9 +- t/db_dependent/api/v1/import_record_matches.t | 167 ++++++++++++++++++ tools/batch_records_ajax.pl | 35 ++-- 16 files changed, 756 insertions(+), 67 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/definitions/import_record_match.yaml create mode 100644 api/v1/swagger/parameters/import_record_match.json create mode 100644 api/v1/swagger/parameters/import_record_match.yaml create mode 100644 api/v1/swagger/paths/import_record_matches.json create mode 100644 api/v1/swagger/paths/import_record_matches.yaml create mode 100755 installer/data/mysql/atomicupdate/bug_22785.pl create mode 100755 t/db_dependent/api/v1/import_record_matches.t diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm index 2692263fe8..f693059d0d 100644 --- a/C4/ImportBatch.pm +++ b/C4/ImportBatch.pm @@ -711,7 +711,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; @@ -1199,6 +1198,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(); @@ -1479,12 +1479,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(); @@ -1517,10 +1518,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 } } @@ -1737,41 +1740,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..cb7d022e78 --- /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->update({ chosen => 0 }); + 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->update({ chosen => 0}) 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/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/definitions/import_record_match.yaml b/api/v1/swagger/definitions/import_record_match.yaml new file mode 100644 index 0000000000..cba025bef0 --- /dev/null +++ b/api/v1/swagger/definitions/import_record_match.yaml @@ -0,0 +1,16 @@ +--- +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 +additionalProperties: false 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/parameters/import_record_match.yaml b/api/v1/swagger/parameters/import_record_match.yaml new file mode 100644 index 0000000000..376c406bca --- /dev/null +++ b/api/v1/swagger/parameters/import_record_match.yaml @@ -0,0 +1,13 @@ +--- +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/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/paths/import_record_matches.yaml b/api/v1/swagger/paths/import_record_matches.yaml new file mode 100644 index 0000000000..67abad548c --- /dev/null +++ b/api/v1/swagger/paths/import_record_matches.yaml @@ -0,0 +1,106 @@ +--- +"/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: Candidate match to choose + type: integer + consumes: + - application/json + produces: + - application/json + responses: + "200": + description: Match updated + "400": + description: Missing or wrong parameters + schema: + $ref: "../swagger.yaml#/definitions/error" + "401": + description: Authentication required + schema: + $ref: "../swagger.yaml#/definitions/error" + "403": + description: Match management not allowed + schema: + $ref: "../swagger.yaml#/definitions/error" + "404": + description: Import record match not found + schema: + $ref: "../swagger.yaml#/definitions/error" + "500": + description: Internal server error + schema: + $ref: "../swagger.yaml#/definitions/error" + "503": + description: Under maintenance + schema: + $ref: "../swagger.yaml#/definitions/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: "../swagger.yaml#/definitions/error" + "403": + description: Match management not allowed + schema: + $ref: "../swagger.yaml#/definitions/error" + "404": + description: Import record matches not found + schema: + $ref: "../swagger.yaml#/definitions/error" + "500": + description: Internal server error + schema: + $ref: "../swagger.yaml#/definitions/error" + "503": + description: Under maintenance + schema: + $ref: "../swagger.yaml#/definitions/error" + x-koha-authorization: + permissions: + tools: manage_staged_marc diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml index 7ffd64d52a..16fa5a0e9c 100644 --- a/api/v1/swagger/swagger.yaml +++ b/api/v1/swagger/swagger.yaml @@ -36,6 +36,8 @@ definitions: $ref: ./definitions/import_batch_profile.yaml import_batch_profiles: $ref: ./definitions/import_batch_profiles.yaml + import_record_match: + $ref: ./definitions/import_record_match.yaml invoice: $ref: ./definitions/invoice.yaml item: @@ -141,6 +143,8 @@ paths: $ref: "./paths/ill_backends.yaml#/~1ill_backends~1{ill_backend_id}" /illrequests: $ref: ./paths/illrequests.yaml#/~1illrequests + "/import/{import_batch_id}/records/{import_record_id}/matches/chosen": + $ref: "./paths/import_record_matches.yaml#/~1import~1{import_batch_id}~1records~1{import_record_id}~1matches~1chosen" /import_batch_profiles: $ref: ./paths/import_batch_profiles.yaml#/~1import_batch_profiles "/import_batch_profiles/{import_batch_profile_id}": @@ -228,6 +232,12 @@ parameters: name: biblio_id required: true type: integer + candidate_match_id_pp: + description: Internal import record match identifier + in: path + name: candidate_match_id + required: true + type: integer cash_register_id_pp: description: Cash register internal identifier in: path @@ -276,6 +286,12 @@ parameters: name: import_batch_profile_id required: true type: integer + import_record_id_pp: + description: Internal import record identifier + in: path + name: import_record_id + required: true + type: integer item_id_pp: description: Internal item identifier in: path diff --git a/installer/data/mysql/atomicupdate/bug_22785.pl b/installer/data/mysql/atomicupdate/bug_22785.pl new file mode 100755 index 0000000000..8a4b92ada8 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_22785.pl @@ -0,0 +1,16 @@ +use Modern::Perl; + +return { + bug_number => "22785", + description => "Add chosen column to import_record_matches", + up => sub { + my ($args) = @_; + my ($dbh, $out) = @$args{qw(dbh out)}; + unless( column_exists('import_record_matches','chosen') ){ + $dbh->do(q{ + ALTER TABLE import_record_matches ADD COLUMN chosen TINYINT null DEFAULT null AFTER score + }); + say $out "chosen column added to import_record_matches"; + } + }, +} diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index f7ec7186b9..1651dd3fa6 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -2939,6 +2939,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 258cadd50b..7eb09b1c44 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 @@ @@ -485,8 +486,8 @@ { "mDataProp": "citation" }, { "mDataProp": "status" }, { "mDataProp": "overlay_status" }, - { "mDataProp": "match_citation" }, - { "mDataProp": "diff_url" }, + { "mDataProp": "" }, + { "mDataProp": "" }, { "mDataProp": "matched" } ], "fnServerData": function(sSource, aoData, fnCallback) { @@ -547,17 +548,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['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('
    •  
      • '); } if (aData['diff_url']) { $('td:eq(5)', nRow).html( @@ -584,6 +613,20 @@ }); [% END %] + $("body").on("change", ".chosen", function(e) { + let apimethod = "DELETE"; + let apidata =""; + if( $(this).val() != 'none' ){ + apimethod = 'PUT'; + apidata = JSON.stringify({ candidate_match_id: $(this).val() }); + } + $.ajax({ + url: '/api/v1/import/[% import_batch_id | html %]/records/'+$(this).data('import_record_id')+'/matches/chosen', + method: apimethod, + data: apidata + }).fail(function(){ alert("Unable to update match choices"); return false; }); + }); + $("body").on("click", ".previewMARC", function(e) { e.preventDefault(); var ltitle = $(this).text(); diff --git a/t/db_dependent/ImportBatch.t b/t/db_dependent/ImportBatch.t index 11167622ff..7548c398ab 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 => 17; +use Test::More tests => 18; use utf8; use File::Basename; use File::Temp qw/tempfile/; @@ -214,6 +214,34 @@ 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"); + } + } + } + +}; + sub get_import_record { my $id_import_batch = shift; return $dbh->do('SELECT * FROM import_records WHERE import_batch_id = ?', undef, $id_import_batch); diff --git a/t/db_dependent/Koha/Import/Record/Matches.t b/t/db_dependent/Koha/Import/Record/Matches.t index f37653b363..36c12a6a7b 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 => 6; use Koha::Import::Record::Matches; use Koha::Database; @@ -31,12 +31,15 @@ $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' ); 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 06e3bf69de..3d706013ba 100755 --- a/tools/batch_records_ajax.pl +++ b/tools/batch_records_ajax.pl @@ -68,21 +68,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; + } } } } @@ -97,12 +99,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.20.1