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

add andFilterWhere condition #35

Closed
albertborsos opened this issue Sep 26, 2019 · 3 comments
Closed

add andFilterWhere condition #35

albertborsos opened this issue Sep 26, 2019 · 3 comments
Labels
status:wontfix This will not be worked on type:enhancement Enhancement. type:question Further information is requested

Comments

@albertborsos
Copy link
Contributor

It would be a nice feature to write code like this:

// grid filtering conditions
$select->andFilterWhere('code', 'like', $entity->code);
$select->andFilterWhere('name', 'like', $entity->name);

instead of this:

// grid filtering conditions
if ($entity->code !== null) {
    $select->andWhere('code', 'like', $entity->code);
}
if ($entity->name !== null) {
    $select->andWhere('name', 'like', $entity->name);
}

I tried to implement it, but I do not really understand yet how it works.

@genhoi
Copy link

genhoi commented Sep 26, 2019

You can do this with custom repositories

use Cycle\ORM\Select\Repository;

class UserRepository extends Repository
{

    public function withFilterWhere($column, $operator, $value)
    {
        $r = $this;
        if ($value !== null) {

            $r = clone $this;
            $r->select->where($column, $operator, $value);
        }

        return $r;
    }
}

Sample usage

$code = 1;
$name = 'cat';

/** @var UserRepository $repo */
$repo = $this->orm->getRepository(User::class);

$users = $repo
    ->withFilterWhere('code', 'like', $code)
    ->withFilterWhere('name', 'like', $name)
    ->findAll();

https://github.com/cycle/docs/blob/master/basic/repository.md
https://github.com/cycle/docs/blob/master/advanced/chained-repository.md

@wolfy-j
Copy link
Contributor

wolfy-j commented Sep 27, 2019

Hi @albertborsos

I'm against adding this feature for 3 main reasons:

  1. The method is confusing, should it skip condition when the value is null/false/empty string?
  2. We are saving 2 lines of code but introducing a new concept of optional conditions, this will make code maintenance harder down the road.
  3. As @genhoi it's much easier to add custom filter logic to repositories (which they are intended for).

Thanks for your suggestion, I'm open to change my opinion if more arguments will surface, this ticket will remain open for the discussion for the next few months.

@wolfy-j wolfy-j added status:wontfix This will not be worked on type:enhancement Enhancement. type:question Further information is requested labels Sep 27, 2019
@albertborsos
Copy link
Contributor Author

Thanks! This solution is good enough for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:wontfix This will not be worked on type:enhancement Enhancement. type:question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants