Skip to content
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

is_allowed improvement #166

Closed
stupidoes opened this issue Jul 26, 2016 · 5 comments
Closed

is_allowed improvement #166

stupidoes opened this issue Jul 26, 2016 · 5 comments

Comments

@stupidoes
Copy link

i found dead code here :

if( $user_id===FALSE){
    return $this->is_group_allowed($perm_id);
} 

since $user_id is already set to get from session above. Even when user not logged in, $user_id would become NULL instead.

if( $user_id == FALSE){
    $user_id = $this->CI->session->userdata('id');
}

and i think, it's better to put 'break' when found permission inside group since it wouldn't need to check another group for the same permission.

foreach( $this->get_user_groups($user_id) as $group ){
    if ( $this->is_group_allowed($perm_id, $group->id) ){
        $g_allowed=TRUE;
        break;
    }
}
@REJack
Copy link
Collaborator

REJack commented Jul 26, 2016

Hi @stupidoes,

in my opinion the first 2 codeblocks are not dead code, only this can check perms for the public group, if we delete this then the public group is useless.

the idea with the break is good, i change it later and create a new release for this.

@stupidoes
Copy link
Author

Could you provide me with case when $user_id is still in FALSE value when entering the condition ? Because i think it will become $this->CI->session->userdata('id') value or NULL if user not logged in.

@REJack
Copy link
Collaborator

REJack commented Jul 27, 2016

😵 sorry you are right the "dead code" is really dead and its not needed too.

I change it in the next days.

@stupidoes
Copy link
Author

Thanks.. :)

@REJack REJack mentioned this issue Nov 5, 2016
REJack added a commit that referenced this issue Nov 6, 2016
@REJack
Copy link
Collaborator

REJack commented Nov 6, 2016

I close this issue 😄
Enhancement is released with v2.5.12

@REJack REJack closed this as completed Nov 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants