[Bug 35797] New: REST API: Add GET route for patronimage
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Bug ID: 35797 Summary: REST API: Add GET route for patronimage Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: shi-yao.wang@inLibro.com CC: tomascohen@gmail.com Add GET route to get patronimage database table data -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #1 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 160993 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160993&action=edit Bug 35797: REST API Add GET route for patronimage Api changes adding GET route for patronimage Test plan: 1- Apply the patch 2- Go to administration -> sys.pref. -> set patronimages to allow 3- Log in koha using a user with edit_borrowers privileges 4- Have a patron with a profile image (add an image if necessary by searching for a patron and then clicking on the default profile image on the top left) 5- Open a new browser tab, write {intranet_url}/api/v1/patrons/{borrowernumber}/image where {intranet_url} is the base intranet url of koha and {borrowernumber} is the borrowernumber of a borrower that has an image 6- Notice the response is a json with 3 keys: imagefile, mimetype and patron_id which corresponds to the fields of patronimage table in the database. Note that imagefile in db and in json response are different because it has been base64 encoded in order to be sent as a string in json 7- Do step 5 with a borrowernumber of a patron without an image and a borrowernumber of a patron that doesn't exist (and other edge cases where there should be an error message displayed such as "Access forbidden", "Authentication required", etc.) 8- Notice an appropriate error message is displayed -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #2 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- First time adding a REST API route. Please feel free to give advice and correction. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Biblibre Sandboxes <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160993|0 |1 is obsolete| | --- Comment #3 from Biblibre Sandboxes <sandboxes@biblibre.com> --- Created attachment 161273 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161273&action=edit Bug 35797: REST API Add GET route for patronimage Api changes adding GET route for patronimage Test plan: 1- Apply the patch 2- Go to administration -> sys.pref. -> set patronimages to allow 3- Log in koha using a user with edit_borrowers privileges 4- Have a patron with a profile image (add an image if necessary by searching for a patron and then clicking on the default profile image on the top left) 5- Open a new browser tab, write {intranet_url}/api/v1/patrons/{borrowernumber}/image where {intranet_url} is the base intranet url of koha and {borrowernumber} is the borrowernumber of a borrower that has an image 6- Notice the response is a json with 3 keys: imagefile, mimetype and patron_id which corresponds to the fields of patronimage table in the database. Note that imagefile in db and in json response are different because it has been base64 encoded in order to be sent as a string in json 7- Do step 5 with a borrowernumber of a patron without an image and a borrowernumber of a patron that doesn't exist (and other edge cases where there should be an error message displayed such as "Access forbidden", "Authentication required", etc.) 8- Notice an appropriate error message is displayed Signed-off-by: Martin AUBEUT <martin.aubeut@gmail.com> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Martin AUBEUT <martin.aubeut@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.aubeut@gmail.com Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Some important remarks: - For retrieving the image, you need to know the `patron_id` on the consumer code (i.e. the path is /patrons/{patron_id}/image), so why returning the patron_id attribute at all? - Why not return the raw/binary image so this is usable on a browser without extra processing on the client side? Look at the biblios endpoint for examples [1] - The MIME type should be returned as a header [2] - The code is not tidy [3] Not a real blocker: - Most of the endpoints we have follow a pattern: GET /objects POST /objects GET /objects/:object_id PUT /objects/:object_id DELETE /objects/:object_id In this particular case, there's no object_id because the patronimage table doesn't even have an auto-increment value. I would like you to think if there could be a use case for which having more than one image per patron could be desirable (in the future). If such was the case, we could do something like GET /patrons/:patron_id/default_image instead of the current route, envisioning a future addition of a patron_image_id and being able to add many images per patron. I'm not asking you to go implement such thing, but to think of a future-proof RESTful API design. [1] Here you can see we return raw binary MARC data https://git.koha-community.org/Koha-community/Koha/src/branch/master/Koha/RE... [2] https://git.koha-community.org/Koha-community/Koha/src/branch/master/Koha/RE... [3] There's a coding guideline for this, but basically follow this instructions to set your dev environment: https://wiki.koha-community.org/wiki/Perltidy -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #5 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 161610 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161610&action=edit Bug 35797: return the image directly instead of base64 in json Test plan: 1- Apply the patch 2- Go to administration -> sys.pref. -> set patronimages to allow 3- Log in koha using a user with edit_borrowers privileges 4- Have a patron with a profile image (add an image if necessary by searching for a patron and then clicking on the default profile image on the top left) 5- Open a new browser tab, write {intranet_url}/api/v1/patrons/{borrowernumber}/default_image where {intranet_url} is the base intranet url of koha and {borrowernumber} is the borrowernumber of a borrower that has an image 6- Notice the image displays in the browser 7- Do step 5 with a borrowernumber of a patron without an image and a borrowernumber of a patron that doesn't exist (and other edge cases where there should be an error message displayed such as "Access forbidden", "Authentication required", etc.) 8- Notice an appropriate error message is displayed -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #6 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- All remarks have been (hopefully well) implemented. I am not sure about a patron having multiple images since I currently cannot see any practical reason for such a thing, but I renamed the route just in case since it's trivial to do. One thing I thought I should comment on is that currently all images uploaded as patron image seem to be converted to png. So I hardcoded the return format for simplicity. Else (if say we preserve the image format), I think we would need some sort of mapping of koha db patronimage mimetype to mojolicious accepted format option (https://docs.mojolicious.org/Mojolicious/Types#DESCRIPTION) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |shi-yao.wang@inLibro.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com, | |philippe.blouin@inlibro.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161273|0 |1 is obsolete| | --- Comment #7 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 170137 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170137&action=edit Bug 35797: REST API Add GET route for patronimage Api changes adding GET route for patronimage Test plan: 1- Apply the patch 2- Run `npm run api:bundle` 2- Go to administration -> sys.pref. -> set patronimages to allow 3- Log in koha using a user with edit_borrowers privileges 4- Have a patron with a profile image (add an image if necessary by searching for a patron and then clicking on the default profile image on the top left) 5- Open a an API testing tool (such as postman), write {intranet_url}/api/v1/patrons/{borrowernumber}/default_image where {intranet_url} is the base intranet url of koha and {borrowernumber} is the borrowernumber of a borrower that has an image 6- Add OAuth 2.0 credentials coming from koha (patron -> more -> manage api keys) and generate a token 7- Send the request and notice the image of the patron is returned 8- Do step 5-7 with a borrowernumber of a patron without an image and a borrowernumber of a patron that doesn't exist (and other edge cases where there should be an error message displayed such as "Access forbidden", "Authentication required", etc.) 9- Notice an appropriate error message is displayed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161610|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- I'm curious about the permissions. I haven't checked but should "list_borrowers" be able to access this too? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #9 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- It would make sense yes. Didn't have that privilege in the past I believe. Adding it -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170137|0 |1 is obsolete| | --- Comment #10 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 170186 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170186&action=edit Bug 35797: REST API Add GET route for patronimage Api changes adding GET route for patronimage Test plan: 1- Apply the patch 2- Run `npm run api:bundle` 2- Go to administration -> sys.pref. -> set patronimages to allow 3- Log in koha using a user with "list_borrowers" or "edit_borrowers" privileges 4- Have a patron with a profile image (add an image if necessary by searching for a patron and then clicking on the default profile image on the top left) 5- Open a an API testing tool (such as postman), write {intranet_url}/api/v1/patrons/{borrowernumber}/default_image where {intranet_url} is the base intranet url of koha and {borrowernumber} is the borrowernumber of a borrower that has an image 6- Add OAuth 2.0 credentials coming from koha (patron -> more -> manage api keys) and generate a token 7- Send the request and notice the image of the patron is returned 8- Do step 5-7 with a borrowernumber of a patron without an image and a borrowernumber of a patron that doesn't exist (and other edge cases where there should be an error message displayed such as "Access forbidden", "Authentication required", etc.) 9- Notice an appropriate error message is displayed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170186|0 |1 is obsolete| | --- Comment #11 from David Nind <david@davidnind.com> --- Created attachment 172201 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=172201&action=edit Bug 35797: REST API Add GET route for patronimage Api changes adding GET route for patronimage Test plan: 1- Apply the patch 2- Run `npm run api:bundle` 2- Go to administration -> sys.pref. -> set patronimages to allow 3- Log in koha using a user with "list_borrowers" or "edit_borrowers" privileges 4- Have a patron with a profile image (add an image if necessary by searching for a patron and then clicking on the default profile image on the top left) 5- Open a an API testing tool (such as postman), write {intranet_url}/api/v1/patrons/{borrowernumber}/default_image where {intranet_url} is the base intranet url of koha and {borrowernumber} is the borrowernumber of a borrower that has an image 6- Add OAuth 2.0 credentials coming from koha (patron -> more -> manage api keys) and generate a token 7- Send the request and notice the image of the patron is returned 8- Do step 5-7 with a borrowernumber of a patron without an image and a borrowernumber of a patron that doesn't exist (and other edge cases where there should be an error message displayed such as "Access forbidden", "Authentication required", etc.) 9- Notice an appropriate error message is displayed Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@gmail.com --- Comment #12 from Jonathan Druart <jonathan.druart@gmail.com> --- Please provide tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 William Lavoie <william.lavoie@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 William Lavoie <william.lavoie@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #172201|0 |1 is obsolete| | --- Comment #13 from William Lavoie <william.lavoie@inLibro.com> --- Created attachment 176695 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176695&action=edit Bug 35797: REST API Add GET route for patronimage Api changes adding GET route for patronimage To run the tests, run 'prove t/db_dependent/api/v1/image.t' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 William Lavoie <william.lavoie@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |william.lavoie@inLibro.com Attachment #176695|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 William Lavoie <william.lavoie@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #172201|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #14 from William Lavoie <william.lavoie@inLibro.com> --- Created attachment 176696 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176696&action=edit Bug 35797: Add tests To run the tests, run 'prove t/db_dependent/api/v1/image.t' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #15 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to William Lavoie from comment #13)
Created attachment 176695 [details] [review] Bug 35797: REST API Add GET route for patronimage
Api changes adding GET route for patronimage To run the tests, run 'prove t/db_dependent/api/v1/image.t'
We lost the original test plan, and the `signed-off by` line -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #16 from William Lavoie <william.lavoie@inLibro.com> --- The test plan is the same as the one described above in comment 11, in addition to running 'prove t/db_dependent/api/v1/image.t'. Since the sign-off applied to the previous test plan, we need a new one. Test plan: 1- Apply the patch 2- Run `npm run api:bundle` 2- Go to administration -> sys.pref. -> set patronimages to allow 3- Log in koha using a user with "list_borrowers" or "edit_borrowers" privileges 4- Have a patron with a profile image (add an image if necessary by searching for a patron and then clicking on the default profile image on the top left) 5- Open a an API testing tool (such as postman), write {intranet_url}/api/v1/patrons/{borrowernumber}/default_image where {intranet_url} is the base intranet url of koha and {borrowernumber} is the borrowernumber of a borrower that has an image 6- Add OAuth 2.0 credentials coming from koha (patron -> more -> manage api keys) and generate a token 7- Send the request and notice the image of the patron is returned 8- Do step 5-7 with a borrowernumber of a patron without an image and a borrowernumber of a patron that doesn't exist (and other edge cases where there should be an error message displayed such as "Access forbidden", "Authentication required", etc.) 9- Notice an appropriate error message is displayed 10- Run 'prove t/db_dependent/api/v1/image.t' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|shi-yao.wang@inLibro.com |william.lavoie@inLibro.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|william.lavoie@inLibro.com |shi-yao.wang@inLibro.com CC|william.lavoie@inLibro.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Needs Signoff |Failed QA --- Comment #17 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Several problems from the QA tool here: [FAIL] t/db_dependent/api/v1/image.t FAIL file_permissions File must have the exec flag FAIL forbidden_patterns forbidden pattern: Use https:// instead of http:// for URLs (line 16) FAIL test_no_warnings 'use Test::NoWarnings' must be present in this test file. FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl t/db_dependent/api/v1/image.t` -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #172201|0 |1 is obsolete| | --- Comment #18 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 189432 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=189432&action=edit Bug 35797: REST API Add GET route for patronimage Api changes adding GET route for patronimage Test plan: 1- Apply the patch 2- Run `npm run api:bundle` 2- Go to administration -> sys.pref. -> set patronimages to allow 3- Log in koha using a user with "list_borrowers" or "edit_borrowers" privileges 4- Have a patron with a profile image (add an image if necessary by searching for a patron and then clicking on the default profile image on the top left) 5- Open a an API testing tool (such as postman), write {intranet_url}/api/v1/patrons/{borrowernumber}/default_image where {intranet_url} is the base intranet url of koha and {borrowernumber} is the borrowernumber of a borrower that has an image 6- Add OAuth 2.0 credentials coming from koha (patron -> more -> manage api keys) and generate a token 7- Send the request and notice the image of the patron is returned 8- Do step 5-7 with a borrowernumber of a patron without an image and a borrowernumber of a patron that doesn't exist (and other edge cases where there should be an error message displayed such as "Access forbidden", "Authentication required", etc.) 9- Notice an appropriate error message is displayed Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176696|0 |1 is obsolete| | --- Comment #19 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 189433 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=189433&action=edit Bug 35797: Add tests To run the tests, run 'prove t/db_dependent/api/v1/image.t' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #20 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Fixed QA issues mentionned -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #189432|0 |1 is obsolete| | --- Comment #21 from David Nind <david@davidnind.com> --- Created attachment 190727 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190727&action=edit Bug 35797: REST API Add GET route for patronimage Api changes adding GET route for patronimage Test plan: 1- Apply the patch 2- Run `npm run api:bundle` 2- Go to administration -> sys.pref. -> set patronimages to allow 3- Log in koha using a user with "list_borrowers" or "edit_borrowers" privileges 4- Have a patron with a profile image (add an image if necessary by searching for a patron and then clicking on the default profile image on the top left) 5- Open a an API testing tool (such as postman), write {intranet_url}/api/v1/patrons/{borrowernumber}/default_image where {intranet_url} is the base intranet url of koha and {borrowernumber} is the borrowernumber of a borrower that has an image 6- Add OAuth 2.0 credentials coming from koha (patron -> more -> manage api keys) and generate a token 7- Send the request and notice the image of the patron is returned 8- Do step 5-7 with a borrowernumber of a patron without an image and a borrowernumber of a patron that doesn't exist (and other edge cases where there should be an error message displayed such as "Access forbidden", "Authentication required", etc.) 9- Notice an appropriate error message is displayed Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #189433|0 |1 is obsolete| | --- Comment #22 from David Nind <david@davidnind.com> --- Created attachment 190728 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190728&action=edit Bug 35797: Add tests To run the tests, run 'prove t/db_dependent/api/v1/image.t' Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Text to go in the| |This enhancement adds a new release notes| |API route to request a | |patron image, if it exists. | |Errors are returned if a | |patron number doesn't exist | |or there is no patron | |image. | | | |Example API request: | |http://127.0.0.1:8081/api/v | |1/patrons/49/default_image | | | |Error messages: | |- If there is no patron | |image: Patron image not | |found. | |- If the patron doesn't | |exist: Patron not found. --- Comment #23 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Before applying the patch, check the API documentation at http://127.0.0.1:8080/api/v1/.html and search for getPatronImage - you shouldn't get any results. 2. Apply the patches 3. Rebuild the API and restart everything: . yarn build . restart_all 4. Repeat step 1, and now you should get some results. 5. I added an image to Mary Burton - borrowernumber = 49 6. Using Bruno API tool, with basic authentication (koha, koha), I made a GET request to GET http://127.0.0.1:8081/api/v1/patrons/49/default_image and an image was returned . 7. With an invalid patron - 999: GET http://127.0.0.1:8081/api/v1/patrons/999/default_image ==> Error message is: { "error": "Patron not found." } 8. With a patron without an image - 51: GET http://127.0.0.1:8081/api/v1/patrons/51/default_image ===> Error message is: { "error": "Patron image not found." } 9. I didn't test using OAuth 2.0 as I couldn't figure out how to use Brun for this, but I did work out part of it. 9.1 To generate a token to test OAuth 2.0 - Set RESTOAuth2ClientCredentials = enable - Generate the credentials for a patron: Patrons > [selected patron] > More > Manage API keys > Generate a new client id/secret pair 10. The tests pass: prove t/db_dependent/api/v1/image.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com QA Contact|tomascohen@gmail.com |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190727|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190728|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #24 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 198744 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198744&action=edit Bug 35797: REST API Add GET route for patronimage Api changes adding GET route for patronimage Test plan: 1- Apply the patch 2- Run `npm run api:bundle` 2- Go to administration -> sys.pref. -> set patronimages to allow 3- Log in koha using a user with "list_borrowers" or "edit_borrowers" privileges 4- Have a patron with a profile image (add an image if necessary by searching for a patron and then clicking on the default profile image on the top left) 5- Open a an API testing tool (such as postman), write {intranet_url}/api/v1/patrons/{borrowernumber}/default_image where {intranet_url} is the base intranet url of koha and {borrowernumber} is the borrowernumber of a borrower that has an image 6- Add OAuth 2.0 credentials coming from koha (patron -> more -> manage api keys) and generate a token 7- Send the request and notice the image of the patron is returned 8- Do step 5-7 with a borrowernumber of a patron without an image and a borrowernumber of a patron that doesn't exist (and other edge cases where there should be an error message displayed such as "Access forbidden", "Authentication required", etc.) 9- Notice an appropriate error message is displayed Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #25 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 198745 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198745&action=edit Bug 35797: Add tests To run the tests, run 'prove t/db_dependent/api/v1/image.t' Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #26 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 198746 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198746&action=edit Bug 35797: (QA follow-up) Tidy test and use https Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #27 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Someone please rename the test file inline? Something like patron_images.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #28 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 198749 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198749&action=edit Bug 35797: Rename test file from 'image.t' to 'patron_images.t' Patch from commit a086eff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|REST API: Add GET route for |Add GET route for patron |patronimage |images -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA CC| |pedro.amorim@openfifth.co.u | |k --- Comment #29 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Hi guys, failing QA: 1) Line 61 of patron_images.t is testing 'Patron not found' for the checkouts endpoint, not the new /default_image endpoint added here. 2) Unused import in V1::Patrons::Image: use MIME::Base64 qw( encode_base64 ); 3) The API controller should check for C4::Context->preference('patronimages') and return 403 if disabled. A library that deliberately disables patron images (e.g. for privacy reasons) would still expose them via the API. A test for this should also be included. 4) The tests are using a superlibrarian user and not testing the specific permissions the endpoint requires (list_borrowers/edit_borrowers) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@openfifth.c | |o.uk --- Comment #30 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Also, shouldn't the schema be more explicit about the reeturn format... 200: description: Patron image content: image/png: schema: type: string format: binary image/jpeg: schema: type: string format: binary Could be check mimetype rather than hard coding it with a comment.. I thought we spotted that mimetype in the same table -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198744|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198745|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198746|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198749|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #31 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 200889 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200889&action=edit Bug 35797: REST API Add GET route for patronimage Api changes adding GET route for patronimage Test plan: 1- Apply the patch 2- Run `npm run api:bundle` 2- Go to administration -> sys.pref. -> set patronimages to allow 3- Log in koha using a user with "list_borrowers" or "edit_borrowers" privileges 4- Have a patron with a profile image (add an image if necessary by searching for a patron and then clicking on the default profile image on the top left) 5- Open a an API testing tool (such as postman), write {intranet_url}/api/v1/patrons/{borrowernumber}/default_image where {intranet_url} is the base intranet url of koha and {borrowernumber} is the borrowernumber of a borrower that has an image 6- Add OAuth 2.0 credentials coming from koha (patron -> more -> manage api keys) and generate a token 7- Send the request and notice the image of the patron is returned 8- Do step 5-7 with a borrowernumber of a patron without an image and a borrowernumber of a patron that doesn't exist (and other edge cases where there should be an error message displayed such as "Access forbidden", "Authentication required", etc.) 9- Notice an appropriate error message is displayed Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #32 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 200890 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200890&action=edit Bug 35797: Add tests To run the tests, run 'prove t/db_dependent/api/v1/image.t' Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #33 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 200891 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200891&action=edit Bug 35797: (QA follow-up) Tidy test and use https Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #34 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 200892 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200892&action=edit Bug 35797: Rename test file from 'image.t' to 'patron_images.t' Patch from commit 7153f94 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #35 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 200893 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200893&action=edit Bug 35797: Syspref, schema path file and tests changes 1- Corrected tests according to the comments 2- Removed unused import 3- Added syspref check of 'patronimages' 4- Tweaked swagger path schema -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #36 from David Nind <david@davidnind.com> --- Failed QA as the tests don't pass: prove t/db_dependent/api/v1/patron_images.t t/db_dependent/api/v1/patron_images.t .. # Failed test '403 Forbidden' # at t/db_dependent/api/v1/patron_images.t line 73. # got: '404' # expected: '403' # Failed test 'exact match for JSON Pointer "/error"' # at t/db_dependent/api/v1/patron_images.t line 73. # got: 'Patron image not found.' # expected: 'Patron images are disabled.' # Looks like you failed 2 tests of 19. t/db_dependent/api/v1/patron_images.t .. 1/2 # Failed test 'Image tests' # at t/db_dependent/api/v1/patron_images.t line 122. # Looks like you failed 1 test of 2. t/db_dependent/api/v1/patron_images.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/2 subtests Test Summary Report ------------------- t/db_dependent/api/v1/patron_images.t (Wstat: 256 (exited 1) Tests: 2 Failed: 1) Failed test: 1 Non-zero exit status: 1 Files=1, Tests=2, 5 wallclock secs ( 0.02 usr 0.01 sys + 4.45 cusr 0.35 csys = 4.83 CPU) Result: FAIL Also, the qa script complains: [FAIL] Koha/REST/V1/Patrons/Image.pm FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl Koha/REST/V1/Patrons/Image.pm` [PASS] api/v1/swagger/paths/patrons_image.yaml [PASS] api/v1/swagger/swagger.yaml [FAIL] t/db_dependent/api/v1/image.t FAIL git_manipulation The file has been added and deleted or renamed in the same patchset SKIP new_line_at_the_end_of_file [FAIL] t/db_dependent/api/v1/patron_images.t FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl t/db_dependent/api/v1/patron_images.t` Testing notes (using KTD): 1. OAuth 2.0: 1.1 Enable RESTOAuth2ClientCredentials system preference 1.2 Go to a patron (for example, koha) > More > Manage API keys 1.3 Generate a new client id/key pair 1.4 Record the details 1.5 In API tool, such as Bruno, record for OAuth 2.0 authentication options: . Access Token URL: http://127.0.0.1:8080/api/v1/oauth/token . Client ID: from details recorded . Client Secret: from details recorded -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200893|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #37 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 200988 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200988&action=edit Bug 35797: Syspref, schema path file and tests changes 1- Corrected tests according to the comments 2- Removed unused import 3- Added syspref check of 'patronimages' 4- Tweaked swagger path schema -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #38 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- The tests are passing for me on ktd. Maybe try running `yarn api:bundle` before running the unit test? Fixed the tidy errors -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200889|0 |1 is obsolete| | Attachment #200890|0 |1 is obsolete| | Attachment #200891|0 |1 is obsolete| | Attachment #200892|0 |1 is obsolete| | Attachment #200988|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #39 from David Nind <david@davidnind.com> --- Created attachment 201006 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201006&action=edit Bug 35797: REST API Add GET route for patronimage Api changes adding GET route for patronimage Test plan: 1- Apply the patch 2- Run `npm run api:bundle` 2- Go to administration -> sys.pref. -> set patronimages to allow 3- Log in koha using a user with "list_borrowers" or "edit_borrowers" privileges 4- Have a patron with a profile image (add an image if necessary by searching for a patron and then clicking on the default profile image on the top left) 5- Open a an API testing tool (such as postman), write {intranet_url}/api/v1/patrons/{borrowernumber}/default_image where {intranet_url} is the base intranet url of koha and {borrowernumber} is the borrowernumber of a borrower that has an image 6- Add OAuth 2.0 credentials coming from koha (patron -> more -> manage api keys) and generate a token 7- Send the request and notice the image of the patron is returned 8- Do step 5-7 with a borrowernumber of a patron without an image and a borrowernumber of a patron that doesn't exist (and other edge cases where there should be an error message displayed such as "Access forbidden", "Authentication required", etc.) 9- Notice an appropriate error message is displayed Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #40 from David Nind <david@davidnind.com> --- Created attachment 201007 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201007&action=edit Bug 35797: Add tests To run the tests, run 'prove t/db_dependent/api/v1/image.t' Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #41 from David Nind <david@davidnind.com> --- Created attachment 201008 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201008&action=edit Bug 35797: (QA follow-up) Tidy test and use https Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #42 from David Nind <david@davidnind.com> --- Created attachment 201009 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201009&action=edit Bug 35797: Rename test file from 'image.t' to 'patron_images.t' Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #43 from David Nind <david@davidnind.com> --- Created attachment 201010 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201010&action=edit Bug 35797: Syspref, schema path file and tests changes 1- Corrected tests according to the comments 2- Removed unused import 3- Added syspref check of 'patronimages' 4- Tweaked swagger path schema Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35797 --- Comment #44 from David Nind <david@davidnind.com> --- (In reply to Shi Yao Wang from comment #38)
The tests are passing for me on ktd. Maybe try running `yarn api:bundle` before running the unit test?
Fixed the tidy errors
Thanks! prove t/db_dependent/api/v1/patron_images.t now passes (I'm sure I did the yarn build before). The qa script still complains, but I'll leave that up to the QA Team to resolve: [FAIL] t/db_dependent/api/v1/image.t FAIL git_manipulation The file has been added and deleted or renamed in the same patchset -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org