https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |new feature Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |lari.taskula@hypernova.fi |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=23998 --- Comment #1 from Lari Taskula <lari.taskula@hypernova.fi> --- Created attachment 95304 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95304&action=edit Bug 23998: REST API for messages - Add /api/v1/messages endpoint Exposes messages database table via REST API. GET /api/v1/messages POST /api/v1/messages PUT /api/v1/messages DELETE /api/v1/messages To test: 1. prove t/db_dependent/api/v1/messages.t https://bugs.koha-community.org/show_bug.cgi?id=23988 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95305|0 |1 is obsolete| | --- Comment #3 from Lari Taskula <lari.taskula@hypernova.fi> --- Created attachment 95310 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95310&action=edit Bug 23998: REST API for messages - Add /api/v1/messages endpoint Exposes messages database table via REST API. GET /api/v1/messages POST /api/v1/messages PUT /api/v1/messages DELETE /api/v1/messages To test: 1. prove t/db_dependent/api/v1/messages.t Sponsored-by: Koha-Suomi Oy -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |david@davidnind.com --- Comment #4 from David Nind <david@davidnind.com> --- Sorry, tests fail 8-(: root@a9bc3171985d:koha(bz23998)$ prove t/db_dependent/api/v1/messages.t t/db_dependent/api/v1/messages.t .. 2/5 # Failed test 'SWAGGER3.2.1' # at t/db_dependent/api/v1/messages.t line 227. # got: '500' # expected: '201' # Failed test 'exact match for JSON Pointer "/patron_id"' # at t/db_dependent/api/v1/messages.t line 227. # got: undef # expected: '95' # Failed test 'exact match for JSON Pointer "/library_id"' # at t/db_dependent/api/v1/messages.t line 227. # got: undef # expected: 'kayx1Yh5' # Failed test 'exact match for JSON Pointer "/message_type"' # at t/db_dependent/api/v1/messages.t line 227. # got: undef # expected: 'B' # Failed test 'exact match for JSON Pointer "/message"' # at t/db_dependent/api/v1/messages.t line 227. # got: undef # expected: 'Old Fox jumped over Cheeseboy' # Failed test 'exact match for JSON Pointer "/manager_id"' # at t/db_dependent/api/v1/messages.t line 227. # got: undef # expected: '94' # Failed test 'SWAGGER3.2.1' # at t/db_dependent/api/v1/messages.t line 243. # got: '500' # expected: '201' # Failed test 'exact match for JSON Pointer "/patron_id"' # at t/db_dependent/api/v1/messages.t line 243. # got: undef # expected: '95' # Failed test 'exact match for JSON Pointer "/library_id"' # at t/db_dependent/api/v1/messages.t line 243. # got: undef # expected: 'kayx1Yh5' # Failed test 'exact match for JSON Pointer "/message_type"' # at t/db_dependent/api/v1/messages.t line 243. # got: undef # expected: 'B' # Failed test 'exact match for JSON Pointer "/message"' # at t/db_dependent/api/v1/messages.t line 243. # got: undef # expected: 'Old Fox jumped over Cheeseboy' # Failed test 'exact match for JSON Pointer "/manager_id"' # at t/db_dependent/api/v1/messages.t line 243. # got: undef # expected: '95' # Failed test 'exact match for JSON Pointer "/errors"' # at t/db_dependent/api/v1/messages.t line 253. # Structures begin differing at: # $got->[1] = HASH(0x557190092810) # $expected->[1] = Does not exist # Looks like you failed 13 tests of 27. # Failed test 'add() tests' # at t/db_dependent/api/v1/messages.t line 265. t/db_dependent/api/v1/messages.t .. 5/5 # Looks like you failed 1 test of 5. t/db_dependent/api/v1/messages.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/5 subtests Test Summary Report ------------------- t/db_dependent/api/v1/messages.t (Wstat: 256 Tests: 5 Failed: 1) Failed test: 3 Non-zero exit status: 1 Files=1, Tests=5, 11 wallclock secs ( 0.02 usr 0.02 sys + 10.38 cusr 1.24 csys = 11.66 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95310|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=23998 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com, | |philippe.blouin@inlibro.com | |, shi-yao.wang@inLibro.com Status|Failed QA |Needs Signoff Assignee|lari.taskula@hypernova.fi |shi-yao.wang@inLibro.com --- Comment #6 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Remade the patch since last patch was 6-7 years ago -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198316|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=23998 --- Comment #9 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Corrected tests and formatting. Ran `qa` on ktd and all steps passed -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198364|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=23998 --- Comment #11 from David Nind <david@davidnind.com> --- (In reply to Shi Yao Wang from comment #9)

