Closed (fixed)
Project:
Translation Management Tool
Version:
7.x-1.x-dev
Component:
Core
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
9 Jan 2013 at 10:56 UTC
Updated:
6 Mar 2014 at 08:30 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
blueminds commentedAdded #access to the delete button at the job checkout page.
About the VBO action - not sure here. I replaced the generic VBO delete action with rules action. But we are not able to define that a specific rules action should consider "administer tmgmt" permissions. So for the moment the Delete Job action is listed, it even will say that all deleted successfully, but it won't for a user without administer tmgmt permissions. This is completely wrong UX, but at least jobs will not get deleted by someone that is not supposed to delete them.
Anyway, any suggestions how to make the UX better are welcome.
Comment #2
berdirOk, so views has built-in entity access, have a look at _views_bulk_operations_entity_access().
There it checks the access mask of an action, which can control what of view/edit/delete is required to execute it.
The problem is that the rules integration (ViewsBulkOperationsRulesComponent) has only view hardcoded, and a custom access() method, that in turn only supports rules_action()->access().
Which leads to RulesAbstractPlugin::access(), which apparently supports an access callback, I'm not exactly sure if $info there is what you define in hook_rules_action_info(), but it might be :)
Worth a try...
Comment #3
blueminds commentedYes, that worked.
Comment #4
berdirYes, confirmed that this is working well.
Not sure how far we want to go here.
a) Should we apply this pattern to the other default rules too? You can view the overview without having permissions for submitting jobs, so the other actions should be hidden too then.
b) We also have default rules for tmgmt_local, which currently expose a custom permission for access control, but that's ugly. We could clean this up by adding access callbacks there too. What's sad is that there's no access arguments option, so we have to add a callback for every different permission check.
c) Test coverage. I don't think we have much test coverage right now, probably a bit for the local translator but I guess nothing for the overview. I think at least doing a permission check for the existence would be useful here.
Miro?
Comment #5
berdirOk, we discussed this. Let's do this:
1. Add an access callback for checkout and cancel to check for entity access submit, can be the same function I think.
2. Do the same for the local task rules/actions, remove the permission flag from the default rules.
3. Add some basic access tests. Visit the overview with all access combinations verify what operations are visible ( only view => no operations, submit => checkout and cancel, => administer (only that permission) => all). Make sure to always assert for visible and not visible.
Don't add tests for actually executing those operations, unless we already have them in the local task tests, then update.
Comment #6
miro_dietiker:-)
Comment #7
blueminds commentedAccess callback for rules implemented. One fail is expected - for some reason user with "provide translation services" does not get the right to assign task to him. In case of uid1 it works so it has to do with permissions, but was not able to find what. Any ideas?
Comment #9
berdirOk, this took me a while.
The trick is to not define our own access callback, but alter the generated component action for our default component, so that we can circumvent the default rules access logic completely. This is also the only option that allows us to differentiate between the assign to me and assign component.
Comment #10
blueminds commentedlooks good
Comment #11
berdirAh, actually, this allows us to solve one of the @todo's in the test. This overlaps a bit with the access test coverage but whatever ;)
Comment #12
berdirCommitted and pushed.