Feature/Discussion: Scopes

[anther]

I want to start a discussion on adding scopes to PHPActiveRecord!

This is a branch that I started working on earlier this year after discovering GH-252 by @CarlosBonetti. He began work on what I thought was a very good idea but a pretty rough implementation of scoping features. I've been tweaking and adding to it to the point that I've used this implementation on several live web-apps and feel that this would add a lot of value to PHPActiveRecord and be very nice to not only have in my home version ;p.

I've since updated his code to have a much smoother and slightly less intrusive interface with pre-existing phpactiverecord code. I've also put some checks in place so that it currently won't override any behavior of previous code if scopes are not used at all, thus in theory all previous code behaves the same.

There are still some rough edge cases in the implementation right now but they're all relatively easy to iron out, as rough edges tend to be :p.

I'm going to pull these examples from code we currently have active. I'm also pretty sure the majority of you know what scopes are and how they work but here's an example of this particular implementation in practice.

class Company extends BaseModel
{

     public static function getAllAgencies()
    {

        return self::scoped()->where('company_type = ?', AGENCY_COMPANY_TYPE)->order('company_name ASC');
    }

    public static function nameSearchCompanies($companyName)
    {
        return self::scoped()->where('company_name LIKE ?', "%$companyName%");
    }

    public static function nameSearchValidCompanies($companyName)
    {
        return self::validCompanies()->nameSearchCompanies($companyName);
    }

  public static function validCompanies()
    {

        return self::scoped()->where('dead = ?', 0)->order('company_name ASC');
    }

    public static function nameSearchValidAgencies($companyName)
    {
        return self::validCompanies()->where(array('company_type' => AGENCY_COMPANY_TYPE))->nameSearchCompanies($companyName);
    }

    public static function nameSearchRepCompanies($companyName){
        return  self::repCompanies()->nameSearchCompanies($companyName);
    }

    public static function repCompanies(){
        $scope = self::validCompanies();
        if (!restrictedPermissionsStuff()) {
            $scope->where("(
                            CID IN (
                                SELECT DISTINCT CID FROM contacts AS c INNER JOIN contact_reps AS cr ON cr.iid=c.IID WHERE uid='".$_SESSION['userid']."'
                                )
                            OR 
                            CID IN (
                                SELECT DISTINCT CID FROM company WHERE company_primary_rep = '".$_SESSION['userid']."'
                            )
                        )");
        }
        return $scope;
    }

}

#You can then do all sorts of cool flexible stuff like
Company::nameSearchValidAgencies('Aristocrats')->all();
Company::nameSearchCompanies('Aristocrats')->all(array('conditions'=>array('id'='15')));
Company::nameSearchCompanies('Aristocrats')->limit(5)->all();

I really want input and some other programmer talents to jump into this because I feel it's very close to being ready to integrate fully and after some scrutiny this can possibly even be added as a feature to the current active build (As it's backwards compatible at the moment).

What do you guys think??

[Rican7]

I really like this idea/implementation. Maybe the API could be a little cleaner, but I love the idea of chaining scope calls. I'd love to hear what others have to say.

[CarlosBonetti]

In my opinion, scopes aren't just a feature to php-activerecord, it's a necessity and many people are waiting for this for long time (me included). But it's not a simple thing to implement. When I wrote the first lines it was to supply my own needs and I decided pushing here to boost the idea.

Thanks @anther for keeping the job and I really expect the idea grow up and become a merged code (after revising and discuss, of course).

[anther]

I can see an improvement would be to update the API to allow return static::scoped($options_array). That way everyone can directly avoid needing to use the "new" syntax with ->where, ->limit, ->from, ->..., as they're essentially just wrappers to those options.

[al-the-x]

I like @anther's current suggestion. It's introducing a brand new feature and breaking an API to build scopes into the current contract, but ppl should be able to opt-in to the feature until it becomes the standard... Perhaps in 2.0?

[gsterjov]

It looks like chaining two scopes that do a join will only use the last join. I'm guessing they aren't being appended like the where clauses. I'll try hacking in some support for multiple joins if I can.

The feature is looking good though, serving me well :+1:

[anther]

@gsterjov Completely an oversight and it makes sense for joins to be appended. Glad the rest of it is working well for you though :).

@al-the-x Is the breaking of the contract adding the static scoped() method? So are you saying that it'd make sense to add a configuration option such as 'use_scopes'=>true, and then at that point the check for the usage of scopes.

[al-the-x]

@anther Yes, that's what I was referring to. The scoped() method gets around the API change nicely... Maybe scope() instead though? Shouldn't that return a specific subclass with scoped versions of the Query API methods to avoid confusion...?

[gsterjov]

What about having the joins resolve itself much like how the 'joins' option currently does? This way we can do ->joins(array('author', 'publisher')) since its already defined as a relationship.

[anther]

As an update, I noticed that I would frequently call scopes with

foreach(Company::validCompanies()->all() as $company)
{
//Things
}

So I added the IteratorAggregate interface to Scope. This even made sense in the case of refactoring more code to use scopes that were previously hard-coded finder functions. Say you had a method that looked like this:

public static function getAllHappyPeople()
{
   return People::all(array('conditions'=>array('happy'=>true));
}

It can now be replaced with the following and have minimal chance of breaking previous code... *kind of!

public static function getAllHappyPeople()
{
   return People::scoped()->where('happy'=>true));
}

You can then simply do

/* no longer need ->all(), as it's called implicitly*/
foreach(Company::validCompanies() as $company)
{
//Things
}

*!People::getAllHappyPeople() Will not return true anymore, since it's returning a scope instance instead of an an array until you attempt to iterate over it.

[Rican7]

I haven't been keeping up with this at all. What's the stability like? Have you had any problems with this feature in your fork?

[koenpunt]

I like the concept of chaining conditions, but I don't like the ::scope() method as api.

What if we add ::where(), ::limit(), ::order() to Model, and you can use them all to start a chain/scope.

Book::where(['author' => 'Some Name']); // => returns a Scope
Book::where(['author' => 'Some Name'])->order(['publication_year' => '2013']); // => returns a Scope

Methods in Scope could be:

• where
• limit
• order
• unscope: remove certain conditions from scope
• reorder: override an order defined on a association
• reverse_order
• rewhere: override where defined on a association
• only: the only conditions to use

Where Scope itself would implement Iterator, or IteratorAggregate

[IllyaMoskvin]

I think this doesn't handle scoped include options correctly when passing several primary ids via conditions. Everything works well if I set include explicitly right before a ::find() or ::count() instead of inside the model definition using default_scope(). It throws the following error: ActiveRecord\DatabaseException: exception 'PDOException' with message 'SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens' in C:\wamp\www\...\php-activerecord\lib\Connection.php:324

I found the problem in OptionBinder.php inside the where() function. It was not accounting for the possibility that $value might be an array. Apologies, I am not well-versed in how gits work just yet, so I cannot add a commit to this pull request. Line 96 should be replaced with this:

                    if( is_array($value) )
                    {
                        $tokens = str_repeat('?,', count($value)-1 ).'?';
                        $this->append_where("$key IN($tokens)", $value);
                    }
                    else
                    {
                        $this->append_where("$key=?", $value);
                    }