Какой запрос выбрать для скопа?

Question

[jazzus]

В модели Product нужно получить объект, принадлежащий авторизованному юзеру.

Запрос 1

public function scopeOfUserBySlug($query, $user, $slug)
    {
        return $query->whereUser_id($user->id)->whereSlug($slug);
    }

Вызывается Product:: ofUserBySlug(Auth::user(), $slug)
Запрос 2

public function ofUserBySlug($user, $slug)
    {
        return $user->products->whereSlug($slug);
    }

Вызывается (new Product)->ofUserBySlug(Auth::user(), $slug)

Что выбрать и почему?
Пс
Сейчас во всем приложении $product = $user->product($slug)
Но т.к. допустил уже несколько ошибок (получал не от авторизованного пользователя и это было незаметно) решил вынести данный запрос в модель. Так правильно делать? Или оставить $user->product($slug) везде? Либо $user->product($slug) убрать в трейт и вызывать Product:: ofUserBySlug(Auth::user(), $slug) А трейт подключить к модели. Хочется, чтобы сразу понятно выглядело, что за запрос. Плюс вдруг нужно будет добавить, что-то к этому билдеру, какой нибудь active, true - и тогда нужно будет во всем приложении менять. В общем лучше наверное вынести. Замен будет много (по данному примеру другие объекты). Как бы вы сделали? (product($slug) –это products->whereSlug($slug);)

Answer 1

[Alex Wells]

Второе - не совсем по теме Eloquent'а. Он про магию, и тут скоупы приходятся к месту. А если нужны будут еще условия? Ну, допустим остаите первым вызов этого метода, а если нужны будут еще условия в виде методов? Вот для чего были созданы скоупы, так что первый вариант - однозначно лучше.

Далее: Product:: под капотом как раз и делает (new Product)-> , только кроме этого он может делать и что-то другое, о чем вы не знаете, поэтому я бы использовал Product:: хотя бы ради этого и какой никакой стандартизации.

Далее: почему бы не сделать $user->products()->whereSlug($slug)? Учитывая контекст, смею предположить что именно такой запрос вам нужен не так уж и часто, а значит это - неплохое решение. Нужен будет эктив - ну добавляете скоуп в модель Product и имеете счастье, типа $user->products()->whereSlug($slug)->isActive(). Последние два метода относятся к конечной модели Product, а значит их дублировать не нужно, а вот релейшены в разных моделях (

$user->products(), $page->products(), $something->products()

) - нужно, так что это правильный вариант.

Тем более, Product не должен быть ответственен за методы юзера (как в вашем втором варианте), а если туда напихать еще несколько от разных моделей, со всякими $active = true - получится каша.

TL;DR: Придерживайтесь того, что предлагает Eloquent, и будет вам счастье.

Comments

[jazzus]

Спасибо за идеи, сейчас буду вникать. Как раз занимаюсь данным вопросом. С утра была идея собрать запрос в модели user. Я вчера с квадратной головой был и не сообразил, что могу в user сделать именованный метод productOfUserBySlug и его уже совершенствовать как угодно с activ и т.д. Единственное у меня не просто User а юзеры, разделенные по ролям.. Думаю, нужно ли мне в методе устраивать дополнительные проверки на роль типа

public function productForGlavnayaBySlug($slug)
    {
      if ($products=$this->products() and $this->hasActiveRole(Role::ID__GLAVNAYA))
      {
          return $products->whereSlug($slug)->whereActive(true)->isApproved();
      }
    }

Технически конечно не нужно, наверное.. Т.к. авторизованный юзер, который имеет продукт все равно какой роли. Он уже владелец по факту. Но идеологически) Вдруг чего. Ошибся где в контроллере. А так есть проверка и сразу покажет. Хотя, как практика показывает, придумаешь такое, увидишь через пару дней, по лбу себе ударишь и на делит))

[Alex Wells]

