https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |15812 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 [Bug 15812] Checkout search with too many results (single character search) causes poor performance or timeout -- 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=30230 --- Comment #1 from George Williams (NEKLS) <george@nekls.org> --- This is a problem for our system also. We have libraries with volunteer staff who do not have "edit_borrowers" permission who can no longer search for borrowers by name. -- 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=30230 mathieu saby <mathsabypro@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mathsabypro@gmail.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #2 from David Cook <dcook@prosentient.com.au> --- We did have some folk struggle due to the change in permissions, although the description for "edit_borrowers" is "Add, modify and view patron information" and the patron search results does show patron information for all patrons. I'm not sure requiring "edit_borrowers" was the right way to go, but I can understand wanting to make it more restricted too. -- 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=30230 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |thibaud.guillot@biblibre.co | |m --- Comment #3 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Hello, This change in patron search has an impact on checkouts, a patron who does not have the permission to edit a borrower cannot perform a checkout, which would be most appropriate for correct this kind of problem ? -- 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=30230 --- Comment #5 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Thanks for your answer, in fact, they can use the cardnumber even if the patron search by name was surely more practical, it changes habits -- 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=30230 --- Comment #7 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- No problem, what would be the best solution to correct this behavior ?(In reply to David Cook from comment #6)

(In reply to Thibaud Guillot from comment #5)

...

Thanks for your answer, in fact, they can use the cardnumber even if the patron search by name was surely more practical, it changes habits

Sorry, my apologies. I agree that it's an issue. I've certainly received complaints about it.

No problem, what would be the best solution to correct this behavior ? -- 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=30230 --- Comment #9 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #8)

I'm not 100% sure that bug 15812 is the problem. I might take a quick little look and see...

Ok my bad I misunderstood how bug 15812 changed things but getting a better picture now. I think that change has been around too long to just revert, but it's clearly a regression. Hmm and actually the thing with the cardnumber working... it seems to only retrieve patrons from the same branch as the logged in account... -- 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=30230 --- Comment #11 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #10)

Note the autocomplete also gives a 403...

{"error":"Authorization failure. Missing required permission(s).","required_permissions":[{"borrowers":"edit_borrowers"},{"tools":"label_creator"},{"serials":"routing"},{"acquisition":"order_manage"}]} -- The authorizations/permissions need a comprehensive review/restructure. I think we've known that for a while. For instance, the reason why I can't retrieve patrons from other branches for checkout is because I don't have the sub permission "view_borrower_infos_from_any_libraries". But because I don't have "edit_borrower", I can't see borrower info from my current library either anyways. The authorization here makes no sense. -- I don't think this issue is really fixable on its own. It would require systemic changes to other functionality to really get it right. A short-term fix might be to create a new subpermission called "view_borrowers" and require circ staff to have that but even that's not quite right. We've worked ourselves into a corner with the current permissions and the functionality. Maybe we do just let "circulate_remaining_permissions" view member.pl and have that implicitly have the ability to "view patron information" despite it not really adhering to the explicit goals of the permission system. That's probably the unfortunate solution here... -- 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=30230 --- Comment #13 from David Cook <dcook@prosentient.com.au> --- (In reply to Thibaud Guillot from comment #7)

No problem, what would be the best solution to correct this behavior ?

tl;dr Someone can add "view_borrowers" and add that to member.pl, the Patrons REST API, and probably a bunch of other places. Or I suppose you could add "circulate_remaining_permissions" to member.pl, the Patrons REST API, and a bunch of other places... which would be theoretically silly but it would be practical. In any case, that someone isn't going to be me. It's too much work for too little gain. Unfortunately, the most practical workaround is to give "edit_borrowers" permission, and I think that's probably what most people have been doing to avoid this issue over the past couple of years. I wish whomever takes this on good luck though, because it would be a good one to fix. -- 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=30230 --- Comment #15 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #14)

This is actually a duplicate of bug 29509 I think.

I think you might be right. Or at least there is a lot of overlap. I think bug 29509 is already engaging with the hardest problem. -- 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=30230 --- Comment #17 from David Cook <dcook@prosentient.com.au> --- (In reply to George Williams (NEKLS) from comment #16)

I don't think removing these things from the search resultswould be terribly difficult if the staff user doesn't have edit_borrower permission and it would solve just about all of the problems that have been discussed here.

Unfortunately, in practice it's more complicated than that, because there other permissions like view_borrower_infos_from_any_libraries and preferences that also need to be taken into account for showing borrower information. It also means updating generic code libraries that are being used for other things where edit_borrower doesn't really apply. It's a bit of a mess. -- 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=30230 --- Comment #18 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #12)

In theory, the REST API was a good idea, but I think we've approached it a bit naively.

Rather than search using GET /api/v1/patrons, I reckon we should be POSTing to something like /api/v1/patrons/search, which can perform more nuanced operations. And we are looking into more nuanced operations but that's also challenging in its own way (especially from a performance perspective).

Actually, something like /api/v1/circ/search/patrons might be better. (Jonathan's ERM and preservation have shown how it's useful to organise endpoints under a module path.) -- 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=30230 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=35381 -- 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=30230 --- Comment #20 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 159409 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159409&action=edit Bug 30230: Add new 'list_borrowers' permission When a patron search is performed only a user with edit_borrowers permission can search by name. Search can works only with cardnumber but it makes searching less intuitive I think. So, as mentioned in the discussion, I've added a new 'list_borrowers' permission, completely independent of 'edit_borrowers', so that I can search for a member via the interface and get the results. In addition to the permission to perform check in and checkouts, this no longer poses an obstacle to simple use. Test plan: 1) Check with a user without 'edit_borrowers' permission that the patron search can only be performed with cardnumber 2) Apply this patch 3) Make the updatedatabase to add new 'list_borrowers' permissions 4) Set 'list_borrowers' permission on one user and see the difference -- 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=30230 --- Comment #22 from Andrew Fuerste-Henry <andrewfh@dubcolib.org> --- I feel like this permission should be grouped with the other borrowers permissions, rather than set off as its own top-level permission. I created a user with only catalogue, circulate_remaining_permissions, and list_borrowers. I was able to check out to a patron using either their cardnumber or name to find them and when I entered a name which found more than one patron I was able to access the patron search screen to select the correct patron. So that all worked as desired. A couple of minor things seem off, though, both on the member.pl search results page: - clicking a patron name in the results list takes one to cgi-bin/koha/members/moremember.pl, a page one would otherwise not be able to access with only list_borrowers - while on member.pl or moremember.pl as a user with only list_borrowers permission, the Search Patrons feature in the main search bar is only partially disabled -- the Search Patrons option still appears, but when it is selected the search box disappears, thereby preventing the search. This is a visually confusing approach and becomes more confusing if one clicks around between the options here, as the search box will reappear when something other than Search Patrons is selected. See the attached screenshot in which the interface is simultaneously indicating both Check Out and Search Patrons. Can we remove Search Patrons from this bar entirely if the user lacks that permission? -- 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=30230 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Assignee|koha-bugs@lists.koha-commun |thibaud.guillot@biblibre.co |ity.org |m --- Comment #24 from David Nind <david@davidnind.com> --- Added assignee. -- 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=30230 --- Comment #25 from David Cook <dcook@prosentient.com.au> --- (In reply to Andrew Fuerste-Henry from comment #22)

I feel like this permission should be grouped with the other borrowers permissions, rather than set off as its own top-level permission.

+1 This needs to be in "permissions" and not "userflags". This is important for the UI and overall permission logic.

A couple of minor things seem off, though, both on the member.pl search results page: - clicking a patron name in the results list takes one to cgi-bin/koha/members/moremember.pl, a page one would otherwise not be able to access with only list_borrowers

It's a good point. I think list_borrowers should be able to go to the moremember.pl page, since "list_borrowers" basically means you can list all patrons and their information. But they shouldn't be able to update/delete. In general, Koha doesn't have very nuanced authorization, but I suppose in this case maybe we should be defining "flagsrequired" based on HTTP method. If it's a GET, then "list_borrowers" can be included. But if it's a POST or something else, it needs to only be "edit_borrowers".

- while on member.pl or moremember.pl as a user with only list_borrowers permission, the Search Patrons feature in the main search bar is only partially disabled -- the Search Patrons option still appears, but when it is selected the search box disappears, thereby preventing the search. This is a visually confusing approach and becomes more confusing if one clicks around between the options here, as the search box will reappear when something other than Search Patrons is selected. See the attached screenshot in which the interface is simultaneously indicating both Check Out and Search Patrons. Can we remove Search Patrons from this bar entirely if the user lacks that permission?

I'm not sure I understand what you're saying here... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159409|0 |1 is obsolete| | --- Comment #26 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 159427 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159427&action=edit Bug 30230: Add new 'list_borrowers' permission When a patron search is performed only a user with edit_borrowers permission can search by name. Search can works only with cardnumber but it makes searching less intuitive I think. So, as mentioned in the discussion, I've added a new 'list_borrowers' permission, completely independent of 'edit_borrowers', so that I can search for a member via the interface and get the results. In addition to the permission to perform check in and checkouts, this no longer poses an obstacle to simple use. Test plan: 1) Check with a user without 'edit_borrowers' permission that the patron search can only be performed with cardnumber 2) Apply this patch 3) Make the updatedatabase to add new 'list_borrowers' permissions 4) Set 'list_borrowers' permission on one user and see the difference -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Andrew Fuerste-Henry <andrewfh@dubcolib.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #28 from Andrew Fuerste-Henry <andrewfh@dubcolib.org> --- With this new patch, I did the following: - create a user with catalogue, circulate_remaining_permissions, and list_borrowers permissions - log into the staff client as that user - in the Checkout field of mainpage.pl, search for "a" (with superlibrarian permissions, this returns all patrons with "a" in their name") - Koha takes you to the search results page, but shows no results and gives message "Something went wrong when loading this table. 403: Forbidden" - go back to the homepage and enter "Henry" in the Checkout box (in the default sandbox data this name finds only one patron and should take you straight to them) - Koha again takes you to the search results page and gives a 403 error -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #29 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 159517 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159517&action=edit Bug 30230 : (follow-up) Fix permission on /patrons api route -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #31 from Andrew Fuerste-Henry <andrewfh@dubcolib.org> --- Created attachment 159559 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159559&action=edit Link to be removed This latest patch addresses all of my concerns except for the "Search Patrons" link in the main search bar on the patron search results page. See attached screenshot. We should hide that "Search Patrons" link entirely. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #33 from David Cook <dcook@prosentient.com.au> --- *** Bug 35381 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=30230 --- Comment #34 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- (In reply to Andrew Fuerste-Henry from comment #31)

Created attachment 159559 [details] Link to be removed

This latest patch addresses all of my concerns except for the "Search Patrons" link in the main search bar on the patron search results page. See attached screenshot. We should hide that "Search Patrons" link entirely.

Hello Andrew, Normally I've taken your comments into account and removed the "Search patrons" link from the main search bar... it's only accessible to members with 'catalog' permission. Unless I've misunderstood, but from my point of view, with a member who only has the 'list_borrowers' permission, this seems correct. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #36 from Lucas Gass <lucas@bywatersolutions.com> --- (In reply to Thibaud Guillot from comment #35)

(In reply to Lucas Gass from comment #32)

...

As this stands no borrowers/staff will get the new permission on update. That is a change in behavior that needs to be avoided.

I think we need to give anyone who has the edit_borrowers permission the list_borrowers permission via a DB update.

Hello Lucas,

This permission is only there to allow search and list view of members, no access to the details of each member (moremember.pl) and therefore in my opinion no update... I've missed something else, or perhaps I'm misunderstanding you.

If staff can see/view borrower information before the patchset they need to have the same behavior after the patchset. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Thibaud Guillot <thibaud.guillot@biblibre.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=30230 --- Comment #39 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Hello, I've added the 'list_borrowers' permission on moremember.pl, as it seemed more logical, I had included it initially but removed it later. A member with only the list_borrowers permission won't be able to add or modify an existing pattern, but will be able to view its information. What I didn't understand was the need to create an atomicupdate to add the list_borrowers permission to members with the 'edit_borrowers' permission (comment 32). I hope I've understood you correctly this time :) thanks in any case for your feedback. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #41 from David Cook <dcook@prosentient.com.au> --- (In reply to Lucas Gass from comment #32)

As this stands no borrowers/staff will get the new permission on update. That is a change in behavior that needs to be avoided.

Originally, I was thinking this too, but now I'm not so sure, since this issue has been around for 2 years now.

I think we need to give anyone who has the edit_borrowers permission the list_borrowers permission via a DB update.

Thibaud has made it so you can have either list_borrowers or edit_borrowers, so I don't think we'd want to give list_borrowers to anyone with edit_borrowers. But perhaps we'd want to give list_borrowers to anyone with "circulate_remaining_permissions". But then I think maybe we don't need to worry about it, since over the last 2 years people will have probably given "edit_borrowers"... so maybe we're catering more to new installs / new users... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Thibaud Guillot <thibaud.guillot@biblibre.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=30230 --- Comment #44 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- (In reply to David Cook from comment #42)

I would've been happy to sign this off, but looks like there is a little perltidy problem with the database update.

If you perltidy that file, then you should be good to go.

--

testing 1 commit(s) (applied to 0d483a5 '22 DBRev 23.12.00.000: Start of a new')

Processing files before patches |========================>| 8 / 8 (100.00%) Processing files after patches |========================>| 8 / 8 (100.00%)

OK api/v1/swagger/paths/patrons.yaml

WARN installer/data/mysql/atomicupdate/bug_29509_add-new-list-borrowers- permission.pl WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 1)

OK koha-tmpl/intranet-tmpl/prog/en/includes/patron-search-header.inc

OK koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc

OK koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt

OK members/member.pl

OK members/members-home.pl

OK members/moremember.pl

Processing additional checks OK!

Thanks for you feedback, I rebased it cause the bug number was not good in update file. So I hope that will be okay now :) Have a nice day -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159633|0 |1 is obsolete| | --- Comment #46 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 159654 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159654&action=edit Bug 30230: Add new 'list_borrowers' permission When a patron search is performed only a user with edit_borrowers permission can search by name. Search can works only with cardnumber but it makes searching less intuitive I think. So, as mentioned in the discussion, I've added a new 'list_borrowers' permission, completely independent of 'edit_borrowers', so that I can search for a member via the interface and get the results. In addition to the permission to perform check in and checkouts, this no longer poses an obstacle to simple use. Test plan: 1) Check with a user without 'edit_borrowers' permission that the patron search can only be performed with cardnumber 2) Apply this patch 3) Make the updatedatabase to add new 'list_borrowers' permissions 4) Set 'list_borrowers' permission on one user and see the difference Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159654|0 |1 is obsolete| | --- Comment #47 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Created attachment 159667 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159667&action=edit Bug 30230: Add new 'list_borrowers' permission When a patron search is performed only a user with edit_borrowers permission can search by name. Search can works only with cardnumber but it makes searching less intuitive I think. So, as mentioned in the discussion, I've added a new 'list_borrowers' permission, completely independent of 'edit_borrowers', so that I can search for a member via the interface and get the results. In addition to the permission to perform check in and checkouts, this no longer poses an obstacle to simple use. Test plan: 1) Check with a user without 'edit_borrowers' permission that the patron search can only be performed with cardnumber 2) Apply this patch 3) Make the updatedatabase to add new 'list_borrowers' permissions 4) Set 'list_borrowers' permission on one user and see the difference Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #49 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 159668 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159668&action=edit Bug 30230 : (follow-up) Fix display issue on search bar -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #51 from Andrew Fuerste-Henry <andrewfh@dubcolib.org> --- (In reply to Thibaud Guillot from comment #50)

Thank you all for this time of testing and feedback. I think I've managed to fix the display problem Andrew was talking about.

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

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #53 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (Fixed commit message formatting to match coding guidelines) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159667|0 |1 is obsolete| | --- Comment #54 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 159988 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159988&action=edit Bug 30230: Add new 'list_borrowers' permission When a patron search is performed only a user with edit_borrowers permission can search by name. Search can works only with cardnumber but it makes searching less intuitive I think. So, as mentioned in the discussion, I've added a new 'list_borrowers' permission, completely independent of 'edit_borrowers', so that I can search for a member via the interface and get the results. In addition to the permission to perform check in and checkouts, this no longer poses an obstacle to simple use. Test plan: 1) Check with a user without 'edit_borrowers' permission that the patron search can only be performed with cardnumber 2) Apply this patch 3) Make the updatedatabase to add new 'list_borrowers' permissions 4) Set 'list_borrowers' permission on one user and see the difference Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #56 from Victor Grousset/tuxayo <victor@tuxayo.net> --- It works :) including fixing the issue in comment 45 Main test plan done with minimal permissions at the start (catalogue, circulate_remaining_permissions) And also with maximal permissions (all but superlibrarian and edit_borrowers) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159988|0 |1 is obsolete| | --- Comment #57 from David Cook <dcook@prosentient.com.au> --- Created attachment 159990 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159990&action=edit Bug 30230: Add new 'list_borrowers' permission When a patron search is performed only a user with edit_borrowers permission can search by name. Search can works only with cardnumber but it makes searching less intuitive I think. So, as mentioned in the discussion, I've added a new 'list_borrowers' permission, completely independent of 'edit_borrowers', so that I can search for a member via the interface and get the results. In addition to the permission to perform check in and checkouts, this no longer poses an obstacle to simple use. Test plan: 1) Check with a user without 'edit_borrowers' permission that the patron search can only be performed with cardnumber 2) Apply this patch 3) Make the updatedatabase to add new 'list_borrowers' permissions 4) Set 'list_borrowers' permission on one user and see the difference Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <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=30230 --- Comment #59 from David Cook <dcook@prosentient.com.au> --- Created attachment 159992 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159992&action=edit Bug 30230: (follow-up) fix unit test Signed-off-by: David Cook <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=30230 --- Comment #60 from David Cook <dcook@prosentient.com.au> --- I'm pretty happy with how this one has turned out. Just added a simple unit test fix follow-up. It is tempting to add a Selenium test, but I'm not sure what it would prove. Would it prove that it could list borrowers or that it can't edit them? Or that it couldn't access other parts of Koha? In any case, marking Passed QA. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #62 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Thanks David for your help in reviewing :)