Corrected tests and formatting. Ran `qa` on ktd and all steps passed

Thanks! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198365|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=23998 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off --- Comment #13 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Rebased -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #14 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- I've got a test failure: * Proving /kohadevbox/koha/t/db_dependent/api/v1/messages.t FAIL # Failed test at /kohadevbox/koha/t/db_dependent/api/v1/messages.t line 253. # found warning: Duplicate ID at /kohadevbox/koha/Koha/Object.pm line 176. # expected to find warning: (?^u:DBD::mysql::st execute failed: Duplicate entry '(.*?)' for key 'PRIMARY') # Looks like you failed 1 test of 27. # Failed test 'add() tests' # at /kohadevbox/koha/t/db_dependent/api/v1/messages.t line 256. # Looks like you failed 1 test of 6. /kohadevbox/koha/t/db_dependent/api/v1/messages.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/6 subtests Also, I don't think you've got required permissions set as they should be. you've got: listMessages: requires full "borrowers" permission getMessage: requires full "borrowers" permission But in the staff interface a user with only "list_borrowers" can see messages on any patron at their library or group. With "view_borrower_infos_from_any_libraries" they can view messages on any patron. These permission requirements do correctly match what's required in the staff interface: addMessage: requires "edit_borrowers" permission updateMessage: requires "edit_borrowers" permission deleteMessage: requires "edit_borrowers" permission -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com --- Comment #16 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- *** Bug 42636 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #202403|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=23998 --- Comment #19 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- (In reply to Andrew Fuerste-Henry from comment #14)

I've got a test failure: * Proving /kohadevbox/koha/t/db_dependent/api/v1/messages.t FAIL

# Failed test at /kohadevbox/koha/t/db_dependent/api/v1/messages.t line 253. # found warning: Duplicate ID at /kohadevbox/koha/Koha/Object.pm line 176. # expected to find warning: (?^u:DBD::mysql::st execute failed: Duplicate entry '(.*?)' for key 'PRIMARY') # Looks like you failed 1 test of 27.

# Failed test 'add() tests' # at /kohadevbox/koha/t/db_dependent/api/v1/messages.t line 256. # Looks like you failed 1 test of 6. /kohadevbox/koha/t/db_dependent/api/v1/messages.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/6 subtests

Also, I don't think you've got required permissions set as they should be. you've got: listMessages: requires full "borrowers" permission getMessage: requires full "borrowers" permission

But in the staff interface a user with only "list_borrowers" can see messages on any patron at their library or group. With "view_borrower_infos_from_any_libraries" they can view messages on any patron.

These permission requirements do correctly match what's required in the staff interface: addMessage: requires "edit_borrowers" permission updateMessage: requires "edit_borrowers" permission deleteMessage: requires "edit_borrowers" permission

Should be fixed (In reply to Andrew Fuerste-Henry from comment #15)

When updating a message, why is one required to include the message_type, patron_id, and message content in the body of the request? If all I want to change is the patron_read_date, why not send only that in the body?

I put those fields as required because they are database fields that are not nullable and without a default value. I agree it's not logical to require them for a put request, but I am unsure how I should resolve this. I wouldn't just remove the requirements because of post but I don't think duplicating the definition file for put is good either. This issue seems to be present for the patron api post vs. put path/definition as well (required: surname, library_id, category_id) if I am not mistaken. Any advice? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 --- Comment #21 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- (In reply to Shi Yao Wang from comment #19)

I put those fields as required because they are database fields that are not nullable and without a default value. I agree it's not logical to require them for a put request, but I am unsure how I should resolve this. I wouldn't just remove the requirements because of post but I don't think duplicating the definition file for put is good either. This issue seems to be present for the patron api post vs. put path/definition as well (required: surname, library_id, category_id) if I am not mistaken.

Any advice?

I think the best course here would be to make the API call a PATCH instead of a PUT. You're correct that this problem exists elsewhere in Koha and there are a handful of bugs looking to move various API calls to PATCH (Bug 43279 and Bug 42916 are the first couple I found). -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff --- Comment #23 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- (In reply to Andrew Fuerste-Henry from comment #21)

I think the best course here would be to make the API call a PATCH instead of a PUT. You're correct that this problem exists elsewhere in Koha and there are a handful of bugs looking to move various API calls to PATCH (Bug 43279 and Bug 42916 are the first couple I found).

I added a patch route using bookings' API path as reference (which duplicates the yaml definition file :/) Kept the put route for complete message object replacement. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 --- Comment #25 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 207255 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=207255&action=edit Bug 23998: REST API for messages - remove message_date from put/patch Patch from commit 3c728d0 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #207159|0 |1 is obsolete| | Attachment #207231|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=23998 --- Comment #27 from David Nind <david@davidnind.com> --- Created attachment 207261 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=207261&action=edit Bug 23998: REST API for messages - Add patch method 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=23998 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #207255|0 |1 is obsolete| | --- Comment #29 from David Nind <david@davidnind.com> --- Comment on attachment 207255 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=207255 Bug 23998: REST API for messages - remove message_date from put/patch Fixed title for third patch for consistency with other patches (Capital "R" for remove). Testing notes (using KTD): 1. Apply patches. 2. Rebuild everything: yarn build 3. Restart everything: restart_all 4. Test pass: prove t/db_dependent/api/v1/messages.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #207260|0 |1 is obsolete| | Attachment #207261|0 |1 is obsolete| | Attachment #207262|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=23998 --- Comment #31 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 207293 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=207293&action=edit Bug 23998: REST API for messages - Add patch method 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=23998 --- Comment #33 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 207295 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=207295&action=edit Bug 23998: (QA follow-up) Tidy test file 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=23998 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@openfifth.co.u | |k Status|Passed QA |Failed QA --- Comment #34 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Hi all, quite a few QA findings here worth addressing: Setup 1) Rebuild the API spec and restart: koha-shell kohadev -c 'cd /kohadevbox/koha && yarn api:bundle' restart_all Finding 1: GET returns 500 once the message's author is deleted 1) Create a patron with username leaver and a password. Then use 'More' > 'Set permissions' to give them 'superlibrarian'. 2) Log out, log in as leaver, and open http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51 3) Click 'Add message', choose 'Staff - Internal note', type "Called about overdue items", and save. 4) Log back in as koha and delete leaver ('More' > 'Delete'). 5) Run this. It returns a 500 with Expected integer - got null: curl -s -u koha:koha 'http://localhost:8081/api/v1/messages?patron_id=51' 6) Open http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51. The staff interface still shows the note without any problem. Finding 2: library_id is optional in the spec, but a message can't be saved without it 1) Run this. It returns a 500: curl -s -u koha:koha -H 'Content-Type: application/json' -X POST http://localhost:8081/api/v1/messages \ -d '{"patron_id":51,"message_type":"L","message":"no library"}' 2) Check http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51. There's no "no library" message. Expected: either a 400 saying library_id is required, or the API defaults library_id to the logged-in user's library. Finding 3: PATCH returns 200 but doesn't save 1) Add a message and note the message_id in the response, <ID>: curl -s -u koha:koha -H 'Content-Type: application/json' -X POST http://localhost:8081/api/v1/messages \ -d '{"patron_id":51,"library_id":"CPL","message_type":"L","message":"Prefers phone calls"}' 2) Clear the message text: curl -s -u koha:koha -H 'Content-Type: application/json' -X PATCH http://localhost:8081/api/v1/messages/<ID> -d '{"message":""}' It returns 200 with "message":"", so it looks as if it saved. 3) Open http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51. The message still reads "Prefers phone calls", because nothing was saved. Finding 4: staff without 'view_borrower_infos_from_any_libraries' can read other libraries' internal messages 1) Create a staff patron at 'Fairfield' with username ffl and a password, and give them only 'Borrowers' > 'list_borrowers' and 'edit_borrowers'. Make sure 'view_borrower_infos_from_any_libraries' is not ticked. 2) Run this. It returns 404: without 'view_borrower_infos_from_any_libraries', they can't see koha's patron record at CPL: curl -s -u ffl:<password> http://localhost:8081/api/v1/patrons/51 3) Run this, using <ID> from Finding 3. It returns 200 with koha's internal ("L") message "Prefers phone calls" at CPL, so the same permission check isn't applied to messages: curl -s -u ffl:<password> http://localhost:8081/api/v1/messages/<ID> Finding 5: the API edits and deletes other libraries' messages despite AllowAllMessageDeletion 1) Leave 'AllowAllMessageDeletion' set to "Don't allow" (the KTD default). 2) Give ffl these extra permissions so they can see koha: 'catalogue' (staff login) and 'Borrowers' > 'view_borrower_infos_from_any_libraries'. 3) Log in as ffl at 'Fairfield' and open http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51 4) Look at the "Prefers phone calls" message from Finding 3 (a CPL message). There's no 'Edit' or 'Delete' next to it. 5) Now delete it through the API as ffl: curl -s -u ffl:<password> -X DELETE http://localhost:8081/api/v1/messages/<ID> -w '%{http_code}\n' 6) It returns 204. Refresh the patron page and the message is gone. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #36 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Fixed findings 1,2,4,5 from comment 34. Point 3 seems like normal behavior since an existing message is not saved when editing and putting empty message on the staff interface as well. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #37 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Thanks! 1, 2 and 5 look good now. Finding 4: still leaks messages: 1) Give a Fairfield staff user (ffl) only 'catalogue', 'list_borrowers' and 'edit_borrowers'. 2) As koha, set library to Fairfield and add a note on patron 51 (Centerville). 3) As ffl, the staff interface blocks patron 51, but the API shows the note: curl -s -u ffl:<password> 'http://localhost:8081/api/v1/messages?patron_id=51' Finding 3: the problem isn't that it doesn't save, it's that it returns 200, which means the update succeeded when it didn't. This happens when PATCHing an empty message, and now also when PATCHing library_id to null, because Koha::Patron::Message::store silently skips saving if any of borrowernumber, message, message_type or branchcode is empty. It should return a 400 with an error instead. Also, the file message_put.yaml shouldn't be needed: mark message_date as readOnly: true in message.yaml instead. That also stops POST from setting it, which it still allows. Please put it back to PQA after following up so it goes back into my queue. Thanks in advance! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 --- Comment #38 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 207678 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=207678&action=edit Bug 23998: fix 500 errors and apply branch and syspref permissions Patch from commit 3fe38f2 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #39 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Hi Shi Yao Wang, thanks for the follow-up! Library scoping now follows the patron's library, and message_date can no longer be set, but I don't think we're quite there yet. Two things left: New finding (from the last follow-up): staff without 'view_borrower_infos_from_any_libraries' get a 500 on ?patron_id=, PATCH, PUT and DELETE 1) Give a Centerville staff user (cpl) only 'catalogue', 'list_borrowers' and 'edit_borrowers'. 2) As koha (logged in at Centerville), add a note on patron 51, then get its message_id: curl -s -u koha:koha 'http://localhost:8081/api/v1/messages?patron_id=51' 3) As cpl, run: curl -s -u cpl:<password> 'http://localhost:8081/api/v1/messages?patron_id=51' curl -s -u cpl:<password> -X PATCH -H 'Content-Type: application/json' -d '{"message":"edited"}' http://localhost:8081/api/v1/messages/<message_id> curl -s -u cpl:<password> -X PUT -H 'Content-Type: application/json' -d '{"patron_id":51,"message_type":"L","message":"edited","library_id":"CPL"}' http://localhost:8081/api/v1/messages/<message_id> curl -s -u cpl:<password> -X DELETE http://localhost:8081/api/v1/messages/<message_id> 4) All four return a 500 ("Column ... in where clause is ambiguous" in the logs). They should return 200, 200, 200 and 204: cpl and patron 51 are both at Centerville, and the note was written at Centerville. The join in search_limited makes the unqualified filters ambiguous (branchcode in update/delete, and the patron_id query param, which the API maps to an unqualified borrowernumber). A subquery avoids it, similar to the pattern in Koha/Biblio.pm:412, something like: my $patrons = Koha::Patrons->search( { branchcode => { -in => \@restricted_branchcodes } } ); $params->{'me.borrowernumber'} = { -in => $patrons->_resultset->get_column('borrowernumber')->as_query }; A test with a restricted librarian would catch this: the existing tests use flags => 4**2, which includes 'view_borrower_infos_from_any_libraries', so search_limited never restricts anything. Finding 2 (still): PATCH {"library_id": null} still returns 200 without saving the change. It should return a 400. 1) As koha, get the message_id of a note on patron 51: curl -s -u koha:koha 'http://localhost:8081/api/v1/messages?patron_id=51' 2) Set its library to null: curl -s -u koha:koha -H 'Content-Type: application/json' -X PATCH http://localhost:8081/api/v1/messages/<message_id> -d '{"library_id":null}' 3) It returns 200 with "library_id":null. 4) Check what was saved: curl -s -u koha:koha http://localhost:8081/api/v1/messages/<message_id> 5) library_id is still the original library, so the change wasn't saved. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 --- Comment #40 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 207902 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=207902&action=edit Bug 23998: fix 500 errors and apply branch and syspref permissions Patch from commit b7a11f4 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23998 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.