Attachment 114814 Details for Bug 22785 – Bug 22785: Allow option to choose which record match is applied during import

[patch] Bug 22785: Allow option to choose which record match is applied during import

Bug-22785-Allow-option-to-choose-which-record-matc.patch (text/plain), 37.68 KB, created by Nick Clemens (kidclamp) on 2021-01-04 16:53:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2021-01-04 16:53:00 UTC

Size: 37.68 KB

patch

obsolete

>From 5f92411d938ce475d5253f14ff6f1e9d15fc2aa3 Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>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 <http://www.gnu.org/licenses>.
>+
>+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 @@
> <style>
>     #jobpanel,#jobstatus,#jobfailed { display : none; }
>     span.change-status { font-style:italic; color:#666; display:none; }
>+    ul.matches li { list-style-type:none; }
> </style>
> </head>
> 
>@@ -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'],
>-                                    '<a target="_blank" href="' + record_details_url
>-                                    + aData['match_id'] + '">' + aData['match_citation'] + '</a>')
>-                            );
>-                        }
>-                        if (aData['diff_url']) {
>-                            $('td:eq(5)', nRow).html(
>-                                '<a href="' + aData['diff_url'] + '">' + _("View") + '</a>'
>-                            );
>+                        if ( aData['matches'].length > 0 ) {
>+
>+                            var any_checked = 0;
>+                            $('td:eq(4)', nRow).html('<ul class="matches"></ul>');
>+                            $('td:eq(5)', nRow).html('<ul class="matches"></ul>');
>+                            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 = '<input type="radio" data-import_record_id="'+aData['import_record_id']+'" class="chosen" name="import_record_id_'+aData['import_record_id']+'_match" value="'+item.candidate_match_id+'" ' + checked + disabled + '> ';
>+
>+                                    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('<li><label for="import_record_id_'+aData['import_record_id']+'_match_'+index+'">'+match_option+
>+                                    matching_msg.format(item.candidate_match_id, item.score,
>+                                        '<a target="_blank" href="' + record_details_url
>+                                        + item.candidate_match_id + '">' + match_citation + '</a></label></li>')
>+                                );
>+                                $('td:eq(5) ul', nRow).append('<li><a href="'
>+                                        + diff_url.format( [% import_batch_id | html %], aData['import_record_id'], item.candidate_match_id, item.record_type) + '">' + _("View") + '</a></li>');
>+                            });
>+                            checked = "";
>+                            if( !any_checked ){ checked = 'checked="checked"'; }
>+                            $('td:eq(4) ul', nRow).prepend('<li><label><input type="radio" data-import_record_id="'+aData['import_record_id']+'" class="chosen" name="import_record_id_'+aData['import_record_id']+'_match" value="none" ' + checked + disabled + ' > Ignore matches</label></li>');
>+                            $('td:eq(5) ul', nRow).prepend('<li>&nbsp;</li>');
>                         }
>                         $('td:eq(6)', nRow).html(
>                             '<a target="_blank" href="' + record_details_url
>@@ -554,6 +578,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/reports/acquisitions_stats.pl b/reports/acquisitions_stats.pl
>index b560e459a4..2fd25f08fd 100755
>--- a/reports/acquisitions_stats.pl
>+++ b/reports/acquisitions_stats.pl
>@@ -496,6 +496,7 @@ sub calculate {
>       if ( @$filters[10] );
> 
>     $strcalc .= " GROUP BY $linefield, $colfield ORDER BY $linefield,$colfield";
>+    warn $strcalc;
>     my $dbcalc = $dbh->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 <http://www.gnu.org/licenses>.
>+
>+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

Actions: View | Diff | Splinter Review

Attachments on bug 22785: 114813 | 114814 | 114815 | 114816 | 120995 | 120996 | 120997 | 121149 | 121167 | 121168 | 121169 | 121170 | 121171 | 121172 | 121173 | 121174 | 121204 | 121205 | 121206 | 121207 | 126369 | 126370 | 126371 | 126372 | 126522 | 126523 | 126524 | 126525 | 126531 | 126532 | 126533 | 126534 | 126535 | 126536 | 126537 | 126538 | 128820 | 128821 | 128822 | 128823 | 128824 | 130726 | 130727 | 130728 | 130729 | 130730 | 131143 | 131144 | 131145 | 131146 | 131147 | 133947 | 133948 | 133949 | 133950 | 133951 | 134127 | 134128 | 134129 | 134130 | 134131 | 134132 | 134219 | 134368 | 134369 | 134370 | 134371 | 134372 | 134373 | 134374 | 134575 | 134608