Bug 33036 - REST API: Merge biblio records
Summary: REST API: Merge biblio records
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Mark Hofstetter
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-02-22 10:49 UTC by Jan Kissig
Modified: 2024-01-03 14:54 UTC (History)
7 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
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 (2.59 KB, patch)
2023-06-16 16:33 UTC, Mark Hofstetter
Details | Diff | Splinter Review
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 (2.04 KB, patch)
2023-06-16 16:35 UTC, Mark Hofstetter
Details | Diff | Splinter Review
Bug 33036: REST API: Merge biblio records (3.86 KB, patch)
2023-06-16 16:39 UTC, Mark Hofstetter
Details | Diff | Splinter Review
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 (9.09 KB, patch)
2023-06-16 16:52 UTC, Mark Hofstetter
Details | Diff | Splinter Review
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 (8.14 KB, patch)
2023-06-16 16:57 UTC, Mark Hofstetter
Details | Diff | Splinter Review
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 (8.14 KB, patch)
2023-06-16 17:06 UTC, Mark Hofstetter
Details | Diff | Splinter Review
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 (8.19 KB, patch)
2023-06-19 11:54 UTC, Jan Kissig
Details | Diff | Splinter Review
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 (8.28 KB, patch)
2023-06-25 15:24 UTC, Mark Hofstetter
Details | Diff | Splinter Review
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 (8.33 KB, patch)
2023-06-26 11:28 UTC, Jan Kissig
Details | Diff | Splinter Review
A MARC21 record in MARCXML format (4.49 KB, application/xml)
2023-10-19 21:07 UTC, Zeno Tajoli
Details
patch to implement the feature (16.23 KB, patch)
2023-10-19 21:09 UTC, Zeno Tajoli
Details | Diff | Splinter Review
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 (16.22 KB, patch)
2023-10-24 11:29 UTC, Jan Kissig
Details | Diff | Splinter Review
An example of JSON file to use in testing (5.80 KB, application/json)
2023-11-15 20:56 UTC, Zeno Tajoli
Details
Patch that implemnts the end point (26.65 KB, patch)
2023-11-15 20:57 UTC, Zeno Tajoli
Details | Diff | Splinter Review
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 (26.30 KB, patch)
2023-11-17 17:59 UTC, Thomas Klausner
Details | Diff | Splinter Review
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 (26.70 KB, patch)
2023-11-19 19:27 UTC, Thomas Klausner
Details | Diff | Splinter Review
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 (26.70 KB, patch)
2023-11-22 11:57 UTC, Jan Kissig
Details | Diff | Splinter Review
An example of JSON file to use in testing (5.80 KB, application/json)
2023-11-24 12:43 UTC, Zeno Tajoli
Details
Bug 33036: Improve merge_with tests (3.17 KB, patch)
2023-12-14 10:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33036: Add more tests (9.17 KB, patch)
2023-12-14 10:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33036: Use Koha::Objects (7.00 KB, patch)
2023-12-14 10:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33036: Improve REST API controller (2.68 KB, patch)
2023-12-14 10:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Update of test number (783 bytes, patch)
2023-12-26 21:10 UTC, Zeno Tajoli
Details | Diff | Splinter Review
Bug 33036: REST API: Merge biblio records implements merging of records (26.93 KB, patch)
2024-01-03 14:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33036: Improve merge_with tests (3.24 KB, patch)
2024-01-03 14:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33036: Add more tests (9.24 KB, patch)
2024-01-03 14:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33036: Use Koha::Objects (7.08 KB, patch)
2024-01-03 14:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33036: Improve REST API controller (2.75 KB, patch)
2024-01-03 14:54 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Kissig 2023-02-22 10:49:01 UTC
Related to the ongoing development of biblios API, what about an endpoint to merge existing biblios including items and acquisition data as the intranet already offers.

Possible params should be
 - biblionumber_to_remain that remains
 - biblionumber_to_merge which will be merged into biblionumber_to_remain
 - optional MARCXML that represents the remaining biblio

If no MARCXML is delivered the merge should take the MARCXML from the remaining biblio as reference.
Comment 1 Mark Hofstetter 2023-06-13 15:46:09 UTC
I'd suggest the following url

PUT: /biblios/{biblio_id}/merge/{biblio_id_to_merge}

where biblio_id is the biblio_id of the record to be kept

additionally we need a list (json, csv ..) of the records that should be taken from the biblio_id_to_merge record

output should be a list from the "report_records" from the preference('MergeReportFields')

