[Bug 16231] New: Correct permission handling in subscription edit menu
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Bug ID: 16231 Summary: Correct permission handling in subscription edit menu Change sponsored?: --- Product: Koha Version: master Hardware: All URL: /cgi-bin/koha/serials/subscription-detail.pl?subscript ionid=X OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: Serials Assignee: oleonard@myacpl.org Reporter: oleonard@myacpl.org QA Contact: testopia@bugs.koha-community.org CC: colin.campbell@ptfs-europe.com The "Edit" toolbar menu for serials contains three items, "Edit subscription," "Edit as new (duplicate)" and "Delete subscription." Each item should be wrapped in its own check for the "CAN_user_serials_*" permission. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 --- Comment #1 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 50089 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=50089&action=edit Bug 16231 - Correct permission handling in subscription edit menu This patch corrects the way subscription edit menu items are displayed according to the user's permissions settings. The edit menu has been updated to use the menu item terms "Edit," "Duplicate," and "Delete" instead of "Edit subscription," "Edit as new (duplicate)", and "Delete subscription. The change makes the terms less redundant and more consistent with other similar menus in Koha. Font Awesome icons have been added to each menu item. This patch also removes a non-standard "fa-lg" class from some Font Awesome icons in the toolbar. To test, apply the patch and log in to the staff client as a user who has permission to create, edit, and delete subscriptions. - Locate an subscription and view the details for it. - The "Edit" menu should show three options: "Edit," "Duplicate," and "Delete." - Confirm that each menu item works correctly. - Repeat the process when logged in as a user with varying combinations of create, edit, and delete permissions. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #50089|0 |1 is obsolete| | --- Comment #2 from Marc Véron <veron@veron.ch> --- Created attachment 50195 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=50195&action=edit Bug 16231 - Correct permission handling in subscription edit menu This patch corrects the way subscription edit menu items are displayed according to the user's permissions settings. The edit menu has been updated to use the menu item terms "Edit," "Duplicate," and "Delete" instead of "Edit subscription," "Edit as new (duplicate)", and "Delete subscription. The change makes the terms less redundant and more consistent with other similar menus in Koha. Font Awesome icons have been added to each menu item. This patch also removes a non-standard "fa-lg" class from some Font Awesome icons in the toolbar. To test, apply the patch and log in to the staff client as a user who has permission to create, edit, and delete subscriptions. - Locate an subscription and view the details for it. - The "Edit" menu should show three options: "Edit," "Duplicate," and "Delete." - Confirm that each menu item works correctly. - Repeat the process when logged in as a user with varying combinations of create, edit, and delete permissions. Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |veron@veron.ch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Comment on attachment 50195 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=50195 Bug 16231 - Correct permission handling in subscription edit menu Review of attachment 50195: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=16231&attachment=50195) ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/includes/serials-toolbar.inc @@ -50,4 @@
<div class="btn-group"> <button class="btn btn-small dropdown-toggle" data-toggle="dropdown"><i class="fa fa-pencil"></i> Edit <span class="caret"></span></button> <ul class="dropdown-menu"> - [% IF ( cannotedit ) %] <li class="disabled"> [% ELSE %]
If you follow 'cannotedit' through the code, you can see that the permission check is quite a complicated one here: sub _can_do_on_subscription It also checks superserials and IndependentBranches. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 --- Comment #4 from Owen Leonard <oleonard@myacpl.org> --- (In reply to Katrin Fischer from comment #3)
If you follow 'cannotedit' through the code
That whole menu is wrapped in "[% UNLESS ( cannotedit ) %]," so I concluded that the checks on "cannotedit" inside the menu were redundant. Does that sound correct? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #5 from Owen Leonard <oleonard@myacpl.org> --- I'm returning this to "signed off" because I think the QA issue is actually okay. I hope someone can take another look and let me know. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 --- Comment #6 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Thx Owen, sorry I didn't come back to this yet. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Owen, started to test the permissions - especially looking at: - edit_subscription - delete_subscription - create_subscription When the user has only delete_subscription out of those three it appears that the option is not visible. Could you take a look please? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Owen Leonard <oleonard@myacpl.org> 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=16231 --- Comment #8 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 50639 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=50639&action=edit Bug 16231 [Follow-up] Correct permission handling in subscription edit menu This follow-up allows for the case where the user has only "delete_subscription" permission. The delete option might be available even if the template's "cannotedit" variable is true. To test, apply the patch and revise a staff user's permission to have only the "delete_subscription" permission under Serials. Log in as that users and locate a subscription and view the details for that subscription. You should see a toolbar on that page containing only a "Delete" button. Repeat the steps above, adding serials permission to the user one by one, confirming that the toolbar continues to show the correct options. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #50639|0 |1 is obsolete| | --- Comment #9 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 50711 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=50711&action=edit Bug 16231 [Follow-up] Correct permission handling in subscription edit menu This follow-up allows for the case where the user has only "delete_subscription" permission. The delete option might be available even if the template's "cannotedit" variable is true. To test, apply the patch and revise a staff user's permission to have only the "delete_subscription" permission under Serials. Log in as that users and locate a subscription and view the details for that subscription. You should see a toolbar on that page containing only a "Delete" button. Repeat the steps above, adding serials permission to the user one by one, confirming that the toolbar continues to show the correct options. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Owen Leonard from comment #8)
This follow-up allows for the case where the user has only "delete_subscription" permission. The delete option might be available even if the template's "cannotedit" variable is true.
I don't understand why, could you please detail? I have not tested the patch, but looking at the diff it seems that it's a behavior change. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Maybe my first test here was flawed. This is something I see retesting this: - Set independentbranches to prevent - Make sure staff patron doesn't have superserials permission = I should not be allowed to change serials from other branches - Go to another branches subscription (by URL manipulation) - With the patches I can: renew (opens wrong window?), receive (ew!) and DELETE - Without the patch on master, I can't delete (better) I'd say this is a change of the behaviour that we don't want - allowing deletions. But overall... permission handling in serials could still be improved a lot. Can we find a good fix for delete here and then see about the other problems later? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think if you don't have edit_subscription, but delete an 'cannotedit = 0 (can edit)' - then maybe show a single 'delete' button? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Owen Leonard <oleonard@myacpl.org> 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=16231 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #50195|0 |1 is obsolete| | Attachment #50711|0 |1 is obsolete| | --- Comment #13 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 79904 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79904&action=edit Bug 16231: Correct permission handling in subscription edit This patch corrects the way subscription edit menu items are displayed according to the user's permissions settings. To test, apply the patch and log in to the staff client as a user who has permission to create, edit, and delete subscriptions. - Locate an subscription and view the details for it. - Confirm that each "Edit" menu item works correctly. - Repeat the process when logged in as a user with varying combinations of create, edit, and delete permissions. - Test as a user limited by IndependentBranches. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 --- Comment #14 from Owen Leonard <oleonard@myacpl.org> --- As far as I can tell you cannot delete a subscription without "edit" permission, despite the separate permissions. I think it has to do with out the "cannotedit" variable is defined. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79904|0 |1 is obsolete| | --- Comment #15 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 82336 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82336&action=edit Bug 16231: Correct permission handling in subscription edit This patch corrects the way subscription edit menu items are displayed according to the user's permissions settings. To test, apply the patch and log in to the staff client as a user who has permission to create, edit, and delete subscriptions. - Locate an subscription and view the details for it. - Confirm that each "Edit" menu item works correctly. - Repeat the process when logged in as a user with varying combinations of create, edit, and delete permissions. - Test as a user limited by IndependentBranches. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82336|0 |1 is obsolete| | --- Comment #16 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 83833 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83833&action=edit Bug 16231: Correct permission handling in subscription edit This patch corrects the way subscription edit menu items are displayed according to the user's permissions settings. To test, apply the patch and log in to the staff client as a user who has permission to create, edit, and delete subscriptions. - Locate an subscription and view the details for it. - Confirm that each "Edit" menu item works correctly. - Repeat the process when logged in as a user with varying combinations of create, edit, and delete permissions. - Test as a user limited by IndependentBranches. Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83833|0 |1 is obsolete| | --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 84680 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84680&action=edit Bug 16231: Correct permission handling in subscription edit This patch corrects the way subscription edit menu items are displayed according to the user's permissions settings. To test, apply the patch and log in to the staff client as a user who has permission to create, edit, and delete subscriptions. - Locate an subscription and view the details for it. - Confirm that each "Edit" menu item works correctly. - Repeat the process when logged in as a user with varying combinations of create, edit, and delete permissions. - Test as a user limited by IndependentBranches. Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |nick@bywatersolutions.com --- Comment #18 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Pushed to Master |Pushed to Stable --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pushed to 18.11.x for 18.11.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #20 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 18.05.x for 18.05.09 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 --- Comment #21 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Owen Leonard from comment #14)
As far as I can tell you cannot delete a subscription without "edit" permission, despite the separate permissions. I think it has to do with out the "cannotedit" variable is defined.
Not sure this is correct, in subscription-detail.pl: 52 # Permission needed if it is a deletion (del) : delete_subscription 53 # Permission needed otherwise : * 54 my $permission = ($op eq "del") ? "delete_subscription" : "*"; Not a big deal I'd say, but maybe a minor bug (?) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16231 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to Stable |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org