Conversation
Amstutz
left a comment
Amstutz
left a comment
There was a problem hiding this comment.
Hi @okaufman
Thx for factoring out some of the new method logic. Did we not say we'd like make the roles having access selectable by the plugin configs screen? If not, at least, get the roles with permission from a new table in the DB to be prepared for this change. Further, see the inline comments.
Thx a lot.
| if (!in_array(2, self::dic()->rbacreview()->assignedGlobalRoles(self::dic()->user()->getId()))) { | ||
|
|
||
| if (!usrtoHelper::getInstance()->checkPluginAccess()) | ||
| // if (!in_array(2, self::dic()->rbacreview()->assignedGlobalRoles(self::dic()->user()->getId()))) |
There was a problem hiding this comment.
What ist this, why is it outcommented, -> probably remove.
classes/class.usrtoHelper.php
Outdated
| } | ||
| } | ||
|
|
||
| public function checkPluginAccess($usr_id=null){ |
There was a problem hiding this comment.
add return value to method signature, probably:
public function checkPluginAccess($usr_id=null): string
There was a problem hiding this comment.
did not listen to uncle bob ;-). Null as default probably wrong. Here especially. Always pass $usr_id to prevent side effects.
classes/class.usrtoHelper.php
Outdated
|
|
||
| } | ||
|
|
||
| protected function getRoleAllowed() |
There was a problem hiding this comment.
add return value method signature probably protected function getRoleAllowed() : int
classes/class.usrtoHelper.php
Outdated
|
|
||
| public function checkPluginAccess($usr_id=null){ | ||
| if (!isset($usr_id)) { | ||
| $usr_id = self::dic()->user()->getId(); |
There was a problem hiding this comment.
And here is the side effect, now this method depends on the state of self::dic()->user(), pass this from the outside always and get rid if this if here.
| { | ||
| // roles named UserTakeOver-User are allowed to use the plugin | ||
| if(self::dic()->rbacreview()->roleExists(self::USRTO_ROLE_NAME)){ | ||
| $roles= self::dic()->rbacreview()->getRolesByFilter(2,0, self::USRTO_ROLE_NAME); |
There was a problem hiding this comment.
Should we not make that selectable by the plugins config screen?
| if(self::dic()->rbacreview()->roleExists(self::USRTO_ROLE_NAME)){ | ||
| $roles= self::dic()->rbacreview()->getRolesByFilter(2,0, self::USRTO_ROLE_NAME); | ||
| } | ||
| return $roles[0]["obj_id"]; |
There was a problem hiding this comment.
Is admin not also allowed, where ist that set?
remove comments change function checkPluginAccess
2 participants
A User with the role "UserTakeOver-User" would be enabled to use the TakeOver Plugin