ideally the whole logic of "merge" from 
/cataloguing/merge.pl

starting with
86     # Rewriting the leader

should go into the storage class
package Koha::Biblio;
Comment 2 Jan Kissig 2023-06-14 07:57:37 UTC
By "additionally we need a list (json, csv ..) of the records that should be taken from the biblio_id_to_merge record" did you mean a list of fields from the biblio_id_to_merge record?
Comment 3 Mark Hofstetter 2023-06-16 16:33:43 UTC
Created attachment 152436 [details] [review]
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

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) execute API calls eg like
   PUT /biblios/{biblio_id}/merge/{biblio_id_to_merge}
   eg:
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/9"
6) the record with the id <biblio_id_to_merge> is deleted now, the record with <biblio_id>
   has all items, etc attached, return code is 200, with the message {"message":"Successfuly merged 9 into 262"}
7) optionally a full MARCXML record may be sent as body of the API call
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/2" -d @marcfile.xml
8) now also the content of the record with <biblio_id> is replaced with the content of the MARCXML file
9) Sign off.
10) Thx

Sponsored-by: Technische Hochschule Wildau
Co-authored-by: domm@plix.at
Comment 4 Mark Hofstetter 2023-06-16 16:35:29 UTC
Created attachment 152437 [details] [review]
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

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) execute API calls eg like
   PUT /biblios/{biblio_id}/merge/{biblio_id_to_merge}
   eg:
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/9"
6) the record with the id <biblio_id_to_merge> is deleted now, the record with <biblio_id>
   has all items, etc attached, return code is 200, with the message {"message":"Successfuly merged 9 into 262"}
7) optionally a full MARCXML record may be sent as body of the API call
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/2" -d @marcfile.xml
8) now also the content of the record with <biblio_id> is replaced with the content of the MARCXML file
9) Sign off.
10) Thx

Sponsored-by: Technische Hochschule Wildau
Co-authored-by: domm@plix.at
Comment 5 Mark Hofstetter 2023-06-16 16:39:07 UTC
Created attachment 152438 [details] [review]
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

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) execute API calls eg like
   PUT /biblios/{biblio_id}/merge/{biblio_id_to_merge}
   eg:
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/9"
6) the record with the id <biblio_id_to_merge> is deleted now, the record with <biblio_id>
   has all items, etc attached, return code is 200, with the message {"message":"Successfuly merged 9 into 262"}
7) optionally a full MARCXML record may be sent as body of the API call
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/2" -d @marcfile.xml
8) now also the content of the record with <biblio_id> is replaced with the content of the MARCXML file
9) Sign off.
10) Thx

Sponsored-by: Technische Hochschule Wildau
Co-authored-by: domm@plix.at
Comment 6 Mark Hofstetter 2023-06-16 16:52:15 UTC
Created attachment 152439 [details] [review]
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

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) execute API calls eg like
   PUT /biblios/{biblio_id}/merge/{biblio_id_to_merge}
   eg:
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/9"
6) the record with the id <biblio_id_to_merge> is deleted now, the record with <biblio_id>
   has all items, etc attached, return code is 200, with the message {"message":"Successfuly merged 9 into 262"}
7) optionally a full MARCXML record may be sent as body of the API call
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/2" -d @marcfile.xml
8) now also the content of the record with <biblio_id> is replaced with the content of the MARCXML file
9) Sign off.
10) Thx

Sponsored-by: Technische Hochschule Wildau
Co-authored-by: domm@plix.at
Comment 7 Mark Hofstetter 2023-06-16 16:57:52 UTC
Created attachment 152440 [details] [review]
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

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) execute API calls eg like
   PUT /biblios/{biblio_id}/merge/{biblio_id_to_merge}
   eg:
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/9"
6) the record with the id <biblio_id_to_merge> is deleted now, the record with <biblio_id>
   has all items, etc attached, return code is 200, with the message {"message":"Successfuly merged 9 into 262"}
7) optionally a full MARCXML record may be sent as body of the API call
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/2" -d @marcfile.xml
8) now also the content of the record with <biblio_id> is replaced with the content of the MARCXML file
9) Sign off.
10) Thx

Sponsored-by: Technische Hochschule Wildau
Co-authored-by: domm@plix.at
Comment 8 Mark Hofstetter 2023-06-16 17:06:27 UTC
Created attachment 152447 [details] [review]
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

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) execute API calls eg like
   PUT /biblios/{biblio_id}/merge/{biblio_id_to_merge}
   eg:
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/9"
6) the record with the id <biblio_id_to_merge> is deleted now, the record with <biblio_id>
   has all items, etc attached, return code is 200, with the message {"message":"Successfuly merged 9 into 262"}
