Bug 22868

After our upgrade to 18.11 we noticed that staff were able to edit/delete purchase suggestions from the home page link.  In the past, staff were allowed staff to create purchase suggestions for patrons, but they could not edit/delete them.  I agree with Lisette, that it would be great to have a separate permission like "enter purchase sugggestions" for staff.

I would also like to say we definitely need to keep these separate.  This has been a little headache.

I want to change this to a bug, as I don't know that it was intended for this to happen.

Circulation staff do create purchase suggestions on patron's behalf regularly, and they do need the "managing purchase suggestions" permission to create these suggestions.

After upgrading to 19.05, we have noticed that if you allow the "managing purchase suggestions" permission (without other acquisition permissions), you can access the budget and see what was ordered. So, if you are a circulation staff, you can access the budget. This is not an ideal situation.

I do agree with the others, it would be great to have a separate permission called "enter purchase suggestions" for staff.

Oh my gosh!  This is not good at all!  We need this fixed ASAP.  I can only do so much to work around this without crippling our circulation staff.  Someone please fix.

Staff account with limited access issues.

For example: circulation staff will require the "managing purchase suggestion" permission to allow them to enter a purchase suggestion on a patron's behalf (on the staff interface). Unfortunately, this permission will also unlock the access to the budget.


Test plan

1) Create a staff account with the following permissions: "Checkout/checkin", "Add, modify and view patron info", "Staff access, allow viewing of catalogue..." and "Manage purchase suggestions" (without other acquisition permissions)

2) Logout and Login to the staff interface with this new account.
3) Access the Acquisition module and notice the budget

Can you confirm that the problematic information is on the Acquisition home page (acqui/acqui-home.pl) and is the fund list?

acqui/acqui-home.pl:        flagsrequired   => { acquisition => '*' },
acqui/booksellers.pl:        flagsrequired   => { acquisition => '*' },
acqui/histsearch.pl:        flagsrequired   => { acquisition => '*' },
acqui/invoice-files.pl:        flagsrequired   => { 'acquisition' => '*' },
acqui/invoice.pl:        flagsrequired   => { 'acquisition' => '*' },
acqui/invoices.pl:        flagsrequired   => { 'acquisition' => '*' },
acqui/modordernotes.pl:    flagsrequired   => { 'acquisition' => '*' },
acqui/ordered.pl:        flagsrequired   => { acquisition => '*' },
acqui/showorder.pl:    flagsrequired   => { acquisition => '*' },
acqui/spent.pl:        flagsrequired   => { acquisition => '*' },
acqui/supplier.pl:        flagsrequired   => { acquisition => '*' },
services/itemrecorddisplay.pl:    flagsrequired   => {acquisition => '*'},


I would suggest to move acquisition.suggestions_manage to a new permission "namespace", suggestions.suggestions_manage.
Would it work?

It would be a good thing to separate permissions:
1 - "Enter purchase suggestions"
2 - "Manage purchase suggestions"

Yes it's on (acqui/acqui-home.pl) page.

It would be a good thing to separate permissions:
1 - "Enter purchase suggestions"
2 - "Manage purchase suggestions"

