Based on your use case alone, I would not pass the constraints to the existing scope – that’s polluting the functionality of the scope. The ofCategoryOrDescendants (as I would name it for clarity) scope should do just that: filter by records which are related to a specific category or its descendants.
If you want to further filter by active categories only, that’s a different scope – and not necessarily one that only needs to be used together with the first one. In your model, I would do this:
class Product extends Model {
public function scopeOfActiveCategory($builder) {
$builder->where('category_active', true);
}
public function scopeOfCategoryOrDescendants($builder, $categoryId) {
$builder->where('category_id', $categoryId)
->orWhereIn('category_id', function($builder, $categoryId) {
$builder->select('id')
->from('categories')
->where('parent_id', $categoryId)
;
})
;
}
}
And then chain the scopes in your controller:
Product::ofCategoryOrDescendant(1)->ofActiveCategory()->get();
This makes the code easier to read in that each scope has a clear and understandable name, and anyone reading it can easily figure out that we’re fetching all products which belong to an active category which is either 1 or a descendant of it. It also has the advantage of making the scopes more reusable: if you want to fetch all products from all active categories, just do Product::ofActiveScope()->get(). You wouldn’t be able to do that if you’d lumped the two together into one scope.
