-
Notifications
You must be signed in to change notification settings - Fork 43
Add option for multiple INI files, add AuthUser component and helper #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Please feel free to give feedback! I will merge the develop already in master for now to allow further improvements there then. |
saeideng
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for added this changes
| return $roles; | ||
| } | ||
|
|
||
| return $roles; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference between this and above return?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method got refactored afterwards already.
| $controller = $event->subject(); | ||
|
|
||
| $authUser = (array)$this->user(); | ||
| $controller->set('_authUser', $authUser); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we wrote own beforeRender Event in our app area?
Do we need to write (set _authUser) in our app?
| * @param array $options | ||
| * @return string | ||
| */ | ||
| protected function _default($title, $options) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing docs
| ]); | ||
|
|
||
| $user = ['id' => 1, 'role_id' => 999]; | ||
| $user = ['id' => 1, 'role_id' => 1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
below assertTrue() trace the "id" in this sample, and not role_id ,even we can remove it(role_id)
999 means we can use any value for it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to clean things up a bit with a PR. Much appreciated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
returned Cannot find role alias for role 999
see below comment
| $alias = array_keys($availableRoles, $role); | ||
| $alias = array_shift($alias); | ||
| if (!$alias || !is_string($alias)) { | ||
| throw new RuntimeException('Cannot find role alias for role ' . $role); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm worried about this
we would just return false for using in cakephp auth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worried in what way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably this make BC issue (i'm not sure now)
maybe some one needs to use just super admin without roles
and maybe some another usage of this plugin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened PR #46
Added:
Also:
Can be released as 1.6
TODO: