"new permission for claims: the librarian can have a permission ""claim for all"". Without this permission, he can claim only on basket he is a ""user"" of " (BibLibre MT6542)
Created attachment 9618 [details] [review] More granular permissions for claiming orders Add a new permission order_claim_for_all. Without this permission, user can only claim for orders in baskets he's authorised to manage (depends on bug 7295)
To test: - Apply patch for bug 7295 first and apply this patch - Run the updatedatabase.pl script - Go to acqui/lateorders.pl Checkboxes in last column must display only if you are an authorised user of the order's basket (creator, user, or connected to same branch as the basket branch, see bug 7295 for more details) - Grant permission 'order_claim_for_all' to your user and go back to acqui/lateorders.pl You must be able to claim for all displayed orders. Without GranularPermissions sysprefs, 'acquisition' permission should allow you to claim for all orders
Hi Julian, I think this is a similar situation as we have for baskets. For baskets you can currently set globally if - you can see only your own baskets - all baskets of your branch - all baskets in the system So I was thinking if we should have somthing similar for claiming. It might make sense to have an option to only be able to claim orders of your own library/branch.
Hi Katrin, Actually, this patch prevent users to claim on baskets they're not authorised to manage. So it take into account the AcqViewBaskets system preference: - If 'all', user can claim for all orders - If 'branch', user can claim for basket of his branch - If 'user', user can claim for baskets he own or he's a 'user' of I think we don't need another system preference here.
Ok. :)
Hm, thinking about the possible cases: If the user can not manage baskets at all or can only manage basket he/she created and has the permission to claim all order - he/she will always be able to claim all baskets from all branches, is that right? And I like that this is a permission, not a system preference :)
(In reply to comment #6) > Hm, thinking about the possible cases: > > If the user can not manage baskets at all or can only manage basket he/she > created and has the permission to claim all order - he/she will always be > able to claim all baskets from all branches, is that right? That's right, 'order_claim_for_all' is like a 'superlibrarian' permission for claims.
Applying: Bug 7292: More granular permissions for claiming orders fatal: sha1 information is lacking or useless (installer/data/mysql/de-DE/mandatory/userpermissions.sql). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge.
Created attachment 10968 [details] [review] Bug 7292: More granular permissions for claiming orders Rebased on master
Same error with this one: Applying: Bug 7292: More granular permissions for claiming orders fatal: sha1 information is lacking or useless (installer/data/mysql/de-DE/mandatory/userpermissions.sql). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge
Created attachment 11284 [details] [review] Bug 7292: More granular permissions for claiming orders Rebased on master Just a reminder: this patch needs patch(es) (at least the first) from bug 7295 in order to work (and apply correctly)
Created attachment 14518 [details] [review] Bug 7292: More granular permissions for claiming orders Rebased on master
Applying: Bug 7292: More granular permissions for claiming orders Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 7292: More granular permissions for claiming orders The copy of the patch that failed is found in: /home/christopher/git/koha/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".
Created attachment 21354 [details] [review] Bug 7292: More granular permissions for claiming orders Add a new permission order_claim_for_all. Without this permission, user can only claim for orders in baskets he's authorised to manage (depends on bug 7295)
Patch recreated on master
fatal: sha1 information is lacking or useless (installer/data/mysql/de-DE/mandatory/userpermissions.sql). Please provide 1 file for DB changes and 1 file (at least) for others changes.
Created attachment 25078 [details] [review] Bug 7292: New permission for claims (database changes only)
Created attachment 25079 [details] [review] Bug 7292: More granular permissions for claiming orders Add a new permission order_claim_for_all. Without this permission, user can only claim for orders in baskets he's authorised to manage
Patches split and rebased on master
I try first with a user who haven't the permission "order_claim_for_all Claim for all orders". On the table of Late orders,the user have checkbox in front of the order line from basket that he've created or that he was added in the list of users authorized for management of this basket. He can see the other line order but he can't check it for claims. I change the permissions to add "order_claim_for_all Claim for all orders". The users can check all the order, even those he haven't created or he doesn't manage. Ok for me.
It might be silly, but I am wondering a bit about the code for the new permission - order_claims_for_all, we have so far: budget_manage_all order_manage_all claim_serials So I think claim_order_all or order_claims_all would have been a bit more consistent. ... But I am going to continue with QA :)
Created attachment 26355 [details] [review] [PASSED QA] Bug 7292: New permission for claims (database changes only) Signed-off-by: Koha Team Lyon 3 <koha@univ-lyon3.fr> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Added sign off according to bugzilla.
Created attachment 26356 [details] [review] [PASSED QA] Bug 7292: More granular permissions for claiming orders Add a new permission order_claim_for_all. Without this permission, user can only claim for orders in baskets he's authorised to manage Signed-off-by: Koha Team Lyon 3 <koha@univ-lyon3.fr> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tested with: - AcqViewBaskets system preference - order_manage_all permission - order_claims_for_all permission Test plan: Each staff member is only allowed to manage their own baskets (AcqViewBaskets). From the 2 test users, none has order_manage_all or order_claims_for_all. In acquisitions vendor search, each can see their own baskets only - OK. On the late orders page, both can see all orders, but can only claim their own. - OK When one of the users is given order_claims_for_all - all orders can be claimed. - OK When AcqViewBaskets is set to 'in system, regardless of owner' all orders can be claimed by both users. OK Passes tests and QA script. Going to provide a follow up with the German translation.
Created attachment 26357 [details] [review] Bug 7292: German translation for new 'claim for all' permission Translates the description of the new user permission 'order_claim_for_all' to German.
Hi Julian, I had passed it, but then it occured to me, that introducing this new permission will mean taking away permissions for existing users as we don't update those. Could you make sure that we set the permission for existing staff users in the database update? I think from what I see in the template that would be all users having order_receive permission?
Created attachment 26677 [details] [review] Bug 7292: Add new permission to users that have 'order_receive' permission
You're right Katrin, this patch modify updatedatabase.pl to add automatically the new permission.
I try to apply against master 3.15.00.052 I think that the patches need to be rebased. I could resolve the CONFLICT on the first patch: Applying: Bug 7292: New permission for claims (database changes only) Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging installer/data/mysql/uk-UA/mandatory/permissions_and_user_flags.sql Auto-merging installer/data/mysql/ru-RU/mandatory/permissions_and_user_flags.sql Auto-merging installer/data/mysql/pl-PL/mandatory/userpermissions.sql Auto-merging installer/data/mysql/nb-NO/1-Obligatorisk/userpermissions.sql Auto-merging installer/data/mysql/it-IT/necessari/userpermissions.sql Auto-merging installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql Auto-merging installer/data/mysql/es-ES/mandatory/userpermissions.sql Auto-merging installer/data/mysql/en/mandatory/userpermissions.sql Auto-merging installer/data/mysql/de-DE/mandatory/userpermissions.sql Failed to merge in the changes. Patch failed at 0001 Bug 7292: New permission for claims (database changes only) Anyway, the second patch doesn't apply: Applying: Bug 7292: More granular permissions for claiming orders Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt I pass the patches to "Patch doesn't apply" status.
Created attachment 28462 [details] [review] Bug 7292: New permission for claims (database changes only) Rebased on master
Created attachment 28463 [details] [review] Bug 7292: More granular permissions for claiming orders Rebased on master
Created attachment 28464 [details] [review] Bug 7292: Add new permission to users that have 'order_receive' permission Rebased on master
Created attachment 28465 [details] [review] Bug 7292: German translation for new 'claim for all' permission Rebased on master
I test against master 3.17.00.007 The patch adds the new permission to users that have 'order_receive' permission: OK. updatedatabase.pl needs to be rebased. The test is OK. I pass the patch to "Signed Off" status.
Created attachment 28484 [details] [review] New permission for claims (database changes only)
Created attachment 28485 [details] [review] More granular permissions for claiming orders
Created attachment 28486 [details] [review] Add new permission to users that have 'order_receive' permission
Created attachment 28487 [details] [review] German translation for new 'claim for all' permission
Same here as for the other permission, could you please update to use the new superlibrarian check? +if ($userflags->{superlibrarian}
Created attachment 30341 [details] [review] Bug 7292: New permission for claims (database changes only) Rebased on master
Created attachment 30342 [details] [review] Bug 7292: More granular permissions for claiming orders Rebased on master
Created attachment 30343 [details] [review] Bug 7292: Add new permission to users that have 'order_receive' permission Rebased on master
Created attachment 30344 [details] [review] Bug 7292: German translation for new 'claim for all' permission Rebased on master
Created attachment 30345 [details] [review] Bug 7292: Use IsSuperLibrarian() instead of $userflags->{superlibrarian}
Just a note from code review: - $template->{VARS}->{'info_claim'} = 1; + $template->param(info_claim => 1); The first line was correct (TT), the second is still working (HTP), but outdated.
The database update works very nicely, seeting the permissions. But when I tried retesting the feature itself, I have run into problems: - AcqViewBaskets: created or managed by staff user - Staff patron doesn't have claim_for_all permission - aqorders.branch != staff patron home branch - aqorders.authorisedby != staff patron borrowernumber - Tried logging in and out after making the changes. ... and it still shows up in the late orders list. What am I missing?
Patron should also not have the permission order_manage_all and should not be in the basket's users list. Maybe we should not check for order_manage_all here. What do you think ?
This still seems a nice to have... but needs an extensive rebase to apply
No movement for years and no response to Katrin's question from several years ago (on bug 7290 or bug 7163), so moving to "WISHLIST".
I'd still like to have this - but resetting to new as we probably need to start fresh.