7) optionally a full MARCXML record may be sent as body of the API call
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/2" -d @marcfile.xml
8) now also the content of the record with <biblio_id> is replaced with the content of the MARCXML file
9) Sign off.
10) Thx

Sponsored-by: Technische Hochschule Wildau
Co-authored-by: domm@plix.at
Comment 9 Jan Kissig 2023-06-19 11:54:57 UTC
Created attachment 152472 [details] [review]
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

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) execute API calls eg like
   PUT /biblios/{biblio_id}/merge/{biblio_id_to_merge}
   eg:
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/9"
6) the record with the id <biblio_id_to_merge> is deleted now, the record with <biblio_id>
   has all items, etc attached, return code is 200, with the message {"message":"Successfuly merged 9 into 262"}
7) optionally a full MARCXML record may be sent as body of the API call
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/2" -d @marcfile.xml
8) now also the content of the record with <biblio_id> is replaced with the content of the MARCXML file
9) Sign off.
10) Thx

Sponsored-by: Technische Hochschule Wildau
Co-authored-by: domm@plix.at
Signed-off-by: Jan Kissig <jkissig@th-wildau.de>
Comment 10 Jan Kissig 2023-06-19 11:57:25 UTC
When using Postman instead of curl to send HTTP-request (body:raw, type:xml) with a body like this:

<?xml version="1.0" encoding="UTF-8"?>
<record
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd"
    xmlns="http://www.loc.gov/MARC21/slim">
  <leader>00224nam a22001097a 4500</leader>
  <controlfield tag="003">OSt</controlfield>
  <controlfield tag="005">20230412064312.0</controlfield>
  <controlfield tag="008">230412b        |||||||| |||| 00| 0 eng d</controlfield>
  <datafield tag="040" ind1=" " ind2=" ">
    <subfield code="c">Ost</subfield>
  </datafield>
  <datafield tag="245" ind1=" " ind2=" ">
    <subfield code="a">NewTitle</subfield>
  </datafield>
</record>

f.e. PUT http://localhost:8081/api/v1/biblios/262/merge/6 I get the error

{
    "errors": [
        {
            "message": "Expected object - got string.",
            "path": "/body"
        }
    ],
    "status": 400
}

The proposed way just works fine:
 7) optionally a full MARCXML record may be sent as body of the API call
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/2" -d @marcfile.xml
Comment 11 Mark Hofstetter 2023-06-25 15:24:29 UTC
Created attachment 152659 [details] [review]
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

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) execute API calls eg like
   PUT /biblios/{biblio_id}/merge/{biblio_id_to_merge}
   eg:
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/9"
6) the record with the id <biblio_id_to_merge> is deleted now, the record with <biblio_id>
   has all items, etc attached, return code is 200, with the message {"message":"Successfuly merged 9 into 262"}
7) optionally a full MARCXML record may be sent as body of the API call
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/2" -d @marcfile.xml
8) now also the content of the record with <biblio_id> is replaced with the content of the MARCXML file
9) Sign off.
10) Thx

Sponsored-by: Technische Hochschule Wildau
Co-authored-by: domm@plix.at
Comment 12 Jan Kissig 2023-06-26 11:28:45 UTC
Created attachment 152678 [details] [review]
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

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) execute API calls eg like
   PUT /biblios/{biblio_id}/merge/{biblio_id_to_merge}
   eg:
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/9"
6) the record with the id <biblio_id_to_merge> is deleted now, the record with <biblio_id>
   has all items, etc attached, return code is 200, with the message {"message":"Successfuly merged 9 into 262"}
7) optionally a full MARCXML record may be sent as body of the API call
   curl  -s -u koha:koha -X PUT  "http://127.0.0.1:8081/api/v1/biblios/262/merge/2" -d @marcfile.xml
8) now also the content of the record with <biblio_id> is replaced with the content of the MARCXML file
9) Sign off.
10) Thx

Sponsored-by: Technische Hochschule Wildau
Co-authored-by: domm@plix.at
Signed-off-by: Jan Kissig <jkissig@th-wildau.de>
Comment 13 Jonathan Druart 2023-08-31 13:59:38 UTC
"From: Tomas Cohen Arazi <tomascohen@theke.io>"

