Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Update framework/web/auth/CAccessControlFilter.php #342

Closed
wants to merge 8 commits into
from

Conversation

Projects
None yet
3 participants

It would be nice to pass params to accessControl so that bizRules works.

I would like to do this in conroller:

public function accessRules() {

        return array(

        array('allow',
                'actions'=>array('update'),
                'roles'=>array('updateActivity'=>array('activity'=>$this->activity)),
        ),


        array('deny',
                'users'=>array('*'),
        ),
        );
    }

with

$bizRule='return ($params["activity"])?$params["activity"]->belongsToLogedUser():false;';
$task=$this->_authManager->createTask('updateOwnActivity','update a activity album by author himself',$bizRule);
$task->addChild('updateActivity');
Owner

qiangxue commented Feb 16, 2012

How will you get $this->activity when calling checkAccess?

Owner

samdark commented Feb 17, 2012

This is for enhancement #341. It was discussed there already.

Owner

qiangxue commented Feb 17, 2012

The usage needs to be documented clearly.

I would like to document it, but I don't know how and my english is very bad.

Owner

samdark commented Feb 17, 2012

Try it. We should get your idea before making a decision. We'll fix English in the end anyway.

Ok but how and where should I do it

Owner

samdark commented Feb 20, 2012

I think the right place is CAccessControlFilter class phpdoc.

Owner

qiangxue commented Feb 20, 2012

Imagine another people wants to make use of this new feature, how would you like him to know about it? You definitely don't want him to read your code to find the usage. So as samdark said, in the minimum you should comment in CAccessControlFilter, especially about accessRules.

Owner

qiangxue commented Feb 20, 2012

Note that a framework is not just about code, documentation perhaps takes more than 50% of it.

Besides, we should also document this enhancement in CHANGELOG. Thanks.

Is it ok like that?

@ghost ghost assigned samdark Feb 21, 2012

Thanks samdark. I put your comments into my patch. Is there something else I must do to merge with the master? Or do you do that?

Owner

samdark commented Feb 21, 2012

@claudejanz if you want to help you can update your repo to latest version, merge it and push to your request branch. This way I'll be able to merge automatically.

samdark added a commit that referenced this pull request Jul 1, 2012

Owner

samdark commented Jul 1, 2012

Merged with some modifications.

@samdark samdark closed this Jul 1, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment