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

Updates for Search-Form Model. #61

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

githubjeka
Copy link
Contributor

Now search model will not inherit Active Record model.
This allows you to get rid of the duct tape method scenarios().
And teaches the right way of using models and active records.

Generated example for common\models\User.

<?php

namespace backend\models; 

use Yii; 
use yii\base\Model; 
use yii\data\ActiveDataProvider; 
use common\models\User; 

/** 
 * UserSearch represents the model behind the search form about `common\models\User`. 
 */ 
class UserSearch extends Model 
{ 
    /** 
    * @var integer 
    */ 
    public $id; 
    /** 
    * @var string 
    */ 
    public $username; 
    /** 
    * @var string 
    */ 
    public $auth_key; 
    /** 
    * @var string 
    */ 
    public $password_hash; 
    /** 
    * @var string 
    */ 
    public $password_reset_token; 
    /** 
    * @var string 
    */ 
    public $email; 
    /** 
    * @var integer 
    */ 
    public $status; 
    /** 
    * @var integer 
    */ 
    public $created_at; 
    /** 
    * @var integer 
    */ 
    public $updated_at; 
    /** 
    * @var string 
    */ 
    public $description; 
    /** 
    * @var integer 
    */ 
    public $signin_at;    
    /** 
    * @var integer 
    */ 
    public $activity_at; 


    /** 
     * @inheritdoc 
     */ 
    public function rules() 
    { 
        return [ 
            [['id', 'status', 'created_at', 'updated_at', 'signin_at', 'activity_at'], 'integer'],
            [['username', 'auth_key', 'password_hash', 'password_reset_token', 'email', 'description'], 'safe'],
        ]; 
    } 

    /** 
     * Creates data provider instance with search query applied 
     * 
     * @param array $params 
     * 
     * @return ActiveDataProvider 
     */ 
    public function search($params) 
    { 
        $query = User::find(); 

        $dataProvider = new ActiveDataProvider([ 
            'query' => $query, 
        ]); 

        $this->load($params); 

        if (!$this->validate()) { 
            // uncomment the following line if you do not want to return any records when validation fails
            // $query->where('0=1'); 
            return $dataProvider; 
        } 

        $query->andFilterWhere([
            'id' => $this->id,
            'status' => $this->status,
            'created_at' => $this->created_at,
            'updated_at' => $this->updated_at,
            'signin_at' => $this->signin_at,
            'activity_at' => $this->activity_at,
        ]);

        $query->andFilterWhere(['like', 'username', $this->username])
            ->andFilterWhere(['like', 'auth_key', $this->auth_key])
            ->andFilterWhere(['like', 'password_hash', $this->password_hash])
            ->andFilterWhere(['like', 'password_reset_token', $this->password_reset_token])
            ->andFilterWhere(['like', 'email', $this->email])
            ->andFilterWhere(['like', 'description', $this->description])

        return $dataProvider; 
    } 
} 

Now search model not inherit Active Record model.
This allows you to get rid of the duct tape method `scenarios()`.
And taught the right way of using models and active records.
@samdark
Copy link
Member

samdark commented Jun 24, 2015

I'm all for merging it. @yiisoft/core-developers opinions?

@samdark samdark added the type:enhancement Enhancement label Jun 24, 2015
@samdark samdark self-assigned this Jun 24, 2015
<?php foreach ($searchAttributes as $attribute) : ?>/**
* @var <?= ($tableSchema !== false) ? $tableSchema->columns[$attribute]->phpType : 'mixed'?>

*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation is missing one space. should be 5 spaces before the * instead of 4. line 40 and also line 38.

@cebe
Copy link
Member

cebe commented Jun 24, 2015

in general I support this change but I am not sure if this counts as a BC break. An application may use the old structure and may need to add new search models while adding more functionality after gii update. This causes confusion/inconsistency in the app code.

@githubjeka
Copy link
Contributor Author

@cebe fixed lost space. Thank.

About BC I agree, but better late than never. Since the current implementation hit me.

@cebe
Copy link
Member

cebe commented Jun 24, 2015

maybe we can keep this open for 2.1 and recommend using custom template for now?

@@ -32,9 +32,16 @@
/**
* <?= $searchModelClass ?> represents the model behind the search form about `<?= $generator->modelClass ?>`.
*/
class <?= $searchModelClass ?> extends <?= isset($modelAlias) ? $modelAlias : $modelClass ?>

class <?= $searchModelClass ?> extends Model
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extending original CRUD model class, while creating a search model is quite widely used practice.
It allows usage of the labels and hints from the original model without necessity of redefining them.

Although extending plain Model in this case may seem to be more consistent, i suppose it should be an option.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this at all. I use a lot of relations on my model searches.

Example if I have a sales model and I need to include how many items and the total of each sale, I need to do something like this

