[Bug 13738] New: Add RESTful Borrower service
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 Bug ID: 13738 Summary: Add RESTful Borrower service Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org We should have a basic RESTful CRUD service for Koha patrons. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |13737 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |12272 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 36081 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36081&action=edit [WIP] Bug 13738 - Add RESTful Borrower service -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- My first inclination is to remove the ability to fetch a borrower by multiple id types. Instead, I think we should force it to be my borrowernumber only, and if you don't have the borrowernumber, you can find it by searching ( not yet implemented ) on userid or cardnumber. Any thoughts? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=13607 CC| |jonathan.druart@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 36081 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36081 [WIP] Bug 13738 - Add RESTful Borrower service Review of attachment 36081: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13738&attachment=36081) ----------------------------------------------------------------- Generally I'm really happy with this, it's a great starting point for us to expand upon. Nice work Kyle! ::: Koha/Service/Borrower.pm @@ +39,5 @@
+ authnotrequired => 0, + needed_flags => { borrowers => 1 }, + routes => [ + [ qr'GET /(\d+)', 'get_borrower' ], + [ qr'POST /(\d+)', 'add_borrower' ],
Untested, but I beleive the '/(\d+) on this line is superflous and may infact mean the POST method is unreachable if my memory serves my correctly. @@ +88,5 @@
+=head2 POST + + Syntax: /rest/borrower + + Creates a new patron. Data must be supplied as a POST of JSON data.
JSON data, I'm loving JSON and fully support it's use.. however, for the borrower service I tihnk we should attempt to support XML aswell via content negotiation. I can see this route being used internally in Koha allot, and also being used for institutions to manage their borrowers externally.. Many said institutions are still stuck in the dark ages of XML ;) @@ +104,5 @@
+ my $data = from_json( $json, { utf8 => 1 } ); + + my $borrower = Koha::Borrower->new()->set($data)->store(); + + $self->output( $borrower->as_hashref() );
Great start, but we should probably end up with some error handling here so we can return a meaningful json error body if the insert isn't allowed for some reason or another. @@ +153,5 @@
+ + my $borrower = Koha::Borrowers->find( { get_id_field() => $id } ); + + if ($borrower) { + $borrower->delete(); #TODO Use C4::Members to delete
Related to the #TODO: Personally, I think I'd rather see the delete functionality from C4::Members rolled into the $borrower object, an eventually for scripts to all strt using the object as the defactly point of truth for this sort of thing. I would be nice to start the move into the Koha with a clean sweep of these sorts of functions :). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Remind me, which bug adds Koha::Borrower and Koha::Borrowers.. it needs to be a dependancy ;) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- About the delete - this will probably create a problem with lists if we don't use the internal functions. We do a bit more there and not all is caught by FK constraints in the database (actually there are some bugs for stuff that we should catch, but don't). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |13019 --- Comment #6 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Martin Renvoize from comment #4)
Remind me, which bug adds Koha::Borrower and Koha::Borrowers.. it needs to be a dependancy ;)
It's bug 13019 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Katrin Fischer from comment #5)
About the delete - this will probably create a problem with lists if we don't use the internal functions. We do a bit more there and not all is caught by FK constraints in the database (actually there are some bugs for stuff that we should catch, but don't).
That's sort of the point of my comment cait.. I'd like to see the Objects moving towards being a single piont fo truth.. so i'd rather see the logic around deleting, creating, updateing and even reading a borrower moved from C4::Members into Koha::Borrowers... whether this starts as the Koha::Borrower object referencing the C4::Members routine, or writing it properly I don't really mind.. but in new code I'd certainly like to strive for 'doing it right' the first time and using only the Koha::* stuff in the Koha::Services unless absolutely nesessary. Just my thoughts -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Sounds all good to me :) Just wanted to point out that actually we *must* do it differently, because else it will be buggy. Also we should have one central place to check for fines, checkouts, existing guarantor etc. - have to get that right. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13738 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ere.maijala@helsinki.fi -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org