jazzus, опять же, юзер не должен быть ответственен за собирание каких-то запросов. То, что вы хотите - называется репозиторий, гуглите. Но в рамках eloquent'а я бы не стал их использовать, а вместо этого просто бы инлайнил внутри сервиса, или даже контроллера нужный запрос.

И нет, никаких проверок внутри запросов быть не должно и быть не может, ибо если вы захотите протестировать этот запрос, то вам что, создавать юзера с ролью в юнит тесте? Глупость. Проверяйте роль (а лучше - пермишен, роли это хрень) в входных местах (FormRequest'ы и/или контроллеры).

tl;dr 2: просто следуйте докумнетации, за вас уже все придумали

[jazzus]

Alex Wells, т.е. мне нужен некий репозиторий. И как я прочитал - в php это просто класс, который хранит различные варианты коллекций объектов. Т.е. методов, которые через различные запросы получает объекты и потом их оттуда вытягивать.
Получается:
Я создаю класс ProductRepository
там делаю запрос (пока еще точно не понял какой)
и вызываю потом в контроллерах (new ProductRepository($slug))->ofUserBySlug();
Т.е. думать и гуглить в направлении этих репозиториев?)) ппц.. сколько там всего

[jazzus]

Alex Wells, ааа т.е. наоборот не советуете их использовать. Понял. Спасибо буду думать

[jazzus]

Alex Wells, Да, насчет ролей это я мягко говоря погорячился..))

Далее: почему бы не сделать $user->products()->whereSlug($slug)? Учитывая контекст, смею предположить что именно такой запрос вам нужен не так уж и часто, а значит это - неплохое решение. Нужен будет эктив - ну добавляете скоуп в модель Product и имеете счастье, типа $user->products()->whereSlug($slug)->isActive().

Вы здесь предлагаете не убирать в отдельный запрос?
Т.е. просто $user->products()->whereSlug($slug); в контроллерах?
У меня сейчас так везде. Ну почти (я сократил до $user->product($slug); а whereSlug($slug) уже в модели user. Считаете ничего не менять не нужно? $user->objects() оставить? И все isActive просто добавлять в контроллерах? Это конечно заманчивый вариант)) Т.к. у меня много где нужно будет рефакторить. Но все же есть ощущение потери контроля над приложением. Т.к. в 4х местах добавишь isActive, а в 5м забудешь. Если все таки выносить эту логику в метод, то в каком лучше месте его реализовывать? Трейт, репозиторий или модель. Как считаете?
Кстати, почему не правильно делать такие старты билдера в модели user? Там ведь есть отношения. Типа products(). По идее логично сделать productBySlug($slug) и productById($id) если products есть. Есть коллекция продуктов у юзера (отношение), и есть конкретный продукт (запрос к отношению). По идее логично. Разве это не Eloquent Style? Я склонялся к такому варианту сегодня и был рад этой идее)

[jazzus]

Alex Wells, мне в данном скопе

public function scopeOfUserBySlug($query, $user, $slug)
    {
        return $query->whereUser_id($user->id)->whereSlug($slug);
    }

не очень нравится, что он идет параллельно отношению. Уже ведь есть отношение в user где определена связь с продуктом и так выходит, что я с этим скопом буду опять искать продукт по user_id через where при живом отношении. Интуитивно как-то не нравится

[jazzus]

Я понимаю, что конкретная модель и запросы должны относится к своей таблице. User относится к users, product к products. И не по Eloquent делать запросы продукта у юзера. Но в данном случае не просто продукт. А продукт пользователя. Т.е. нечто, что состоит из двух моделей - объект отношения hasMany с дополнительными условиями.

[jazzus]

В общем скрипя зубами и в муках выбора перенес в скопы продукта. Хотя так до конца и не понял, что лучше из юзера или из продукта билдер получать
Product::ofUserBySlug($slug);
Или
Auth::user()->productBySlug($slug);
Судя по отзывам и статьям на хабре люди хранят билдеры где им нравится (для Лары не принципиально т.к. она сама, как один большой репозиторий).

В итоге сделал скоп в Product

