yiisoft / yii2-debug

Debug Extension for Yii 2
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
201 stars 149 forks source link

Changed return statement to false in init #438

Closed nilsburg closed 4 years ago

nilsburg commented 4 years ago

Currently if you want to extend UserPanel and implement a custom init method UserPanel::init() blocks the execution if the user is not logged or panel is disabled without being able to check if UserPanel::init is returning true or false.

By changing the return statement to 'false' you can check where it is passing or not without blocking the execution.

Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Tests pass? ✔️
Fixed issues comma-separated list of tickets # fixed by the PR, if any
samdark commented 4 years ago

Would you please post an example of a class extending it you have problem with?

nilsburg commented 4 years ago

Maybe my approach is not the right one, but here is what I am trying to do. In the Switch view of the UserPanel I want to add a column 'Role' and add a filter with a dropdown that is filled with results from a query. I can't to this in the config of the app since the DB is not loaded at that moment, so I need to extent UserPanel and add the column there:

class UserPanel extends \yii\debug\panels\UserPanel
{
    public function init()
    {
        parent::init();
        $this->filterColumns = [
            'user_id',
            'user',
            [
                'attribute'=>'role',
                'label'=>'Role',
                'filter'    => Html::activeDropDownList($this->filterModel, 'role', Roles::dropdown(), ['prompt' => 'Select', 'class'=>'form-control']),
            ]
        ];
    }
}

The problem is that when the user is not logged in, parent::init() returns nothing and the rest of the code in there is not executed, so the property filterModel is not configured. By changing the return value to false in UserPanel::init() I can do the following:

class UserPanel extends \yii\debug\panels\UserPanel
{
    public function init()
    {
        if(parent::init()){
            $this->filterColumns = [
                'user_id',
                'user',
                [
                    'attribute'=>'role',
                    'label'=>'Role',
                    'filter'    => Html::activeDropDownList($this->filterModel, 'role', Roles::dropdown(), ['prompt' => 'Select', 'class'=>'form-control']),
                ]
            ];
        }
    }
}

Without changing the return statement to false in UserPanel::init() it will enter the condition and will throw an error because filterModel is not configured

samdark commented 4 years ago

"return nothing" actually returns null and the condition if(parent::init()){ would work exactly the same way as with false. Are you sure the problem is not elsewhere?

nilsburg commented 4 years ago

Because there is no other return statement in UserPanel::init(), it always returns null, so there is no way to check if it entered the first condition in UserPanel::init(). Another solution would be adding a return true at the end of UserPanel::init().

/**
     * {@inheritdoc}
     * @throws InvalidConfigException
     */
    public function init()
    {
        if (!$this->isEnabled() || $this->getUser()->isGuest) {
            return;
        }

        $this->userSwitch = new UserSwitch(['userComponent' => $this->userComponent]);
        $this->addAccessRules();

        if (!is_object($this->filterModel)
            && class_exists($this->filterModel)
            && in_array('yii\debug\models\search\UserSearchInterface', class_implements($this->filterModel), true)
        ) {
            $this->filterModel = new $this->filterModel;
        } elseif ($this->getUser() && $this->getUser()->identityClass) {
            if (is_subclass_of($this->getUser()->identityClass, 'yii\db\ActiveRecord')) {
                $this->filterModel = new \yii\debug\models\search\User();
            }
        }
        return true;
    }
samdark commented 4 years ago

Wouldn't $this->userSwitch !== null check work?

samdark commented 4 years ago

We should not add return to init() since the interface doesn't return anything.

nilsburg commented 4 years ago

Ah yes, didn't see that. That works. Thanks.