It is tempting to add a Selenium test

Run, you fool! it's a trap! => Bug 29285 (In reply to Jonathan Druart from comment #61)

In doubt, cover all use cases? :D

To maximize random test failures :D -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #64 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This doesn't look like too big a change and I think the permission will be useful for other use cases that we still need to target. So yay! BUT: I got questions. 1) Should we update existing staff patron accounts to make this change more convenient? Should everyone having edit_borrowers also have list_borrowers? 2) Permission description/scope And for my understanding: the permission description reads: Search and list patrons Do we include "view" in list? And should we add it to the description? How does it work in combination with view_borrower_infos_from_any_libraries? I think the problem right now is that our sub permissions in borrowers are not always very clear and I'd like to try and make sure we improve things moving forward! So please bear with my questions :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #66 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---

How does it work in combination with view_borrower_infos_from_any_libraries?

Or more: how does it relate to this one? I am just trying to get a clear picture on how this works. Please switch back to PQA after reply, so I don't miss it. Thanks! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #68 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I reviewed the patch more closely today and tested. We'll need some more small fixes: 1) + bug_number => "BUG_30230", This is wrong and should be just 30230. 2) Please add view to the permission descriptions in the SQL and template file: + "INSERT IGNORE INTO permissions (module_bit, code, description) VALUES (4, 'list_borrowers', 'Search and list patrons')" 3) In the main navigation (not start page, but on top) the Patrons module is missing with only catalogue and list_borrowers permission. I quite like the way how this lets you limit patron search to your own library in combination with library groups. When view_borrower_infos_from_any_libraries is added, I can search them all. But I am not required to also give edit_borrowers. This is nice. Please help to finish this! :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #70 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *ping* Still waiting for the follow-up! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #71 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 161068 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161068&action=edit Bug 30230: (follow-up) Add list permission on home-search include and update atomic update file -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159991|0 |1 is obsolete| | --- Comment #73 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 161241 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161241&action=edit Bug 30230: (follow-up) Fix display issue on search bar Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161068|0 |1 is obsolete| | --- Comment #75 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 161243 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161243&action=edit Bug 30230: (follow-up) Add list permission on home-search include and update atomic update file Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|dcook@prosentient.com.au |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #77 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Hold off.. I'm writing API tests for this now. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #79 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 161246 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161246&action=edit Bug 30230: (QA follow-up) Also add list_borrowers to the singular endpoint -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161246|0 |1 is obsolete| | --- Comment #82 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 161251 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161251&action=edit Bug 30230: (QA follow-up) Also add list_borrowers to the singular endpoint -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #83 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- The filtering code was getting in the way of the tests.. the weird 'view_borrower_infos_from_any_libraries' sub-permission that's not entirely a permission in the same way all others are. Anyway.. tests are fixed and I'm happy to PQA now -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #84 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---

2) Please add view to the permission descriptions in the SQL and template file:

+ "INSERT IGNORE INTO permissions (module_bit, code, description) VALUES (4, 'list_borrowers', 'Search and list patrons')"

The .sql file for new installations and the .inc for description in the template were missed. Fixed in a follow-up.

3) In the main navigation (not start page, but on top) the Patrons module is missing with only catalogue and list_borrowers permission.

This was missed as well, fixed in a follow-up to header.inc. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #85 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35980 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35980 [Bug 35980] Add message to patron needs permission check -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|24.05.00 |24.05.00,23.11.03 released in| | Patch complexity|--- |Medium patch --- Comment #88 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.03 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 --- Comment #89 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- *** Bug 32502 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=30230 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |Needs documenting CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #90 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11 unless requested -- You are receiving this mail because: You are watching all bug changes.