$parentClass = get_parent_class(); // we can discuss about this later, ignore it for now()
$query = $parentClass::find()->joinWith(['items', 'store.direction', 'customer'])->select([
    $parentClass::tableName() . '.*',
    'itemNumber' => 'count(disticnt item.id)',
    'totalSale' => 'sum(item.cost)',
]);

where I have a methods getItemNumber(), setItemNumber(), getTotalSale() and setTotalSale() defined on the Sales active record. With this now i have to define that all over. Also other type of validations and validations such as behaviors that I used for a model. Example: set full name property of a user on the after find based on the attributes last name and first name from the db table.

@samdark samdark added this to the 2.1.0 milestone Jun 24, 2015
@cebe
Copy link
Member

cebe commented Jun 24, 2015

I also just noticed that this change conflicts with the description of searching for related data in the guide: http://www.yiiframework.com/doc-2.0/guide-output-data-widgets.html#working-with-model-relations you can not easily define attributes that contain a "." in plain Model.

@lav45
Copy link
Contributor

lav45 commented Jun 25, 2015

@githubjeka 👍

@samdark
Copy link
Member

samdark commented Jun 25, 2015

Right. So there are cons and these are significant.

@lav45
Copy link
Contributor

lav45 commented Jun 25, 2015

In this case there is need to use the validator string instead of safe

@lynicidn
Copy link

and filter created_at and updated_at need as interval

@lav45
Copy link
Contributor

lav45 commented Jun 25, 2015

@cebe, not in plain Model, an instance of the class ActiveDataProvider
I just checked and I have everything working perfectly

    <?= GridView::widget([
        'dataProvider' => $dataProvider,
        'filterModel' => $searchModel,
        'columns' => [
            'username',
            'profile.email',
            [ // Display RBAC role description
                'label' => 'Роль',
                'attribute' => 'role',
                'value' => 'itemNames.0.description',
                'filter' => User::getRoleLest(),
            ],
        ],
    ]); ?>

@cebe
Copy link
Member

cebe commented Jun 25, 2015

@lav45 how did you manage to filter by profile.email using plain Model?

@lav45
Copy link
Contributor

lav45 commented Jun 26, 2015

@cebe
image

class UserSearch extends Model
{
    // ...
    public $email;

    public function rules()
    {
        return [
            // ...
            [['email'], 'string'],
        ];
    }

    public function search($params)
    {
        $query = User::find()
            ->joinWith(['itemNames', 'profile'])
            ->where(['status' => User::STATUS_ACTIVE]);

        $dataProvider = new ActiveDataProvider([
            'query' => $query,
        ]);

        $dataProvider->sort->attributes['email'] = [
            'asc' => ['profile.email' => SORT_ASC],
            'desc' => ['profile.email' => SORT_DESC],
        ];

        if (!($this->load($params) && $this->validate())) {
            return $dataProvider;
        }

        $query
            ->andFilterWhere(['like', 'email', $this->email])
            ->andFilterWhere(['like', 'username', $this->username])
            ->andFilterWhere(['item_name' => $this->role]);

        return $dataProvider;
    }
}
    <?= GridView::widget([
        'dataProvider' => $dataProvider,
        'filterModel' => $searchModel,
        'columns' => [
            'username',
            //'profile.email',
            [
                'attribute' => 'email',
                'value' => 'profile.email',
            ],
            [ // Display RBAC role description
                'label' => 'Роль',
                'attribute' => 'role',
                'value' => 'itemNames.0.description',
                'filter' => User::getRoleLest(),
            ],
        ],
    ]); ?>

@cebe
Copy link
Member

cebe commented Jun 26, 2015

great, thanks for sharing this.

@lynicidn
Copy link

i also paste my variant

<?php

namespace common\base;

use Yii;
use yii\base\Model;

/**
 * Class SearchModel
 * @package common\base
 */
abstract class SearchModel extends Model
{
    /**
     * @var bool|array|\yii\data\Sort
     */
    public $sort;

    /**
     * @var bool|array|\yii\data\Pagination
     */
    public $pagination;

    /**
     * @var \yii\db\ActiveQuery
     */
    public $query;

    /**
     * @inheritdoc
     */
    public function __construct(\yii\db\ActiveQueryInterface $query = null, $config = [])
    {
        $this->query = $query;
        parent::__construct($config);
    }

    /**
     * @inheritdoc
     */
    public function init()
    {
        parent::init();
        if (!$this->query) {
            $this->query = $this->defaultQuery();
        }
    }

    /**
     * @return \yii\db\ActiveQuery
     */
    abstract public function defaultQuery();

