To recreate: - have a system with 2+ branches - have a bib with 2+ items, all with holdingbranch=A - set logged in library to A - go to bib details page, confirm each item has an Edit button in the holdings table - edit one item, set holdingbranch=B - reload bib details page - confirm the item now at branch B does not have an edit button
I meant to include the detail that we are not using library groups to limit editing of records.
Hm, I think that is another side effect of bug 37392. Can you try if the view_borrower_infos_from_any_libraries makes any difference?
(In reply to Katrin Fischer from comment #2) > Hm, I think that is another side effect of bug 37392. > Can you try if the view_borrower_infos_from_any_libraries makes any > difference? I've observed this on two different superlibrarian accounts, so I hadn't really considered a permissions-based solution. Just now I created an account with (catalogue) and (editcatalogue) permissions. They did not get an edit button for an item at a different library. When I added the view_borrower_infos_from_any_libraries permission it did not make the button appear.
confirmed in main
(In reply to Andrew Fuerste-Henry from comment #3) > (In reply to Katrin Fischer from comment #2) > > Hm, I think that is another side effect of bug 37392. > > Can you try if the view_borrower_infos_from_any_libraries makes any > > difference? > > I've observed this on two different superlibrarian accounts, so I hadn't > really considered a permissions-based solution. Just now I created an > account with (catalogue) and (editcatalogue) permissions. They did not get > an edit button for an item at a different library. When I added the > view_borrower_infos_from_any_libraries permission it did not make the button > appear. You also need edit_items I believe And try to add a library group with all your libraries within and remove the view_borrower_infos_from_any_libraries... But I think it's still the same bit of code that causes both bugs, even if we cannot use the same workaround. I dug into the code making the button display yesterday, and there is definitely something wrong there. For someone looking here, please also look at my notes on bug 37392.
Created attachment 169388 [details] [review] Bug 37383: Use libraries_where_can_edit_items to check if items are editable To test: 1 - have a system with 2+ branches 2 - have a bib with 2+ items, all with holdingbranch=A 3 - set logged in library to A 4 - go to bib details page, confirm each item has an Edit button in the holdings table 5 - edit one item, set holdingbranch=B 6 - reload bib details page 7 - confirm the item now at branch B does not have an edit button 8 - APPLY PATCH and restart_all 9 - Try 1 - 6 again, this time you should see the proper edit buttons 10 - Set up library group: * Create a library group for library A + B * Action: Limit item editing by group 11 - Set up test user: * Create a staff patron with these permissions: * catalogue * fast_cataloguing * edit_items * view_borrower_infos_from_any_libraries * edit borrowers * Home library: library A 12 - Set up test items: * Create a record with 3 items with different home libraries: * A * B * C 13 - We expect the use to be able to edit A and B, but not C. 14 - With the same user turn OFF the view_borrower_infos_from_any_libraries permission 15 - Behavior should not change, we expect the use to be able to edit A and B, but not C. 16 - Make sure StaffLocationOnDetail still works as expected for all users.
Created attachment 169395 [details] [review] Bug 37383: Use libraries_where_can_edit_items to check if items are editable To test: 1 - have a system with 2+ branches 2 - have a bib with 2+ items, all with holdingbranch=A 3 - set logged in library to A 4 - go to bib details page, confirm each item has an Edit button in the holdings table 5 - edit one item, set holdingbranch=B 6 - reload bib details page 7 - confirm the item now at branch B does not have an edit button 8 - APPLY PATCH and restart_all 9 - Try 1 - 6 again, this time you should see the proper edit buttons 10 - Set up library group: * Create a library group for library A + B * Action: Limit item editing by group 11 - Set up test user: * Create a staff patron with these permissions: * catalogue * fast_cataloguing * edit_items * view_borrower_infos_from_any_libraries * edit borrowers * Home library: library A 12 - Set up test items: * Create a record with 3 items with different home libraries: * A * B * C 13 - We expect the use to be able to edit A and B, but not C. 14 - With the same user turn OFF the view_borrower_infos_from_any_libraries permission 15 - Behavior should not change, we expect the use to be able to edit A and B, but not C. 16 - Make sure StaffLocationOnDetail still works as expected for all users. Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Thanks Lucas! The syspref StaffLocationOnDetail does not exist yet (see bug 15461)
(In reply to Michaela Sieber from comment #8) > Thanks Lucas! > > The syspref StaffLocationOnDetail does not exist yet (see bug 15461) Michaela, thanks for testing. I put the wrong system preference name in the test plan. The one I meant to mention was StaffDetailItemSelection, sorry for the confusion!
(In reply to Lucas Gass from comment #9) > (In reply to Michaela Sieber from comment #8) > > Thanks Lucas! > > > > The syspref StaffLocationOnDetail does not exist yet (see bug 15461) > > Michaela, thanks for testing. I put the wrong system preference name in the > test plan. The one I meant to mention was StaffDetailItemSelection, sorry > for the confusion! OK, then I have bad news: StaffDetailItemSelection is enabled, but with my limited user I don't see the first column with the ckeckboxes And one more thing: on the item details page ( cgi-bin/koha/catalogue/moredetail.pl?biblionumber=131 ) I can only edit the item that matches the homebranch of my limited user
> OK, then I have bad news: > StaffDetailItemSelection is enabled, but with my limited user I don't see > the first column with the ckeckboxes Depending on how limited your user is, I think this is expected. In catalogue/detail.pl I see that the checkbox column should be hidden when the sys pref is on if users have certain 'tools' permissions like items_batchmod and items_batchdel: > And one more thing: > on the item details page ( > cgi-bin/koha/catalogue/moredetail.pl?biblionumber=131 ) > I can only edit the item that matches the homebranch of my limited user IMO, this should be a separate bug report.
Created attachment 169429 [details] [review] Bug 37383: Use libraries_where_can_edit_items to check if items are editable To test: 1 - have a system with 2+ branches 2 - have a bib with 2+ items, all with holdingbranch=A 3 - set logged in library to A 4 - go to bib details page, confirm each item has an Edit button in the holdings table 5 - edit one item, set holdingbranch=B 6 - reload bib details page 7 - confirm the item now at branch B does not have an edit button 8 - APPLY PATCH and restart_all 9 - Try 1 - 6 again, this time you should see the proper edit buttons 10 - Set up library group: * Create a library group for library A + B * Action: Limit item editing by group 11 - Set up test user: * Create a staff patron with these permissions: * catalogue * fast_cataloguing * edit_items * view_borrower_infos_from_any_libraries * edit borrowers * Home library: library A 12 - Set up test items: * Create a record with 3 items with different home libraries: * A * B * C 13 - We expect the use to be able to edit A and B, but not C. 14 - With the same user turn OFF the view_borrower_infos_from_any_libraries permission 15 - Behavior should not change, we expect the use to be able to edit A and B, but not C. 16 - Make sure StaffDetailItemSelection still works as expected for all users. With the sys pref on the checkbox should show up for superlibrarians and users with 'tools' permissions. Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
New patch attached with an updated test plan, keeping Michaela's sign-off line.
Created attachment 169435 [details] [review] Bug 37383: Use libraries_where_can_edit_items to check if items are editable To test: 1 - have a system with 2+ branches 2 - have a bib with 2+ items, all with holdingbranch=A 3 - set logged in library to A 4 - go to bib details page, confirm each item has an Edit button in the holdings table 5 - edit one item, set holdingbranch=B 6 - reload bib details page 7 - confirm the item now at branch B does not have an edit button 8 - APPLY PATCH and restart_all 9 - Try 1 - 6 again, this time you should see the proper edit buttons 10 - Set up library group: * Create a library group for library A + B * Action: Limit item editing by group 11 - Set up test user: * Create a staff patron with these permissions: * catalogue * fast_cataloguing * edit_items * view_borrower_infos_from_any_libraries * edit borrowers * Home library: library A 12 - Set up test items: * Create a record with 3 items with different home libraries: * A * B * C 13 - We expect the use to be able to edit A and B, but not C. 14 - With the same user turn OFF the view_borrower_infos_from_any_libraries permission 15 - Behavior should not change, we expect the use to be able to edit A and B, but not C. 16 - Make sure StaffDetailItemSelection still works as expected for all users. With the sys pref on the checkbox should show up for superlibrarians and users with 'tools' permissions. Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Thanks for all the hard work! Pushed to main for the next 24.11.00 release as RM Assistant
Backported to 24.05.x for upcoming 24.05.04
Depends on Bug 33568 not in 23.11.x
Lucas, can you explain why the original code was not working?
I see, was it because we testing the holding library instead of the home library? if ( can_edit_items_from[row.holding_library_id] ){ vs if ( can_edit_items_from[row.home_library_id] ){ If so, why didn't you simply adjusted the two lines in the template? IIRC it seemed weird to pass an empty array if items from all libraries were editable. Not blocking or requesting a follow-up, just asking ;)
(In reply to Jonathan Druart from comment #19) > I see, was it because we testing the holding library instead of the home > library? > > if ( can_edit_items_from[row.holding_library_id] ){ > > vs > > if ( can_edit_items_from[row.home_library_id] ){ Yes, the @libraries array was only returning home libraries, not holding. So that's why there was missing edit button for things that a librarian should have been able to edit. > If so, why didn't you simply adjusted the two lines in the template? Sorry for my ignorance, which 2 lines? > IIRC it seemed weird to pass an empty array if items from all libraries were > editable. Agreed, the empty array is weird but it's the solution I found. You should also look at Nick's Bug 37392. I am also happy to do additional work, or follow-up, if you think we could improve what we currently have.
(In reply to Lucas Gass from comment #20) > (In reply to Jonathan Druart from comment #19) > > If so, why didn't you simply adjusted the two lines in the template? > Sorry for my ignorance, which 2 lines? I would have expected your patch to simply adjust holding_library_id: if ( can_edit_items_from[row.holding_library_id] ){ with if ( can_edit_items_from[row.home_library_id] ){ koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers/tables/items/catalogue_detail.inc lines 296 and 747 in current main.
*** Bug 37300 has been marked as a duplicate of this bug. ***