public function scopeOfUserBySlug($query, $slug)
    {
         return $query->where('user_id', Auth::id())
                      ->whereSlug($slug);
    }

И сразу с проверкой по Auth внутри(без передачи юзера), чтоб уж наверняка только авторизованный юзер имел доступ

[jazzus]

Alex Wells, А вот теперь я в коде увидел почему скоп из продукта единственно верный вариант, а юзер не подходит. т.к. если например product вызывается из list то я бы уже не мог просто вставить скоп
ofUser(); т.к. он бы находился в юзере. Нужно было бы танцевать с бубном. Так что хорошо, что послушал спасибо)

[Alex Wells]

jazzus, коммент1: да, примерно так. Только лучше dependenncy injection'ом его резолвить, мало ли какие у него зависимости могут быть. Гугли eloquent repository example

коммент2: Я бы не стал, потому что мое имхо - это лишний оверхед. Мб с доктриной (другая orm популярная в php комьюнити) они бы и зашли мне, но не с елоквентом.

коммент3: Если у вас запросы повторяются - то либо у вас что-то с приложением не так, либо выносите в класс-репозиторий)

коммент4: ну потому что надо использовать отношения напрямую, а не городить отношения из скоупов (по сути, ваш scopeOfUserBySlug это такой себе костыль для самодельных отношений, но за вас уже это давно сделали)

[Alex Wells]

jazzus, коммент5: не важно, даже если бы это был UserProduct, или UserDetails - это отдельная модель, и она отвечает сама за себя. А иначе в чем вообще смысл выносить это в отдельную модель? С такими успехами можно вфигачить все в json массив в юзере и все.

коммент6: второй вариант, конечно-же. Вы спрашиваете у юзера, какие у него есть продукты, а не у продуктов, кто из них принадлежит юзеру. Ну первый вариант же проще и логичней звучит, разве нет?

public function scopeOfUserBySlug($query, $slug)
    {
         return $query->where('user_id', Auth::id())
                      ->whereSlug($slug);
    }

Извините, но это полная х.. дичь) Что where('user_id'), что фасад auth внутри scope, что whereSlug, что и сама суть всего скоупа. Это пример того как делать НЕ НУЖНО.

[jazzus]

приплыли

[jazzus]

я уже во всем приложении исправил. Сделал ScopeTrait для всех юзерных моделей с методом

public function scopeAllOfAuthUser($query)
    {
       return $query->where('user_id', Auth::id());
    }

У меня и было Auth::user()->products до этого..

[Alex Wells]

jazzus, $user->products()->whereSlug($slug); - вот что я предложил изначально. И оно либо так, либо так же но в методе репозитория, никак по другому.

[jazzus]

Это пример того как делать НЕ НУЖНО.

Можете запрос написать, как нужно делать?
И в какой модели. Так будет проще. Заранее спасибо)

[jazzus]

Alex Wells,

вот что я предложил изначально. И оно либо так, либо так же но в методе репозитория, никак по другому.

Ясно. Спасибо) В общем придумал я себе развлечение по пустому рефакторингу на 2 дня.. Тогда буду делать уже репозиторий, чтобы не зря

[jazzus]

любо гит протестировать))

[jazzus]

откатился обратно и понял, что репозиторий в случае Auth::user()->metod нафиг не нужен только усложнит читаемость

[Alex Wells]

jazzus, ну, если вы не хотите дублировать, и уверенны, что указанный запрос НИКОГДА не будет менятся, если только не во всех местах сразу, то выносите в репозиторий. Если вы не уверены в этом, а это так в большинстве случаев, то оставляйте как есть)

[jazzus]

Alex Wells, уже оставил, посмотрел и не понял зачем эту тему содал)) Но зато теперь уверен, что вызывать нужно из юзера без каки-то навязчивых идей)

[Alex Wells]

jazzus, у меня чуство, что вы 24/7 обновляете страницу тостера. Отвечаете моментально)

[jazzus]

Alex Wells, только зашел)