No permission in permission.perm_list that give permission to override MAX_HOLDS

Bug #1320301 reported by Blake GH
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
In Progress
Undecided
Josh Stompro

Bug Description

When a patron is at max holds and someone is attempting to add another hold this error occurs:

"The patron has reached the maximum number of holds"

Currently the only way to override this is with the permission "Everything". The system needs MAX_HOLDS.override defined in permission.perm_list.

This is the string required as defined by PermitHold.pm line 211 where the LEGACY_HOLD_EVENT_MAP hash map defines 'config.hold_matrix_test.max_holds' => 'MAX_HOLDS'

I fixed this in our consortium:

insert into permission.perm_list(code,description)
values('MAX_HOLDS.override','Allows user to override the "The patron has reached the maximum number of holds"');

Revision history for this message
Mike Rylander (mrylander) wrote :

FWIW, for sites that lack but need this permission, you should be able to add it through the staff client under Admin -> Server Admin -> Permissions.

Revision history for this message
Blake GH (bmagic) wrote :
tags: added: pullrequest
Revision history for this message
Ben Shum (bshum) wrote :

Hmm, reviewed Blake's commit and have some thoughts...

1) In the upgrade script provided, he uses a hardcoded ID of 555 (but we've already gone past this number in stock seed data)

2) We need a change to the seed data file (Open-ILS/src/sql/Pg/950.data.seed-values.sql) to add the permission for fresh system installs. At which point in there, we'll define a proper ID for the new permission.

3) Do we need this permission applied to any staff groups by default (circ administrator, etc.?)

For the upgrade script, while I'm generally reluctant to specify an exact ID for the permissions, since these can and tend to be wildly variable in actual production systems, the expectation is that any custom permissions live above ID 1000, so anything under that is safe to insert. That said, we should add some protection in the upgrade script to fail peacefully if the permission was already added manually by the consortium in the past.

Marking back to "In Progress" and assigning to Blake for now to give this some more poking, but he may seek further assistance from folks with more DB upgrade script writing experience. And also, does this quality for a test plan and possible PGTap test?

Changed in evergreen:
assignee: nobody → Blake GH (blake-j)
status: New → In Progress
tags: removed: pullrequest
Revision history for this message
Blake GH (bmagic) wrote :

Ben,

Good point. I reworked it allowing it to get the next available ID number below 1000. I created the pgTAP test as well.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/blake/LP1320301

tags: added: pullrequest
Blake GH (bmagic)
Changed in evergreen:
assignee: Blake GH (blake-j) → nobody
tags: added: permissions
Changed in evergreen:
assignee: nobody → Josh Stompro (u-launchpad-stompro-org)
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.