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

Subclass query building #31

Closed
joshbruce opened this issue Dec 12, 2016 · 3 comments
Closed

Subclass query building #31

joshbruce opened this issue Dec 12, 2016 · 3 comments

Comments

@joshbruce
Copy link

joshbruce commented Dec 12, 2016

Using Laravel 5.3

I'm not sure if I'm expecting something that should be there or not.

class Base extends Model { }

class Sub extends Base { }

class SubAlt extends Base { }

I would expect doing something like this:

Sub::all()

Expectation:

\Collection {
    all: [
        \Sub { }
    ]
}

Getting:

\Collection {
    all: [
        \Sub { }
        \SubAlt { }
    ]
}

Not sure if this is part of the Laravel 5.3 ticket #23 or not.

@joshbruce
Copy link
Author

joshbruce commented Dec 12, 2016

I made the following, which seems to work:

class Base extends Model
{
    public static function allQuery($columns = ['*'])
    {
        if (!in_array(static::class, static::$singleTableSubclasses)) {
            $query = null;
            foreach(static::$singleTableSubclasses as $subClass) {
                if (is_null($query)) {
                    $query = static::where('type', $subClass::$singleTableType);

                } else {
                    $query->orWhere('type', $subClass::$singleTableType);

                }
            }
            return $query;
        }
        return static::where('type', static::$singleTableType);
    }

    public static function all($columns = ['*']) 
    {
        return static::allQuery($columns)->get();
    }
}

This allows you to do something like:

Base::all()

/*
\Collection {
    all: [
        \Sub { }
        \SubAlt { }
    ]
}
*/

Sub::all()
/*
\Collection {
    all: [
        \Sub { }
    ]
}
*/

SubAlt::all()
/*
\Collection {
    all: [
        \SubAlt { }
    ]
}
*/

Believe this could be added to the trait itself and accomplish the same goal, but I'm not sure it's the most elegant solution.

@joshbruce
Copy link
Author

joshbruce commented Dec 12, 2016

Second iteration:

  • Uses DB.
  • Allows columns specification.
  • Cleaner implementation overall.
  • Takes better advantage of developer settable properties.
    public static function query($columns = ['*'])
    {
        $obj = new static();
        $query = DB::table($obj->table)->select($columns);
        if (in_array(static::class, static::$singleTableSubclasses)) {
            $query = $query->where(static::$singleTableTypeField, static::$singleTableType);
        }
        return $query;
    }

    public static function all($columns = ['*']) 
    {
        return static::query($columns)->get();
    }

Going to be adding this to my own trait internal to my project. I have a mental note to submit a PR, but cannot promise timing and want to make sure I'm covering as many use cases as possible using my own project as the guinea pig first.

@jonspalmer
Copy link
Owner

I can't reproduce the error you describe and have added test that proves it works today dd0668b.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants