http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18166|0 |1 is obsolete| | --- Comment #2 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Created attachment 18592 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18592&action=edit Bug 10258: Remove erroneous call to haspermission in basket.pl First parameter (userid) was not passed making the test useless. Removed the test for now. Current behavior does not change. Just adding the first parameter here does not make sense. Using the permission here needs more than that. Test plan: Open a closed basket in Acquisition. Check if you see all available basketgroups in the combo at the right side. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Marcel, Does the following code fix the problem? - my $member = GetMember(borrowernumber => $loggedinuser); - if ($basket->{closedate} && haspermission({ acquisition => 'group_manage'} )) { + my $member = GetMember(borrowernumber => $loggedinuser)->{userid}; + if ($basket->{closedate} && haspermission($uid, { acquisition => 'group_manage'} )) { -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 --- Comment #5 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to comment #3)

Marcel, Does the following code fix the problem?

- my $member = GetMember(borrowernumber => $loggedinuser); - if ($basket->{closedate} && haspermission({ acquisition => 'group_manage'} )) { + my $member = GetMember(borrowernumber => $loggedinuser)->{userid}; + if ($basket->{closedate} && haspermission($uid, { acquisition => 'group_manage'} )) {

Hi Jonathan, What I meant with adding the userid is not the solution, is what you propose here. I think that if you do this, you need to change the template too. With your patch, a user without this permission sees the button Change group and in the combo Add new group. If he clicks the button, he gets a login form warning him about permissions. I would not opt for that. Perhaps it would be better to remove the combo and button in this case; more friendly. Can I obsolete your patch now and let you do this in a qa followup? Thanks. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Trivial patch QA Contact| |jonathan.druart@biblibre.co | |m --- Comment #6 from Jonathan Druart <jonathan.druart@biblibre.com> --- QA comment: This patch comments some useless lines of code. No side effect possible, but an issue still exists. Marked as Passed QA. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 --- Comment #8 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 18944 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18944&action=edit Bug 10258: fix permissions check for setting basket group for order basket Improve the code that displays and allows staff to set the basket group from the basket details page for a closed basket. Prior to this patch, a staff member who did not have the group_manage acquisition permission would still see a control to change the group that the basket belongs to; attempting to change the group would present with with a login page. This patch also does some tidying of how basket group details are passed to the template. To test: [1] Create an order basket and close it. Do not assign it to a basket group. [2] View the basket details while logged in as a staff user who has the order_manage acquisitions permission but not the group_manage. The displayed basket group should be "No group". [3] Switch to a staff user who also has the group_manage permission, then view the basket details again. The basket group field should now be a select input that allows you to change the basket group. [4] Change the basket group. Verify that the basket group you selected is now displayed as the current group for that order basket. The basket group delivery and billing place fields should also now be displayed. [5] Close the basket group set in the previous step, then view the basket details again. This time, the basket group name should be displayed with a suffix of " (closed)", and no input to change the group should be displayed. [6] Swith to a staff user who does not have the group_manage permission, view the basket details, and verify that the basket name is displayed with a suffix of " (closed)". Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18853|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Trivial patch |Small patch -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 19039 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19039&action=edit screenshot of dialog described in comment I think we are almost there, but there is one little thing I am still worried about: When I close the basket with the user that has not group_manage permission, I still get offered the yellow dialog on closing the basket that asks me if I want to create a basket group with the same name as the order and put my order in it. If you check the checkbox and save you go to an error page: Error: You do not have permission to access this page. Click to log out... I think ideally we should not have this happen, but remove that bit from the dialog. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 --- Comment #14 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 19054 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19054&action=edit Bug 10258: offer to create basket group only when staff user has correct permission If the staff user does not have the group_manage acquisition permission, do not offer to create a new basket group when closing an order basket. This avoids a situation where if a staff member without that permission tries to close a basket and chose the option to create a bakset group, they would be redirected to the login page. To test: [1] Log in as a staff user that does not have the acquisition/group_manage permission. [2] Create a new order basket, attach at least one order line to it, then close it. [3] Verify that the confirmation page does not offer to create a basket group with the same name as the order basket. [4] Log in as a staff user that has the acquisition/group_manage permission. [5] Create and close an order basket. [6] Verify that this time, the confirmation page *does* offer to create a basket group. Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19053|0 |1 is obsolete| | --- Comment #16 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 19648 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19648&action=edit Bug 10258: fix permissions check for setting basket group for order basket Improve the code that displays and allows staff to set the basket group from the basket details page for a closed basket. Prior to this patch, a staff member who did not have the group_manage acquisition permission would still see a control to change the group that the basket belongs to; attempting to change the group would present with with a login page. This patch also does some tidying of how basket group details are passed to the template. To test: [1] Create an order basket and close it. Do not assign it to a basket group. [2] View the basket details while logged in as a staff user who has the order_manage acquisitions permission but not the group_manage. The displayed basket group should be "No group". [3] Switch to a staff user who also has the group_manage permission, then view the basket details again. The basket group field should now be a select input that allows you to change the basket group. [4] Change the basket group. Verify that the basket group you selected is now displayed as the current group for that order basket. The basket group delivery and billing place fields should also now be displayed. [5] Close the basket group set in the previous step, then view the basket details again. This time, the basket group name should be displayed with a suffix of " (closed)", and no input to change the group should be displayed. [6] Swith to a staff user who does not have the group_manage permission, view the basket details, and verify that the basket name is displayed with a suffix of " (closed)". Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|jonathan.druart@biblibre.co |m.de.rooy@rijksmuseum.nl |m | -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #18 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Looks good to me. Thanks for improvements, Galen. No complaints from koha-qa. Passed QA -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10258 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |tomascohen@gmail.com --- Comment #20 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This patch has been pushed to 3.12.x, will be in 3.12.3. Thanks Galen! -- You are receiving this mail because: You are watching all bug changes.