(In reply to Marie-Luce Laflamme from comment #10)
> Yes it's on (acqui/acqui-home.pl) page.
> 
> It would be a good thing to separate permissions:
> 1 - "Enter purchase suggestions"
> 2 - "Manage purchase suggestions"

It will not fix the problem.
The issue here is that script are checking for acquisition => '*' permission, which means "whichever permission is ok for access".
Splitting the permission will not block the access to these pages.

That's why I suggestion to move the manage_suggestions out of the "acquisition". We could split the permission later if still needed.

It may not fix the problem, but we should definitely find a better way to handle purchase suggestions.

As mentioned earlier, circulation staff required the "managing purchase suggestions" permission to create purchase suggestions on a patron's behalf. 

However, you do not want circulation staff to decide if a purchase suggestion should be ordered nor access to the budget. Only librarians should manage suggestions.

There are 2 problems:

- access to the budget without permission
- the need of 2 separate permissions (" enter" vs "manage" purchase suggestions)

(In reply to Marie-Luce Laflamme from comment #12)
> It may not fix the problem, but we should definitely find a better way to
> handle purchase suggestions.
> 
> As mentioned earlier, circulation staff required the "managing purchase
> suggestions" permission to create purchase suggestions on a patron's behalf. 
> 
> However, you do not want circulation staff to decide if a purchase
> suggestion should be ordered nor access to the budget. Only librarians
> should manage suggestions.
> 
> There are 2 problems:
> 
> - access to the budget without permission
> - the need of 2 separate permissions (" enter" vs "manage" purchase
> suggestions)

Yes, I agree with the 2 permissions but it's an enhancement request, not a "critical" bug (also not a regression).

(In reply to Jonathan Druart from comment #13)

> 
> Yes, I agree with the 2 permissions but it's an enhancement request, not a
> "critical" bug (also not a regression).

Not sure why you would call this an enhancement.  This is a bug.  And I don't recall this behavior on past versions.  At some point this changed, and is not a desired effect.

(In reply to Christopher Brannon from comment #14)
> (In reply to Jonathan Druart from comment #13)
> 
> > 
> > Yes, I agree with the 2 permissions but it's an enhancement request, not a
> > "critical" bug (also not a regression).
> 
> Not sure why you would call this an enhancement.  This is a bug.  And I
> don't recall this behavior on past versions.  At some point this changed,
> and is not a desired effect.

I did not try but I think that prior to bug 11911, when you were able to create a new suggestion, you also were able to edit it. That is why we need 2 permissions. That must be fixed on a separated bug report, it's an enhancement request.

The "critical" bit here is that you can now access acquisition screens if you have the acquisition.manage_suggestions permission. That must be fixed here, it's a regression.

As I am the one to blame here I can say something about previous behaviour.
Before the new permission was added, there was 'no permission' needed for making suggestions and managing them in Koha. If you had the link to the suggestions page, you could do it. Koha was just hiding the link to the acquisition module from display.

So I introduced a new permission to remedy that... but that caused the problem we see now: we have other pages that require very lax permissions (any acq permission).

I think what Jonathan suggests, moving this out of acq will be a nice fix for the bug. It will go back to where we were before, but we will have a permission to turn it on and off.

Then the next step we should discuss how to split it up for editing etc.

Note: I am not a fan of splitting CRUD wise, because this means that you can enter a suggestion with a silly typo (add), but you won't be able to fix it (update) or delete it, if the patron no longer wants it.

I think it's better to think about use cases here. Like: allow to change status, allow to enter acq information or similar.

Note: moving it out of acq will also make it way easier to add those sub-permissions!

Created attachment 97334 [details] [review]
Bug 22868: Move suggestions_manage subperm out of acquisition perm

Bug 11911 replaced the permission of suggestions.pl (create a purchase
suggestion) from catalogue => 1 to acquisition => 'suggestions_manage'.
However we have a lot of acquisition scripts that have lax permissions
(acquisition => '*' which means any sub permissions of acquisition is
enough).

That causes problem when a circulation staff can create purchase
suggestions but not access acquisition information.

One solution is to move the suggestions_manage subpermission out of the
acquisition permission and create a new suggestion permission.

Test plan:
0. Setup
* Create a patron with several permission (and full acquisition
permission)
* Create another patron with several permission, and suggestions_manage
permission
* Create another patron without the suggestions_manage permission
1. Apply the patch and execute the update database entry
2. Note that the third patron you create still does not have
suggestions_manage
3. Confirm that you can create a purchase suggestion if you have
suggestions_manage, but cannot access acquisition pages if you do not
have any subpermissions of the acquisition permission

Marie-Luce or Christopher, could one of you sign-off on this? It should work on on the sandboxes.

Created attachment 97666 [details] [review]
Bug 22868: Move suggestions_manage subperm out of acquisition perm

Bug 11911 replaced the permission of suggestions.pl (create a purchase
suggestion) from catalogue => 1 to acquisition => 'suggestions_manage'.
However we have a lot of acquisition scripts that have lax permissions
(acquisition => '*' which means any sub permissions of acquisition is
enough).

That causes problem when a circulation staff can create purchase
suggestions but not access acquisition information.

One solution is to move the suggestions_manage subpermission out of the
acquisition permission and create a new suggestion permission.

Test plan:
0. Setup
* Create a patron with several permission (and full acquisition
permission)
* Create another patron with several permission, and suggestions_manage
permission
* Create another patron without the suggestions_manage permission
1. Apply the patch and execute the update database entry
2. Note that the third patron you create still does not have
suggestions_manage
3. Confirm that you can create a purchase suggestion if you have
suggestions_manage, but cannot access acquisition pages if you do not
have any subpermissions of the acquisition permission

I've tested the patch and here are my results...

- a patron with a super librarian permission can manage and add suggestion
- a patron without "suggestion management" permission cannot add suggestion



There is a tiny issue with the 3rd type of patron account...

- patron with several permissions (including "suggestion management") can add suggestion, but it also has the ability to "manage suggestions" (accept or reject suggestions) when you click on "Back to suggestion."


To reproduce:
Create a patron with "suggestion management" permission (see previous comment for details)
Apply the patch
Login with a patron who has the "suggestion management" permission
Find a patron account.
Click on "Purchase suggestion" and Add a suggestion.
After submitting the suggestion, click on the title of the suggestion to assess the request.
At the bottom of the suggestion form, click on "Back to suggestion"

After clicking on "Back to suggestion", you are sent to the "Suggestion Management" page (Acquisition > Suggestion), where you can accept or reject suggestions.

(In reply to Marie-Luce Laflamme from comment #21)
> There is a tiny issue with the 3rd type of patron account...
> 
> - patron with several permissions (including "suggestion management") can
> add suggestion, but it also has the ability to "manage suggestions" (accept
> or reject suggestions) when you click on "Back to suggestion."

Yes, it is the expected behavior.
With suggestions_manage you can manage suggestions :) Which means you can add/edit/delete purchase suggestions.

On this bug report we are dealing with acquisition access, a librarian with suggestions_manage but no acquisition permission cannot access budgets/funds information.

Personally, I think if a staff member can add a suggestion, they need to be able to edit or remove as well, but from the patron interface, not from the global table.  Less likely to affect the wrong patron.  That seems the most logical.  The global table needs its own permission.

I agree with Christopher and going off of my comment from above-- "In the past, staff were allowed staff to create purchase suggestions for patrons, but they could not edit/delete them."--We want this feature back.

Circ staff need to be able to enter the suggestions but not manage them--we have technical services staff that complete that task, not circulation staff.  The idea is to make it easier for the circ staff to help the patrons put in suggestions since most of the patrons don't log in to their accounts to do it themselves.

(In reply to karen.fellows from comment #24)
> I agree with Christopher and going off of my comment from above-- "In the
> past, staff were allowed staff to create purchase suggestions for patrons,
> but they could not edit/delete them."--We want this feature back.

I believe this is not true - the page was still accessible and staff could have added/edited anytime, just the acq module link would not show on the start page. We have used this 'loophole' for ages by linking to the page directly for libraries not using the acq module otherwise. There was NO permission check on the suggestions module page (apart from "catalogue").

I just tested this in a 17.11.:
- Create a user without any acq access
- Go to any patron account
- Create a new purchase suggestion
- Click on the "purchase suggestions" part of the breadcrumbs
- You can managed (edit/add) your heart out.

The regression is, that you can see other acq details, but not in the suggestions handling. 

> Circ staff need to be able to enter the suggestions but not manage them--we
> have technical services staff that complete that task, not circulation
> staff.  The idea is to make it easier for the circ staff to help the patrons
> put in suggestions since most of the patrons don't log in to their accounts
> to do it themselves.

I am not saying we could not make this more granular. The change here will even make that possible. By moving the suggestion permission to be a top-level permission, we will be able to have sub permissions as a next step. I am just saying, let's fix it to where we were before - then move forward from there.

Created attachment 97951 [details] [review]
Bug 22868: Move suggestions_manage subperm out of acquisition perm

Bug 11911 replaced the permission of suggestions.pl (create a purchase
suggestion) from catalogue => 1 to acquisition => 'suggestions_manage'.
However we have a lot of acquisition scripts that have lax permissions
(acquisition => '*' which means any sub permissions of acquisition is
enough).

That causes problem when a circulation staff can create purchase
suggestions but not access acquisition information.

One solution is to move the suggestions_manage subpermission out of the
acquisition permission and create a new suggestion permission.

Test plan:
0. Setup
* Create a patron with several permission (and full acquisition
permission)
* Create another patron with several permission, and suggestions_manage
permission
* Create another patron without the suggestions_manage permission
1. Apply the patch and execute the update database entry
2. Note that the third patron you create still does not have
suggestions_manage
3. Confirm that you can create a purchase suggestion if you have
suggestions_manage, but cannot access acquisition pages if you do not
have any subpermissions of the acquisition permission

Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>

Created attachment 98100 [details] [review]
Bug 22868: Move suggestions_manage subperm out of acquisition perm

Bug 11911 replaced the permission of suggestions.pl (create a purchase
suggestion) from catalogue => 1 to acquisition => 'suggestions_manage'.
However we have a lot of acquisition scripts that have lax permissions
(acquisition => '*' which means any sub permissions of acquisition is
enough).

That causes problem when a circulation staff can create purchase
suggestions but not access acquisition information.

One solution is to move the suggestions_manage subpermission out of the
acquisition permission and create a new suggestion permission.

Test plan:
0. Setup
* Create a patron with several permission (and full acquisition
permission)
* Create another patron with several permission, and suggestions_manage
permission
* Create another patron without the suggestions_manage permission
1. Apply the patch and execute the update database entry
2. Note that the third patron you create still does not have
suggestions_manage
3. Confirm that you can create a purchase suggestion if you have
suggestions_manage, but cannot access acquisition pages if you do not
have any subpermissions of the acquisition permission

Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

This patch will fix the regression found (see comments before), for further improving/splitting permissions, it would be great if separate bug reports could be reported.

There are small glitches:
- The top permission has one sub permission now. It might have been slightly more consistent to have it only as a top level module permission, but seeing requests here for more granular permissions, this might see more work later.
- The breadcrumbs show Acquisitions, but when you click on it, you got no permission. I am not seeing this as a blocker, as this is what we had before the regression was introduced. So I think we can think about this separately.

Nice work everyone!

Pushed to master for 20.05

Pushed to 19.11.x branch for 19.11.03

backported to 19.05.x for 19.05.08

Backported to 18.11.x for 18.11.14.

*** Bug 22402 has been marked as a duplicate of this bug. ***