https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33036
Jonathan Druart <jonathan.druart+k...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|Signed Off |Failed QA
--- Comment #23 from Jonathan Druart <jonathan.druart+k...@gmail.com> ---
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.
--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
