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

Extract local Model scopes to Eloquent Builder traits #227

Merged
merged 3 commits into from
Feb 24, 2023

Conversation

antonkomarev
Copy link
Member

@antonkomarev antonkomarev commented Jan 6, 2023

This is breaking change!

Reactable trait methods scopeWhereReactedBy, scopeWhereNotReactedBy, scopeJoinReactionCounterOfType, scopeJoinReactionTotal were moved to ReactableEloquentBuilderTrait.

To start using them you have to create custom Eloquent Builder class, use trait in it and declare that it should be used in your model as default query builder in newEloquentBuilder method.

/**
 * @method static UserEloquentBuilder query()
 */
class User extends Model
{
    public function newEloquentBuilder($query): UserEloquentBuilder
    {
        return new UserEloquentBuilder($query);
    }
}

class UserEloquentBuilder extends \Illuminate\Database\Eloquent\Builder
{
    use \Cog\Laravel\Love\Reactable\ReactableEloquentBuilderTrait;

    // Other User model local query scopes
}

@antonkomarev antonkomarev force-pushed the extract-model-local-scopes-to-builder-traits branch 7 times, most recently from 760db14 to 22c13ed Compare January 8, 2023 11:18
@antonkomarev
Copy link
Member Author

@vesper8 are you using custom Eloquent Builder classes? I'd prefer to go with this soltution. What do you think?

@vesper8
Copy link

vesper8 commented Jan 8, 2023

Hrm... @antonkomarev yes actually I am using one because I also use https://github.com/MatanYadaev/laravel-eloquent-spatial

Which requires this as part of its setup


    public function newEloquentBuilder($query): SpatialBuilder
    {
        return new SpatialBuilder($query);
    }

And my models that are using this, are also Reacters or Reactants so.. if your package also starts requiring this.. I think it's going to cause a really bad conflict and I won't be able to use it.. or.. it will take some weird hacking so I can use both builders somehow?

@antonkomarev
Copy link
Member Author

antonkomarev commented Jan 8, 2023

@vesper8 that's why I'm providing this methods as trait for the Eloquent Builder, but not actually builder. This way we will be able to compose them with other methods.

I see here 2 ways:

  1. Propose to author of laravel-eloquent-spatial package to move those methods to Trait and use it in Builder, to let people compose with their own builders
  2. Make new Eloquent Builder in your app which will extend SpatialBuilder (since it's not final) and use traits from Love's Eloquent Builders

@vesper8
Copy link

vesper8 commented Jan 8, 2023

@antonkomarev I like that link of thinking. Worse case if I need to fork or make my own builder and extend it, that won't be an issue.. I can manage that.

@antonkomarev
Copy link
Member Author

In this solution I like that all these methods are placed as closer to Query Builder as possible and IDE auto-complete will work out of the box.

@antonkomarev antonkomarev force-pushed the extract-model-local-scopes-to-builder-traits branch from 50bb409 to bcd0f58 Compare February 24, 2023 11:51
@antonkomarev antonkomarev merged commit 1df2ed9 into master Feb 24, 2023
@antonkomarev antonkomarev deleted the extract-model-local-scopes-to-builder-traits branch February 24, 2023 13:06
@antonkomarev
Copy link
Member Author

@vesper8 Laravel Love v9 has been released with this feature and Laravel 10 support.

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

Successfully merging this pull request may close these issues.

2 participants