From e650c2b4ed7e7cad9b082354c87ecad7d6730f3c Mon Sep 17 00:00:00 2001 From: Zeno Tajoli Date: Wed, 15 Nov 2023 08:41:17 +0100 Subject: [PATCH] Bug 33036: REST API: Merge biblio records implements merging of records + attached items, subscriptions etc via the API as an alternative to the web interface: cgi-bin/koha/cataloguing/merge.pl This is a slightly improved version of Zenos patch: I (domm) have converted the code in Koha::Biblio to a more DBICy style and packed it into a transaction (as requested in Comment 23) Even the QA script is happy now! To test: 1) you need an API user with the permissions "editcatalogue" 2) two records: one to be merged into (with biblio_id, eg 262) and another one from which to merge (with biblio_id_to_merge, eg 9) which will be deleted! both records may/should have items, subscription, subscriptionhistory, serial, suggestions orders and holds 3) check both records via the web 4) Apply patch 5) Write a JSON file with inside the field 'biblio_id_to_merge' and the biblionumber from wihich to merge. As example: { "biblio_id_to_merge" : 9 } 6) Execute an API call with correct headers and location. For example: curl -s -u koha:koha --header "Content-Type: application/json" --header "Accept: application/marc-in-json" --request POST "http://127.0.0.1:8080/api/v1/biblios/262/merge" -d @file.json You must to setup the headers and to use a json file with parameters 7) The record with the id 9 is deleted now, the record with 262 has all items, etc attached, the return is: return code 200 and the changed record 262 in marc-in-json format 8) It is possible to override biblio data with an external bib record. You need to put external bib record into the json file in marc-in-json format. To write use the json file uploaded as example You need to fill the fields 'rules' and 'datarecord'. The field 'rules' must contains 'override_ext' To do the call: curl -s -u koha:koha --header "Content-Type: application/json" --header "Accept: application/marc-in-json" --request POST "http://127.0.0.1:8080/api/v1/biblios/XXX/merge" -d @file_with_recod.json 9) The record in 'biblio_id_to_merge' is deleted now, in biblio XXX now there are the bibliographic data of field 'datarecord' of json file, the return is: return code 200 and the changed record XXX in marc-in-json format 10) Go into intranet and do a search. Select two or (better) more record. 11) Merge them; merge must be a success. 12) Test with prove -v t/db_dependent/Koha/Biblio.t 13) Test with prove -v t/db_dependent/api/v1/biblios.t To test with curl the step 8 you can customize the json file attached in bugzilla. The marc-in-json record inside follows the MAR21 standard Sponsored-by: Technische Hochschule Wildau Co-authored-by: Zeno Tajoli Co-authored-by: Thomas Klausner Co-authored-by: Mark Hofstetter <> Signed-off-by: Jan Kissig --- Koha/Biblio.pm | 70 +++++ Koha/REST/V1/Biblios.pm | 84 ++++++ api/v1/swagger/definitions/merge_biblios.yaml | 28 ++ api/v1/swagger/paths/biblios_merge.yaml | 49 ++++ api/v1/swagger/swagger.yaml | 4 + cataloguing/merge.pl | 65 +---- t/db_dependent/Koha/Biblio.t | 38 ++- t/db_dependent/api/v1/biblios.t | 251 +++++++++++++++++- 8 files changed, 533 insertions(+), 56 deletions(-) create mode 100644 api/v1/swagger/definitions/merge_biblios.yaml create mode 100644 api/v1/swagger/paths/biblios_merge.yaml diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index 7d0b244085..42a835b098 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -22,11 +22,17 @@ use Modern::Perl; use List::MoreUtils qw( any ); use URI; use URI::Escape qw( uri_escape_utf8 ); +use Try::Tiny; use C4::Koha qw( GetNormalizedISBN GetNormalizedUPC GetNormalizedOCLCNumber ); +use C4::Biblio qw( DelBiblio ); +use C4::Serials qw( CountSubscriptionFromBiblionumber ); +use C4::Reserves qw( MergeHolds ); +use C4::Acquisition qw( ModOrder GetOrdersByBiblionumber ); use Koha::Database; use Koha::DateUtils qw( dt_from_string ); +use Koha::Exception; use base qw(Koha::Object); @@ -1919,6 +1925,70 @@ sub opac_summary_html { return $summary_html; } +=head3 merge_with + + my $biblio = Koha::Biblios->find($biblionumber); + $biblio->merge_with(\@biblio_ids); + + This subroutine merges a list of bibliographic records into the bibliographic record. + This function DOES NOT CHANGE the bibliographic metadata of the record. But it links all + items, holds, subscriptions, serials issues and article_requests to the record. After doing changes + bibliographic records listed are deleted + +=cut + +sub merge_with { + my ( $self, $biblio_ids ) = @_; + + my $schema = Koha::Database->new()->schema(); + my $ref_biblionumber = $self->biblionumber; + my %results = ( 'biblio_id' => $ref_biblionumber, 'merged_ids' => [] ); + + # Ensure the keeper isn't in the list of records to merge + my @biblio_ids_to_merge = grep { $_ ne $ref_biblionumber } @$biblio_ids; + + try { + $schema->txn_do( + sub { + foreach my $bn_merge (@biblio_ids_to_merge) { + my $from_biblio = Koha::Biblios->find($bn_merge); + $from_biblio->items->move_to_biblio($self); + $from_biblio->article_requests->update( + { biblionumber => $ref_biblionumber }, + { no_triggers => 1 } + ); + + for my $resultset_name (qw(Subscription Subscriptionhistory Serial Suggestion)) { + $schema->resultset($resultset_name)->search( { biblionumber => $bn_merge } ) + ->update( { biblionumber => $ref_biblionumber } ); + } + + # TODO should this be ported to more modern DB usage (i.e. DBIx::Class)? + my @allorders = GetOrdersByBiblionumber($bn_merge); + foreach my $myorder (@allorders) { + $myorder->{'biblionumber'} = $ref_biblionumber; + ModOrder($myorder); + + # TODO : add error control (in ModOrder?) + } + + # Move holds + MergeHolds( $schema->storage->dbh, $ref_biblionumber, $bn_merge ); + my $error = DelBiblio($bn_merge); #DelBiblio return undef unless an error occurs + if ($error) { + die $error; + } else { + push( @{ $results{merged_ids} }, $bn_merge ); + } + } + } + ); + } catch { + Koha::Exception->throw($_); + }; + return \%results; +} + =head2 Internal methods =head3 type diff --git a/Koha/REST/V1/Biblios.pm b/Koha/REST/V1/Biblios.pm index 846a0fb94d..ec174d4f80 100644 --- a/Koha/REST/V1/Biblios.pm +++ b/Koha/REST/V1/Biblios.pm @@ -35,6 +35,7 @@ use List::MoreUtils qw( any ); use MARC::Record::MiJ; use Try::Tiny qw( catch try ); +use JSON qw( decode_json ); =head1 API @@ -892,4 +893,87 @@ sub list { }; } +=head3 merge + +Controller function that handles merging two biblios. If an optional +MARCXML is provided as the request body, this MARCXML replaces the +bibliodata of the merge target biblio. Syntax format inside the request body +must match with the Marc format used into Koha installation (MARC21 or UNIMARC) + +=cut + +sub merge { + my $c = shift->openapi->valid_input or return; + my $ref_biblionumber = $c->param('biblio_id'); + my $json = decode_json( $c->req->body ); + my $bn_merge = $json->{'biblio_id_to_merge'}; + my $framework = $json->{'framework_to_use'}; + my $rules = $json->{'rules'}; + my $override_rec = $json->{'datarecord'}; + + if ( ( !defined $rules ) || ( $rules eq '' ) ) { $rules = 'override'; } + if ( ( !defined $override_rec ) ) { $override_rec = ''; } + if ( ( !defined $framework ) ) { $framework = ''; } + + my $biblio = Koha::Biblios->find($ref_biblionumber); + if ( not defined $biblio ) { + return $c->render( + status => 404, + json => { error => sprintf( "[%s] biblio to merge into not found", $ref_biblionumber ) } + ); + } + my $frombib = Koha::Biblios->find($bn_merge); + if ( not defined $frombib ) { + return $c->render( + status => 404, + json => { error => sprintf( "[%s] from which to merge not found", $bn_merge ) } + ); + } + + if ( ( $rules eq 'override_ext' ) && ( $override_rec eq '' ) ) { + return $c->render( + status => 404, + json => { + error => + "With the rule 'override_ext' you need to insert a bib record in marc-in-json format into 'record' field." + } + ); + } + + if ( ( $rules eq 'override' ) && ( $framework ne '' ) ) { + return $c->render( + status => 404, + json => { error => "With the rule 'override' you can not use the field 'framework_to_use'." } + ); + } + + my $results; + eval { + if ( $rules eq 'override_ext' ) { + my $record = MARC::Record::MiJ->new_from_mij_structure($override_rec); + $record->encoding('UTF-8'); + if ( $framework eq '' ) { $framework = $biblio->frameworkcode; } + my $chk = ModBiblio( $record, $ref_biblionumber, $framework ); + if ( $chk != 1 ) { die "Error on ModBiblio"; } # ModBiblio returns 1 if everything as gone well + my @biblio_ids_to_merge = ($bn_merge); + $results = $biblio->merge_with( \@biblio_ids_to_merge ); + } + if ( $rules eq 'override' ) { + my @biblio_ids_to_merge = ($bn_merge); + $results = $biblio->merge_with( \@biblio_ids_to_merge ); + } + }; + if ($@) { + return $c->render( status => 400, json => { error => $@ } ); + } else { + $c->respond_to( + mij => { + status => 200, + format => 'mij', + data => $biblio->metadata->record->to_mij + } + ); + } +} + 1; diff --git a/api/v1/swagger/definitions/merge_biblios.yaml b/api/v1/swagger/definitions/merge_biblios.yaml new file mode 100644 index 0000000000..ac4c6c8955 --- /dev/null +++ b/api/v1/swagger/definitions/merge_biblios.yaml @@ -0,0 +1,28 @@ +--- +type: object +properties: + biblio_id_to_merge: + type: integer + description: Biblionumber from which to merge + rules: + type: + - string + - "null" + description: Internally identifier of a merge algoritm. Now two identifier are supported, 'override' and 'override_ext'. + 'override' is to use when you the bibliographic data of biblio_id as resulting bibliographic data. The null value is equivalent + of 'override'. + 'override_ext' is to use only with a value in datarecord field. In fact is mandatory to use if you insert a record inside datarecord field. + framework_to_use: + type: + - string + - "null" + description: Framework code, you can use it only with a value in datarecord field. With null value it uses the framework + code of record to be merged into. + datarecord: + description: Bibliographic record used as result of the merge. It uses the format MARC-in-JSON + type: + - object + - "null" +additionalProperties: false +required: + - biblio_id_to_merge diff --git a/api/v1/swagger/paths/biblios_merge.yaml b/api/v1/swagger/paths/biblios_merge.yaml new file mode 100644 index 0000000000..45618a29c1 --- /dev/null +++ b/api/v1/swagger/paths/biblios_merge.yaml @@ -0,0 +1,49 @@ +"/biblios/{biblio_id}/merge": + post: + x-mojo-to: Biblios#merge + operationId: mergeBiblios + tags: + - merge_biblios + summary: Merge Biblios + parameters: + - name: biblio_id + in: path + description: Bilblionumber + required: true + type: string + - name: body + in: body + required: true + description: JSON Object with params and an optional marc record in MARC-in-JSON format + schema: + $ref: "../swagger.yaml#/definitions/merge_biblios" + consumes: + - application/json + produces: + - application/marc-in-json + responses: + '200': + description: The merge result as a biblio record + '404': + description: Biblio not found + schema: + "$ref": "../swagger.yaml#/definitions/error" + '401': + description: Authentication required + schema: + "$ref": "../swagger.yaml#/definitions/error" + '403': + description: Access forbidden + 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: + catalogue: "editcatalogue" diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml index 46966e40b2..dadeae92cb 100644 --- a/api/v1/swagger/swagger.yaml +++ b/api/v1/swagger/swagger.yaml @@ -118,6 +118,8 @@ definitions: $ref: ./definitions/job.yaml library: $ref: ./definitions/library.yaml + merge_biblios: + $ref: ./definitions/merge_biblios.yaml order: $ref: ./definitions/order.yaml patron: @@ -243,6 +245,8 @@ paths: $ref: "./paths/biblios_item_groups.yaml#/~1biblios~1{biblio_id}~1item_groups~1{item_group_id}~1items" "/biblios/{biblio_id}/item_groups/{item_group_id}/items/{item_id}": $ref: "./paths/biblios_item_groups.yaml#/~1biblios~1{biblio_id}~1item_groups~1{item_group_id}~1items~1{item_id}" + "/biblios/{biblio_id}/merge": + $ref: "./paths/biblios_merge.yaml#/~1biblios~1{biblio_id}~1merge" "/cash_registers/{cash_register_id}/cashups": $ref: "./paths/cash_registers.yaml#/~1cash_registers~1{cash_register_id}~1cashups" "/cashups/{cashup_id}": diff --git a/cataloguing/merge.pl b/cataloguing/merge.pl index 7fe218845e..083394f52d 100755 --- a/cataloguing/merge.pl +++ b/cataloguing/merge.pl @@ -86,33 +86,12 @@ if ($merge) { # Rewriting the leader my $biblio = Koha::Biblios->find($ref_biblionumber); $record->leader($biblio->metadata->record->leader()); - + #Take new framework code my $frameworkcode = $input->param('frameworkcode'); # Modifying the reference record ModBiblio($record, $ref_biblionumber, $frameworkcode); - # Moving items and article requests from the other record to the reference record - $biblio = $biblio->get_from_storage; - foreach my $biblionumber (@biblionumbers) { - my $from_biblio = Koha::Biblios->find($biblionumber); - $from_biblio->items->move_to_biblio($biblio); - $from_biblio->article_requests->update({ biblionumber => $ref_biblionumber }, { no_triggers => 1 }); - } - - my $sth_subscription = $dbh->prepare(" - UPDATE subscription SET biblionumber = ? WHERE biblionumber = ? - "); - my $sth_subscriptionhistory = $dbh->prepare(" - UPDATE subscriptionhistory SET biblionumber = ? WHERE biblionumber = ? - "); - my $sth_serial = $dbh->prepare(" - UPDATE serial SET biblionumber = ? WHERE biblionumber = ? - "); - my $sth_suggestions = $dbh->prepare(" - UPDATE suggestions SET biblionumber = ? WHERE biblionumber = ? - "); - my $report_header = {}; foreach my $biblionumber ($ref_biblionumber, @biblionumbers) { # build report @@ -148,42 +127,20 @@ if ($merge) { push @report_records, \%report_record; } - foreach my $biblionumber (@biblionumbers) { - # Moving subscriptions from the other record to the reference record - my $subcount = CountSubscriptionFromBiblionumber($biblionumber); - if ($subcount > 0) { - $sth_subscription->execute($ref_biblionumber, $biblionumber); - $sth_subscriptionhistory->execute($ref_biblionumber, $biblionumber); - } - - # Moving serials - $sth_serial->execute($ref_biblionumber, $biblionumber); - - # Moving suggestions - $sth_suggestions->execute($ref_biblionumber, $biblionumber); - - # Moving orders (orders linked to items of frombiblio have already been moved by move_to_biblio) - my @allorders = GetOrdersByBiblionumber($biblionumber); - foreach my $myorder (@allorders) { - $myorder->{'biblionumber'} = $ref_biblionumber; - ModOrder ($myorder); - # TODO : add error control (in ModOrder?) + my $rmerge; + eval { + my $newbiblio = Koha::Biblios->find($ref_biblionumber); + $rmerge = $newbiblio->merge_with( \@biblionumbers ); + }; + if ($@) { + push @errors, $@; } - # Deleting the other records - if (scalar(@errors) == 0) { - # Move holds - MergeHolds($dbh, $ref_biblionumber, $biblionumber); - my $error = DelBiblio($biblionumber); - push @errors, $error if ($error); - } -} - # Parameters $template->param( - result => 1, - report_records => \@report_records, - report_header => $report_header, + result => 1, + report_records => \@report_records, + report_header => $report_header, ref_biblionumber => scalar $input->param('ref_biblionumber') ); diff --git a/t/db_dependent/Koha/Biblio.t b/t/db_dependent/Koha/Biblio.t index 13be09f466..b338c86fee 100755 --- a/t/db_dependent/Koha/Biblio.t +++ b/t/db_dependent/Koha/Biblio.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 30; +use Test::More tests => 31; use Test::Exception; use Test::Warn; @@ -486,6 +486,42 @@ subtest 'to_api() tests' => sub { $schema->storage->txn_rollback; }; +subtest 'merge of records' => sub { + plan tests => 3; + $schema->storage->txn_begin; + + #Three biblio + my $biblio1 = $builder->build_sample_biblio( { title => 'Title number 1' } ); + my $biblio2 = $builder->build_sample_biblio( { title => 'Title number 2' } ); + my $biblio3 = $builder->build_sample_biblio( { title => 'Title number 3' } ); + + # One items each + my $item1_1 = $builder->build_sample_item( { biblionumber => $biblio1->biblionumber, barcode => 'bar11' } ); + my $item2_1 = $builder->build_sample_item( { biblionumber => $biblio2->biblionumber, barcode => 'bar22' } ); + my $item3_1 = $builder->build_sample_item( { biblionumber => $biblio3->biblionumber, barcode => 'bar33' } ); + my $results = ''; + my @to_merge = ( $biblio2->biblionumber, $biblio3->biblionumber ); + + my $pre_merged_rs = Koha::Biblios->search( + { biblionumber => [ $biblio1->biblionumber, $biblio2->biblionumber, $biblio3->biblionumber ] } ); + is( $pre_merged_rs->count, 3, '3 biblios exist' ); + + eval { $results = $biblio1->merge_with( \@to_merge ); }; + if ($@) { + is( 0, 1, "Not working. Error: " . $@ ); + } else { + my $items = $biblio1->items; + is( $items->count, 3, "After merge we have 3 items on first record" ); + + my $merged_rs = Koha::Biblios->search( + { biblionumber => [ $biblio1->biblionumber, $biblio2->biblionumber, $biblio3->biblionumber ] } ); + is( $merged_rs->count, 1, 'only 1 biblio left, the merged ones are gone' ); + } + + #End work + $schema->storage->txn_rollback; +}; + subtest 'suggestions() tests' => sub { plan tests => 3; diff --git a/t/db_dependent/api/v1/biblios.t b/t/db_dependent/api/v1/biblios.t index cb4f02bb1e..bad9a571fd 100755 --- a/t/db_dependent/api/v1/biblios.t +++ b/t/db_dependent/api/v1/biblios.t @@ -20,7 +20,7 @@ use Modern::Perl; use utf8; use Encode; -use Test::More tests => 13; +use Test::More tests => 14; use Test::MockModule; use Test::Mojo; use Test::Warn; @@ -1811,3 +1811,252 @@ subtest 'update_item() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'merge() tests' => sub { + plan tests => 12; + $schema->storage->txn_begin; + + my $mij_rec = q|{ + "fields": [ + { + "001": "2504398" + }, + { + "005": "20200421093816.0" + }, + { + "008": "920610s1993 caub s001 0 eng " + }, + { + "010": { + "ind1": " ", + "subfields": [ + { + "a": " 92021731 " + } + ], + "ind2": " " + } + }, + { + "020": { + "subfields": [ + { + "a": "05200784462 (Test mij)" + } + ], + "ind1": " ", + "ind2": " " + } + }, + { + "040": { + "subfields": [ + { + "a": "DLC" + }, + { + "c": "DLC" + }, + { + "d": "DLC" + } + ], + "ind2": " ", + "ind1": " " + } + }, + { + "041": { + "ind2": " ", + "subfields": [ + { + "a": "enggrc" + } + ], + "ind1": "0" + } + }, + { + "082": { + "subfields": [ + { + "a": "480" + }, + { + "2": "20" + } + ], + "ind2": "0", + "ind1": "0" + } + }, + { + "100": { + "ind2": " ", + "subfields": [ + { + "a": "Mastronarde, Donald J." + }, + { + "9": "389" + } + ], + "ind1": "1" + } + }, + { + "245": { + "ind1": "1", + "subfields": [ + { + "a": "Introduction to Attic Greek (Using mij) /" + }, + { + "c": "Donald J. Mastronarde." + } + ], + "ind2": "0" + } + }, + { + "260": { + "subfields": [ + { + "a": "Berkeley :" + }, + { + "b": "University of California Press," + }, + { + "c": "c1993." + } + ], + "ind2": " ", + "ind1": " " + } + }, + { + "300": { + "ind1": " ", + "subfields": [ + { + "a": "ix, 425 p. :" + }, + { + "b": "maps ;" + }, + { + "c": "26 cm." + } + ], + "ind2": " " + } + }, + { + "650": { + "subfields": [ + { + "a": "Attic Greek dialect" + }, + { + "9": "7" + } + ], + "ind2": "0", + "ind1": " " + } + }, + { + "942": { + "subfields": [ + { + "2": "ddc" + }, + { + "c": "BK" + } + ], + "ind2": " ", + "ind1": " " + } + }, + { + "955": { + "subfields": [ + { + "a": "pc05 to ea00 06-11-92; ea04 to SCD 06-11-92; fd11 06-11-92 (PA522.M...); fr21 06-12-92; fs62 06-15-92; CIP ver. pv07 11-12-93" + } + ], + "ind2": " ", + "ind1": " " + } + }, + { + "999": { + "subfields": [ + { + "c": "3" + }, + { + "d": "3" + } + ], + "ind1": " ", + "ind2": " " + } + } + ], + "leader": "01102pam a2200289 a 8500" + }|; + + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 0 } + } + ); + my $password = 'thePassword123'; + $patron->set_password( { password => $password, skip_validation => 1 } ); + my $userid = $patron->userid; + + my $title_1 = 'Title number 1'; + my $title_2 = 'Title number 2'; + my $biblio1 = $builder->build_sample_biblio( { title => $title_1 } ); + my $biblio2 = $builder->build_sample_biblio( { title => $title_2 } ); + my $biblio_id1 = $biblio1->biblionumber; + my $biblio_id2 = $biblio2->biblionumber; + my $json_input1 = '{ "biblio_id_to_merge": "' . $biblio_id2 . '" }'; + + $t->post_ok( "//$userid:$password@/api/v1/biblios/$biblio_id1/merge" => + { 'Content-Type' => 'application/json', 'Accept' => 'application/marc-in-json' } => $json_input1 ) + ->status_is( 403, 'Not enough permissions to merge two bib records' ); + + # Add permissions + $patron->flags(516)->store; + + $t->post_ok( + "//$userid:$password@/api/v1/biblios/$biblio_id1/merge" => { 'Content-Type' => 'application/weird+format' } => + $json_input1 )->status_is( 400, 'Not correct headers' ); + + my $result = + $t->post_ok( "//$userid:$password@/api/v1/biblios/$biblio_id1/merge" => + { 'Content-Type' => 'application/json', 'Accept' => 'application/marc-in-json' } => $json_input1 ) + ->status_is(200)->tx->res->body; + like( $result, qr/$title_1/, "Merged record has the correct title" ); + unlike( $result, qr/$title_2/, "Merged record doesn't have the wrong title" ); + + my $biblio3 = $builder->build_sample_biblio( { title => 'Title number 3' } ); + my $biblio_id3 = $biblio3->biblionumber; + my $json_input2 = '{ "biblio_id_to_merge": "' . $biblio_id3 . '", + "rules": "override_ext", + "datarecord": ' . $mij_rec . ' }'; + $result = + $t->post_ok( "//$userid:$password@/api/v1/biblios/$biblio_id1/merge" => + { 'Content-Type' => 'application/json', 'Accept' => 'application/marc-in-json' } => $json_input2 ) + ->status_is(200)->tx->res->body; + like( $result, qr/Using mij/, "Update with Marc-in-json record" ); + unlike( $result, qr/$title_1/, "Change all record with dat in the 'datarecord' field" ); + + $schema->storage->txn_rollback; + } -- 2.30.2