Bug 36613

Summary: Tidy merge biblio feature
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: REST APIAssignee: Bugs List <koha-bugs>
Status: NEW --- QA Contact:
Severity: normal    
Priority: P5 - low CC: lucas, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 33036    
Bug Blocks:    

Description Tomás Cohen Arazi (tcohen) 2024-04-16 18:26:52 UTC
Bug 33036 introduced a feature for merging records using the API. But some of the return statuses are wrong:


```perl
    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'." }
        );
    }
```

Both should be '400 - Bad request'
Comment 1 Tomás Cohen Arazi (tcohen) 2024-04-18 12:51:57 UTC
There are also a few things to consider:

* Creating a 'merge' resource should return a 201.

* The use of `respond_to` doesn't look correct. You either always return mij, or have some fallback mechanism. I tried tweaking the tests so they pass application/marc and even though the operation succeeded, I got an empty reply and a 204 instead of a 200!:

```
    #   Failed test '200 OK'
    #   at t/db_dependent/api/v1/biblios.t line 2155.
    #          got: '204'
    #     expected: '200'

    #   Failed test 'Update with Marc-in-json record'
    #   at t/db_dependent/api/v1/biblios.t line 2159.
    #                   ''
    #     doesn't match '(?^u:Using mij)'
    # Looks like you failed 2 tests of 12.
```

so not a failure, or an explosion, just a weird behavior!

* the `catch` block is returning a 400 instead of a 500 (unhandled server error). I assume there were reasons to return a 400. I think they should be individually handled properly, and then fallback to 500.