This does not seem correct.
Comment 14 Jonathan Druart 2023-08-31 14:02:44 UTC
1. Tests are missing

2. You are duplicating code. The correct way to do this is to first move the code to a module, provide test, then call that method from merge.pl and your Mojo controller.
Comment 15 Jonathan Druart 2023-08-31 14:15:28 UTC
Also don't forget to run the QA script:

 FAIL   Koha/REST/V1/Biblios.pm
   FAIL   forbidden patterns
                forbidden pattern: Use of $c->validation in API controllers should be avoided (bug 33556) (line 876)
                forbidden pattern: Use of $c->validation in API controllers should be avoided (bug 33556) (line 877)
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 138, now: 175)
Comment 16 Zeno Tajoli 2023-10-05 22:21:21 UTC
Hi Jonathan, I'm trying to develop this patch so I ask same explanation about your requests:
1)'first move the code to a module'.
In this patch the new verb 'merge' is inside an existing module, 'Koha/REST/V1/Biblios.pm' and it is also a Mojo controller.
So the code is in the correct place.

2)'provide test'
OK, to do.

3)'then call that method from merge.pl and your Mojo controller'
Yes, I need to change the merge.pl and use the REST API instead of the present code.

Is it a correct work plan?
Comment 17 Jonathan Druart 2023-10-12 13:17:42 UTC
(In reply to Zeno Tajoli from comment #16)
> Hi Jonathan, I'm trying to develop this patch so I ask same explanation
> about your requests:
> 1)'first move the code to a module'.
> In this patch the new verb 'merge' is inside an existing module,
> 'Koha/REST/V1/Biblios.pm' and it is also a Mojo controller.
> So the code is in the correct place.

To make it reusable from another place than the REST API.
It needs to be placed into Koha::Biblio (see Koha::Patron::merge_with)

> 3)'then call that method from merge.pl and your Mojo controller'
> Yes, I need to change the merge.pl and use the REST API instead of the
> present code.

No, you need to call Koha::Biblio::merge_with from merge.pl and the REST API controller.
Comment 18 Zeno Tajoli 2023-10-19 21:07:27 UTC
Created attachment 157481 [details]
A MARC21 record in MARCXML format

You can use MARCXML file for tests
Comment 19 Zeno Tajoli 2023-10-19 21:09:20 UTC
Created attachment 157482 [details] [review]
patch to implement the feature
Comment 20 Zeno Tajoli 2023-10-19 21:13:57 UTC
This is a rewrite of the feature with the use of a module, tests, and perltidy.
Now merge.pl and the REST API controller use Koha::Biblio::merge_with.

Test plan:
    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) execute API calls eg like
       PUT /biblios/{biblio_id}/merge/{biblio_id_to_merge}
       eg:
       curl  -s -u koha:koha -X POST  "http://127.0.0.1:8081/api/v1/biblios/262/merge/9"
    6) the record with the id <biblio_id_to_merge> is deleted now, the record with <biblio_id>
       has all items, etc attached, return code is 200, with the message {"message":"Successfuly merged 9 into 262"}
    7) optionally a full MARCXML record may be sent as body of the API call
       curl  -s -u koha:koha -X POST  "http://127.0.0.1:8081/api/v1/biblios/262/merge/2" -d @marcfile.xml
    8) now also the content of the record with <biblio_id> is replaced with the content of the MARCXML file
    9) Go into intramet and do a search. Select two or (better) more record.
    10) Merge them; merge must be a success.
    11) Test with prove -v t/db_dependent/Koha/Biblio.t
Comment 21 Jan Kissig 2023-10-24 11:29:26 UTC
Created attachment 157731 [details] [review]
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

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) execute API calls eg like
       POST /biblios/{biblio_id}/merge/{biblio_id_to_merge}
       eg:
       curl  -s -u koha:koha -X POST  "http://127.0.0.1:8081/api/v1/biblios/262/merge/9"
    6) the record with the id <biblio_id_to_merge> is deleted now, the record with <biblio_id>
       has all items, etc attached, return code is 200, with the message {"message":"Successfuly merged 9 into 262"}
    7) optionally a full MARCXML record may be sent as body of the API call
       curl  -s -u koha:koha -X POST  "http://127.0.0.1:8081/api/v1/biblios/262/merge/2" -d @marcfile.xml
    8) now also the content of the record with <biblio_id> is replaced with the content of the MARCXML file
    9) Go into intramet and do a search. Select two or (better) more record.
    10) Merge them; merge must be a success.
    11) Test with prove -v t/db_dependent/Koha/Biblio.t