    /**
     * Creates data provider instance with search query applied
     * @param array $params
     * @param null|string $formName
     * @return \yii\data\ActiveDataProvider
     */
    public function search($params = [], $formName = null)
    {
        $query = $this->createQuery();
        if ($this instanceof \yii\base\Model && $this->load($params) && $this->validate()) {
            /** @var self $this*/
            $query->filterWhere($this->filters());
        }

        return $this->createDataProvider($query);
    }

    /**
     * @return \yii\db\ActiveQuery
     */
    protected function createQuery()
    {
        return clone $this->query;
    }

    /**
     * @param \yii\db\ActiveQuery $query
     * @return \yii\data\ActiveDataProvider
     */
    protected function createDataProvider($query)
    {
        $config = ['class' => 'yii\data\ActiveDataProvider', 'query' => $query];
        if ($this->sort !== null) {
            $config['sort'] = $this->sort;
        }
        if ($this->pagination !== null) {
            $config['pagination'] = $this->pagination;
        }

        return Yii::createObject($config);
    }

    /**
     * @return array
     */
    public function filters()
    {
        return [];
    }
}

and example of search model

<?php

namespace backend\models;

use Yii;
use common\base\SearchModel;
use common\models\Ticket;

/**
 * TicketSearch represents the model behind the search form about `common\models\Ticket`.
 */
class TicketSearch extends SearchModel
{
    public $sort = false;

    public $text;
    public $department;
    public $status;

    /**
     * @inheritdoc
     */
    public function defaultQuery()
    {
        return Ticket::find();
    }

    /**
     * @inheritdoc
     */
    public function rules()
    {
        return [
            ['department', 'in', 'range' => array_keys($this->departmentLabels())],
            ['status', 'in', 'range' => array_keys($this->statusLabels())],
            ['text', 'safe'],
        ];
    }

    /**
     * @inheritdoc
     */
    public function filters()
    {
        return ['and',
            [
                'status' => $this->status,
                'department' => $this->department,
            ],
            ['or',
                ['alt' => $this->text],
                ['contact_jabber' => $this->text],
                ['contact_email' => $this->text],
                ['contact_icq' => $this->text],
                ['token' => $this->text],
                ['like', 'subject', $this->text],
//                ['like', 'message', $this->text],
            ]
        ];
    }

    /**
     * Labels for select tag of form
     * @return array
     */
    public static function departmentLabels()
    {
        return Ticket::departmentLabels();
    }

    /**
     * Labels for select tag of form
     * @return array
     */
    public static function statusLabels()
    {
        return Ticket::statusLabels();
    }

    /**
     * @inheritdoc
     */
    protected function createQuery()
    {
        $query = parent::createQuery()
            ->joinWith(['lastReply' => function (\yii\db\ActiveQuery $query) {
                $query->orWhere(['lastReply.created_at' => null]);
            }])
            ->orderBy([
                'ticket.status' => SORT_DESC,
                'IF(lastReply.created_at IS NULL, ticket.created_at, lastReply.created_at)' => SORT_DESC,
//                'lastReply.created_at' => SORT_DESC,
            ]);

        return $query;
    }
}

query in constructor for oop style. If we can use relation as query we can use it as

$search = new UserSearch ($adminRole->getRoles());
where getRoles return ActiveRelationTrait

@lav45
Copy link
Contributor

lav45 commented Jun 26, 2015

@lynicidn I think you better create a new issue

@lynicidn
Copy link

@lav45 it not issue it my variant solve

@creocoder
Copy link
Contributor

Great suggestion, vote to merge this!

@Faryshta
Copy link
Contributor

question. why can't we have the search as an scenario of the original model?

as far as i see it it would just be adding an 'except' => 'search' for the required rules. Am I missing anything?

@lav45
Copy link
Contributor

lav45 commented Jul 15, 2015

@Faryshta, Scenarios only need to edit data, and search nothing should be written, that it is proposed to inherit the Search model from the parent class, and Model. Thus the class Search is the ordinary form which receives and validates the data and nothing or where no records.

@lynicidn
Copy link

@lav45 i use scenarion in my search model.

  1. find all book collections (only shared)
  2. find my collections (shared and privat)
    and scenarios deny mass assign attributes as user_id in search 2 or shared in search 1
    =)

@Faryshta
Copy link
Contributor

@lav45 i disagree with your statement that scenarios are meant for data edition.

The most common example for using scenarios is user login and you don't need to edit the user when it logs in, actually thats the entire point of using an scenario in that case.

@lav45
Copy link
Contributor

lav45 commented Jul 16, 2015

@Faryshta, I understand you about this login form is now trying to tell, but there are no scenarios.

@Faryshta
Copy link
Contributor

@lav45
Copy link
Contributor

lav45 commented Aug 28, 2017

@Faryshta SCENARIO is a good tool for small tasks. But we are talking about Composite pattern

@samdark samdark removed this from the 3.0.0 milestone Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants