v6 compat update #5
Conversation
Had nothing to do with upgrade, but they should be able to see the link to the files, just not upload.
Language tweaks for roles.
update version number
|
@intoeetive can you take a look at this one? I tested until I got a headache, and I think the permissions are good, even with multiple roles. But it could definitely use a a really good look over. Thanks! |
There was a problem hiding this comment.
@robinsowell I did not test, just looked through the code. I think all is good mostly, however I posted few questions.
| if ($this->has_role(array(2, 3, 4), TRUE)) { | ||
| $this->can_upload = 'n'; | ||
| } |
There was a problem hiding this comment.
I'm not sure we need to force no access here with the new permissions system. Unless we need this special restriction (e.g. to keep current behaviour)
There was a problem hiding this comment.
I wasn't sure either, I just kept it because it was existing and I was making minimal changes.
| // update for v6 return primary member roles | ||
|
|
||
| $this->member_groups = ee('Model')->get('Role') | ||
| ->filter('role_id', 'NOT IN', array('2', '3', '4')) |
There was a problem hiding this comment.
Is role 1 removed here intentionally?
There was a problem hiding this comment.
It really should exclude superadmin (this is for populating the selects)- but see #6
So I left superadmin in there- they probably shouldn't be and if you don't select them it's moot. But this way, you can still unselect all of the non-super.
I left it for now, but when that's fixed, they should be dropped. Not elegant, but functional, and I wasn't going to redo that display right now.
| @@ -611,6 +605,42 @@ function __construct($return = FALSE) | |||
| $this->return_data = $this->_deny_if('redirect_page', $this->return_data); | |||
|
|
|||
| } | |||
|
|
|||
|
|
|||
| public function has_role($roles, $strict = FALSE) | |||
There was a problem hiding this comment.
Is there specific reason to have this function written instead of reusing ee('Permission')->hasRole($role_id)?
There was a problem hiding this comment.
because I did not realize there was a hasRole, lol! Yea, let me rewrite that, because it's annoying.
and a php 7 tweak.
First draft, needs a good test, especially on permissions.