[Bug 42406] New: Create permission to allow user to delete only their own reports
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 Bug ID: 42406 Summary: Create permission to allow user to delete only their own reports Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Reports Assignee: koha-bugs@lists.koha-community.org Reporter: andrew@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: lisette@bywatersolutions.com It would be helpful to have a permission that allows a user to delete reports they've created themselves but not any report created by another user. The existing permission is "delete_reports" That could be renamed "delete_all_reports" and we could add "delete_own_reports." -- 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=42406 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- 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=42406 --- Comment #1 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 197670 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197670&action=edit Bug 42406: Split delete_reports into delete_own_reports and delete_all_reports This patch splits delete_reports into delete_own_reports and delete_all_reports While delete_all_reports supercedes delete_own_reports it seems to make more sense than delete_own_reports and delete_others_reports. With delete_all_reports we can have a user who can delete all, without specific permission to delete their own The other way we would have delete others, and you could not delete your own, which seems more odd To test: 1 - Have 2 users - 1 with delete_reports, 1 without 2 - Apply patches, update database 3 - Confirm user with permission now has 'delete_all_reports' and other user still has no reports deletion permission 4 - Give the second user 'delete_own_reports' permission 5 - Create 4 reports as each user 6 - Confirm view/edit pages show the delete button for each user, and confirm they both work 7 - You shoudl have 2 reports left for each user 8 - Confirm each user can delete their own report from the saved SQL list: http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?op=list 9 - There should be 1 report from each 10 - Give the second user 'delete_all_reports' permission in one tab and reload the saved reports in another tab 11 - Select both reports in the second tab 12 - Remove 'delete_all_reports' from the second user, leaving 'delete_own_reports' in the first tab 13 - Click 'Delete selected' in the reports tab 14 - Confirm you get 'You do not have permission to continue' 15 - Hit back, deselect other users report, confirm you can delete your own with 'Delete selected' 16 - Sign off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 --- Comment #2 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 197671 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197671&action=edit Bug 42406: (QA follow-up) Add missing POD Patch from commit c999d7b -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #3 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- When logged in as a user with only delete_own_reports, I am not given the Delete option in the dropdown on the Edit button after I've run one of my own reports or immediately after saving a new report. A user with delete_all_report (or with delete_reports before these patches) is offered a delete option in both of these cases. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 Nick Clemens (kidclamp) <nick@bywatersolutions.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=42406 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197670|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=42406 --- Comment #4 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 197977 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197977&action=edit Bug 42406: Split delete_reports into delete_own_reports and delete_all_reports This patch splits delete_reports into delete_own_reports and delete_all_reports While delete_all_reports supercedes delete_own_reports it seems to make more sense than delete_own_reports and delete_others_reports. With delete_all_reports we can have a user who can delete all, without specific permission to delete their own The other way we would have delete others, and you could not delete your own, which seems more odd To test: 1 - Have 2 users - 1 with delete_reports, 1 without 2 - Apply patches, update database 3 - Confirm user with permission now has 'delete_all_reports' and other user still has no reports deletion permission 4 - Give the second user 'delete_own_reports' permission 5 - Create 4 reports as each user 6 - Confirm view/edit pages show the delete button for each user, and confirm they both work 7 - You shoudl have 2 reports left for each user 8 - Confirm each user can delete their own report from the saved SQL list: http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?op=list 9 - There should be 1 report from each 10 - Give the second user 'delete_all_reports' permission in one tab and reload the saved reports in another tab 11 - Select both reports in the second tab 12 - Remove 'delete_all_reports' from the second user, leaving 'delete_own_reports' in the first tab 13 - Click 'Delete selected' in the reports tab 14 - Confirm you get 'You do not have permission to continue' 15 - Hit back, deselect other users report, confirm you can delete your own with 'Delete selected' 16 - Sign off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197671|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=42406 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |38414 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197977|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=42406 --- Comment #5 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 198106 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198106&action=edit Bug 42406: Split delete_reports into delete_own_reports and delete_all_reports This patch splits delete_reports into delete_own_reports and delete_all_reports While delete_all_reports supercedes delete_own_reports it seems to make more sense than delete_own_reports and delete_others_reports. With delete_all_reports we can have a user who can delete all, without specific permission to delete their own The other way we would have delete others, and you could not delete your own, which seems more odd To test: 1 - Have 2 users - 1 with delete_reports, 1 without 2 - Apply patches, update database 3 - Confirm user with permission now has 'delete_all_reports' and other user still has no reports deletion permission 4 - Give the second user 'delete_own_reports' permission 5 - Create 4 reports as each user 6 - Confirm view/edit pages show the delete button for each user, and confirm they both work 7 - You shoudl have 2 reports left for each user 8 - Confirm each user can delete their own report from the saved SQL list: http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?op=list 9 - There should be 1 report from each 10 - Give the second user 'delete_all_reports' permission in one tab and reload the saved reports in another tab 11 - Select both reports in the second tab 12 - Remove 'delete_all_reports' from the second user, leaving 'delete_own_reports' in the first tab 13 - Click 'Delete selected' in the reports tab 14 - Confirm you get 'You do not have permission to continue' 15 - Hit back, deselect other users report, confirm you can delete your own with 'Delete selected' 16 - Sign off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 Andrew Fuerste-Henry <andrew@bywatersolutions.com> 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=42406 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198106|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=42406 --- Comment #6 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 198114 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198114&action=edit Bug 42406: Split delete_reports into delete_own_reports and delete_all_reports This patch splits delete_reports into delete_own_reports and delete_all_reports While delete_all_reports supercedes delete_own_reports it seems to make more sense than delete_own_reports and delete_others_reports. With delete_all_reports we can have a user who can delete all, without specific permission to delete their own The other way we would have delete others, and you could not delete your own, which seems more odd To test: 1 - Have 2 users - 1 with delete_reports, 1 without 2 - Apply patches, update database 3 - Confirm user with permission now has 'delete_all_reports' and other user still has no reports deletion permission 4 - Give the second user 'delete_own_reports' permission 5 - Create 4 reports as each user 6 - Confirm view/edit pages show the delete button for each user, and confirm they both work 7 - You shoudl have 2 reports left for each user 8 - Confirm each user can delete their own report from the saved SQL list: http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?op=list 9 - There should be 1 report from each 10 - Give the second user 'delete_all_reports' permission in one tab and reload the saved reports in another tab 11 - Select both reports in the second tab 12 - Remove 'delete_all_reports' from the second user, leaving 'delete_own_reports' in the first tab 13 - Click 'Delete selected' in the reports tab 14 - Confirm you get 'You do not have permission to continue' 15 - Hit back, deselect other users report, confirm you can delete your own with 'Delete selected' 16 - Sign off Signed-off-by: Judy Poyer <jpoyer@aacpl.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=42406 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- Think I would've preferred "delete_any_reports" rather than "delete_all_reports" but that's splitting hairs. On permissions.inc "Delete SQL reports you have created"... I don't love the use of the word "you" there but it looks like there's already a precedent with "Perform inventory of your catalog" But that's just me being fussy. -- For real things... It looks like the bug number and description are missing from installer/data/mysql/atomicupdate/bug_42406.pl but I can add those in... I have the feeling that there is a unit test that needs updating... I'll look at that first... That's probably good for my first pass on the splinter review... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #7)
I have the feeling that there is a unit test that needs updating... I'll look at that first...
Bingo: t/Koha/Auth/Permissions.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #9 from David Cook <dcook@prosentient.com.au> --- Sorry folks but it's the dreaded fake ancestor... Error: Apply failed: Git command (git am -3 /tmp/eDMFxvHVpL/0001-198114.patch) failed: error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt). error: could not build fake ancestor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #9)
Sorry folks but it's the dreaded fake ancestor...
Error: Apply failed: Git command (git am -3 /tmp/eDMFxvHVpL/0001-198114.patch) failed: error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt). error: could not build fake ancestor
Ah wait that's on me. I didn't think the dependency was necessary but I guess it is after all! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 --- Comment #11 from David Cook <dcook@prosentient.com.au> --- (In reply to Andrew Fuerste-Henry from comment #6)
Created attachment 198114 [details] [review] Bug 42406: Split delete_reports into delete_own_reports and delete_all_reports
This patch splits delete_reports into delete_own_reports and delete_all_reports
While delete_all_reports supercedes delete_own_reports it seems to make more sense than delete_own_reports and delete_others_reports.
Mmm that's an interesting point. That's a tough one. It's a good example of how our current permission system isn't granular enough. I think that you're probably right here, because in a different permission system... it would likely either be "none", "all", or "X,Y,Z". Because we can't do "OR" when it comes to assigning permissions, this is probably our best bet. Because elsewhere it wouldn't be "none","all","me","everyone_else". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 --- Comment #12 from David Cook <dcook@prosentient.com.au> --- There's a discrepancy between the atomic update and the userpermissions.sql for the description but I can fix that... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 --- Comment #13 from David Cook <dcook@prosentient.com.au> --- There are a few places that still have CAN_user_reports_delete_reports koha-tmpl/intranet-tmpl/prog/en/includes/guided-reports-view.inc koha-tmpl/intranet-tmpl/prog/en/modules/reports/reports-home.tt I'll fix those too -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 --- Comment #14 from David Cook <dcook@prosentient.com.au> --- Hmm I don't think I like those [%- " "|$raw -%] tags. I think an alternative would be to just do the majority of the [% %] stuff above the <a> element and then just pop in short [% can_create %] and [% can_delete %] type tags. But... it looks like Jonathan added those bug 38414 so nevermind... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 --- Comment #15 from David Cook <dcook@prosentient.com.au> --- That said, that bit of code fails the QA tools. That's all right. Let's see how my idea works after all. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |dcook@prosentient.com.au |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198114|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=42406 --- Comment #16 from David Cook <dcook@prosentient.com.au> --- Created attachment 198122 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198122&action=edit Bug 42406: Split delete_reports into delete_own_reports and delete_all_reports This patch splits delete_reports into delete_own_reports and delete_all_reports While delete_all_reports supercedes delete_own_reports it seems to make more sense than delete_own_reports and delete_others_reports. With delete_all_reports we can have a user who can delete all, without specific permission to delete their own The other way we would have delete others, and you could not delete your own, which seems more odd To test: 1 - Have 2 users - 1 with delete_reports, 1 without 2 - Apply patches, update database 3 - Confirm user with permission now has 'delete_all_reports' and other user still has no reports deletion permission 4 - Give the second user 'delete_own_reports' permission 5 - Create 4 reports as each user 6 - Confirm view/edit pages show the delete button for each user, and confirm they both work 7 - You shoudl have 2 reports left for each user 8 - Confirm each user can delete their own report from the saved SQL list: http://localhost:8081/cgi-bin/koha/reports/guided_reports.pl?op=list 9 - There should be 1 report from each 10 - Give the second user 'delete_all_reports' permission in one tab and reload the saved reports in another tab 11 - Select both reports in the second tab 12 - Remove 'delete_all_reports' from the second user, leaving 'delete_own_reports' in the first tab 13 - Click 'Delete selected' in the reports tab 14 - Confirm you get 'You do not have permission to continue' 15 - Hit back, deselect other users report, confirm you can delete your own with 'Delete selected' 16 - Sign off Signed-off-by: Judy Poyer <jpoyer@aacpl.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=42406 --- Comment #17 from David Cook <dcook@prosentient.com.au> --- Created attachment 198123 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198123&action=edit Bug 42406: (QA follow-up) fix little inconsistencies and errors 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=42406 --- Comment #18 from David Cook <dcook@prosentient.com.au> --- Created attachment 198124 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198124&action=edit Bug 42406: (QA follow-up) fix QA tool failure 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=42406 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 --- Comment #19 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 Michelle Spinney <mspinney@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mspinney@clamsnet.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting --- Comment #20 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- This will not be backported to 25.11.x due to it being deemed an enhancement or a dependancy not being met -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42406 Bug 42406 depends on bug 38414, which changed state. Bug 38414 Summary: Reports permissions not properly enforced https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38414 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org