To do test with curl you canuse the MARCXML (MARC21) file attached in bugzilla.

Sponsored-by: Technische Hochschule Wildau
Co-authored-by: domm@plix.at
Signed-off-by: Jan Kissig <jkissig@th-wildau.de>
Comment 22 Jan Kissig 2023-10-24 12:37:31 UTC
Tested merge with
- attached holds
- attached items
- attached subscription
- attached order
- POSTing XML
Comment 23 Jonathan Druart 2023-10-24 12:52:09 UTC
Hi Zeno,

Some QA feedbacks:

1. Please tidy new code (using perltidy https://wiki.koha-community.org/wiki/Perltidy)

2. Those lines are non needed and must be removed:

+use C4::Serials qw( CountSubscriptionFromBiblionumber ); (duplicate)

+    #my $biblio = Koha::Biblios->find($self->biblionumber);

+    #say Dumper $results;

3. New REST API endpoint needs to be covered by tests

4.
+      - $ref: "../swagger.yaml#/parameters/marc_schema_header"
=> You are not using it (x-record-schema) in the controller code

5. The error handling is wrong:
  a. You should raise an exception instead of returning
  b. You should run the whole merge code in a transaction

6. Not sure about the response of the endpoint, IMO it should return the biblio generated by the merge.
Also the response contains "merged" and "kept" with the IDs we passed, it does not seem needed.

Note that Tomas suggested on the list:
POST /biblios/123/merges
{
    "biblio_id": 456,
    "rules": ?
}

We will need his feedback here.
Comment 24 Tomás Cohen Arazi 2023-10-24 12:57:47 UTC
(In reply to Jonathan Druart from comment #23)
> 
> 6. Not sure about the response of the endpoint, IMO it should return the
> biblio generated by the merge.
> Also the response contains "merged" and "kept" with the IDs we passed, it
> does not seem needed.

I'd say it needs to return the resulting object, with the Location header pointing to the /biblios/:biblio_id endpoint (instead of the /merge)

> Note that Tomas suggested on the list:
> POST /biblios/123/merges
> {
>     "biblio_id": 456,
>     "rules": ?
> }

Yes, we certainly need a way to specify what to pick from which record... An initial implementation could just implement a full overwrite (i.e. effectively moving the items and removing the biblio) but it needs to allow, by design, for future enhancements like this, so it is more useful.
Comment 25 Zeno Tajoli 2023-10-27 11:16:30 UTC
Hi Jonathan Druart,

are you sure that there are problems with tidying for this patch?
I use perltidy conf that are in the git archive and I do a check with 
koha-qa.pl -v 2 -c 1 and I pass the test.
Comment 26 Jonathan Druart 2023-10-27 12:07:19 UTC
(In reply to Zeno Tajoli from comment #25)
> Hi Jonathan Druart,
> 
> are you sure that there are problems with tidying for this patch?
> I use perltidy conf that are in the git archive and I do a check with 
> koha-qa.pl -v 2 -c 1 and I pass the test.

+    if($@)
+    {   push @errors, $@;
+    }   else
+    {   if ($rmerge->{'status'}==0)
+        {   push @errors, $rmerge->{'error'};
         }

This looks wrong, we have 

  8 # } else {
  9 --cuddled-else

in our .perltidyrc (at the root of Koha src)
Comment 27 Katrin Fischer 2023-10-27 18:30:59 UTC
See here too: https://wiki.koha-community.org/wiki/Perltidy
Comment 28 Zeno Tajoli 2023-11-03 07:55:28 UTC
Hi Thomas,
could you write here a full example with curl on how do you want the call of the API ?
I need parmas of curl and headers, json file fields and how to pass the MARC21/UNIMARC data (a field of the json file?).
To undestand better how to do the work.
Comment 29 Zeno Tajoli 2023-11-15 20:56:08 UTC
Created attachment 159011 [details]
An example of JSON file to use in testing
Comment 30 Zeno Tajoli 2023-11-15 20:57:45 UTC
Created attachment 159012 [details] [review]
Patch that implemnts the end point
Comment 31 Zeno Tajoli 2023-11-15 21:05:39 UTC
A new version that follow the requests given.

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
Comment 32 Thomas Klausner 2023-11-17 17:59:16 UTC
Created attachment 159103 [details] [review]
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)

The QA script complains about files being less tidy, but if we add the perltidy changes to this commit I'm afraid the actual change will be much less clear to review. So I did not run perltidy on the changed files.

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 <ztajoli@gmail.com>
Co-authored-by: Thomas Klausner <domm@plix.at>
Co-authored-by: Mark Hofstetter <<mark@hofstetter.at>>
Signed-off-by: Jan Kissig <jkissig@th-wildau.de>
Comment 33 Thomas Klausner 2023-11-19 19:27:54 UTC
Created attachment 159120 [details] [review]
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 <ztajoli@gmail.com>
Co-authored-by: Thomas Klausner <domm@plix.at>
Co-authored-by: Mark Hofstetter <<mark@hofstetter.at>>
Signed-off-by: Jan Kissig <jkissig@th-wildau.de>
Comment 34 Jan Kissig 2023-11-22 11:57:29 UTC
Created attachment 159191 [details] [review]
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 <ztajoli@gmail.com>
Co-authored-by: Thomas Klausner <domm@plix.at>
Co-authored-by: Mark Hofstetter <<mark@hofstetter.at>>
Signed-off-by: Jan Kissig <jkissig@th-wildau.de>
Comment 35 Zeno Tajoli 2023-11-24 12:41:30 UTC
Sign off done by Jan Kissig as I see.
Comment 36 Zeno Tajoli 2023-11-24 12:43:48 UTC
Created attachment 159239 [details]
An example of JSON file to use in testing
Comment 37 Jonathan Druart 2023-12-14 10:29:03 UTC
Created attachment 159826 [details] [review]
Bug 33036: Improve merge_with tests
Comment 38 Jonathan Druart 2023-12-14 10:29:06 UTC
Created attachment 159827 [details] [review]
Bug 33036: Add more tests
Comment 39 Jonathan Druart 2023-12-14 10:29:09 UTC
Created attachment 159828 [details] [review]
Bug 33036: Use Koha::Objects

Better to use Koha::Objects everywhere and have the same pattern for the
different related objects.
Comment 40 Jonathan Druart 2023-12-14 10:29:12 UTC
Created attachment 159829 [details] [review]
Bug 33036: Improve REST API controller
Comment 41 Jonathan Druart 2023-12-14 10:29:24 UTC
1. Added some improvements and missing unit tests

2. Wondering if we should not support more than json for the marc record. Not considering blocker however (can be done later if needed).

3. Error handling is not the best, but it was bad already (ModBiblio and DelBiblio erk!) but we improved it.

IMO This can go now, but I would like a last round of test (+ review) from Mark, domm or Zeno on my follow-ups.
Comment 42 Mark Hofstetter 2023-12-18 08:49:32 UTC
looks ok to me

for testing/using please don't forget to activate RESTBasicAuth

/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RESTBasicAuth

cheers and thx to all
Comment 43 Zeno Tajoli 2023-12-26 21:10:12 UTC
Created attachment 160302 [details] [review]
Update of test number
Comment 44 Zeno Tajoli 2023-12-26 21:22:58 UTC
I check the patches of Jonathan and for me are OK.

I add a new little patch because there is a new test inside t/db_dependent/API/v1/biblios.t (bug 35574).

Now there is only JSON for inputting a MARC record because inside JSON I also insert other parameters ["biblio_id_to_merge", rules", "framework_to_use"]

I don't know how to pass them without using JSON.
Comment 45 Jonathan Druart 2024-01-03 14:54:23 UTC
Created attachment 160468 [details] [review]
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 <ztajoli@gmail.com>
Co-authored-by: Thomas Klausner <domm@plix.at>
Co-authored-by: Mark Hofstetter <<mark@hofstetter.at>>
Signed-off-by: Jan Kissig <jkissig@th-wildau.de>

Bug 33036: Update of test number.

File ../biblios.t was update with a new subutest.
So we need this update to have a 'OK' after test running.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 46 Jonathan Druart 2024-01-03 14:54:26 UTC
Created attachment 160469 [details] [review]
Bug 33036: Improve merge_with tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 47 Jonathan Druart 2024-01-03 14:54:29 UTC
Created attachment 160470 [details] [review]
Bug 33036: Add more tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 48 Jonathan Druart 2024-01-03 14:54:32 UTC
Created attachment 160471 [details] [review]
Bug 33036: Use Koha::Objects

Better to use Koha::Objects everywhere and have the same pattern for the
different related objects.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 49 Jonathan Druart 2024-01-03 14:54:35 UTC
Created attachment 160472 [details] [review]
Bug 33036: Improve REST API controller

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>