Hello Laracast community, I'm trying to refactor my Model Class into it's own Service Class to separate Logic from the Models and make the Controller easier to read and maintain. On top of that, I know that my controller sucks big time and needs a lot of improvement to make it more "eloquentish". No question about that.
I'm having this relation in my ItemModel (besides many other HasOne or BelongstTo relations on other tables):
public function price(): HasMany
{
return $this->hasMany(Price::class);
}
public function currentPrice(): HasOne
{
return $this->hasOne(Price::class)->ofMany([
'published_at' => 'max',
'id' => 'max',
], function(QueryBuilder $query) {
$query->where('published_at', '<', now());
});
}
Obviously there is a item(): BelongsTo method on the Price Model.
Now, I want to keep the price(): HasMany relation on the Item Model but want to move the currentPrice() method to the Service Class (or in other words: having that login as part of the eloquent query itself) and call that one method from the controller. I tried and failed miserably doing this:
class ItemService
{
public function getItems()
{
$data = Item::with([
'retailer' => function ($q) {
$q->where('active', 1);
},
'label',
'price' => function ($query) {
$query->where('published_at', 'max');
$query->where('id', 'max');
},
'weight'
])->paginate();
if ($data->count() <= 0) {
throw new \Exception('No items available');
}
return $data;
}
}
I also tried Item::query() with whereRelation() and many more without much luck.
You not gonna like what you see now: the controller. I by myself can't stand it, so please don't judge as I'm still learning. Ohh, and there's so much wrong with it like pagination, sorting, more complex search queries incl. filters etc. ... I know and am working on it.
private function getItems()
{
$data = Cache::remember('item', 0, function () {
$data = Item::with(['retailer', 'label', 'price', 'weight'])->paginate(4);
return $data->map(function ($item) {
return [
'id' => $item->id,
'name' => $item->article_name,
'info' => $item->short_description,
'image' => env('ASSET_URL') . '/images/articles/' . $item->cat_d_id . '/' . $item->article_image_url,
'weight' => ItemHelper::weightOrAmount($item->weight->last()->weight, $item->weight->last()->amount),
'price_weighted' => ItemHelper::getWeightedPrice($item->weightOrAmount->last()->weight, $item->weightOrAmount->last()->amount, $item->currentPrice->price),
'original_url' => $item->article_url,
'retailer_name' => $item->retailer->name,
'retailer_logo' => env('ASSET_URL') . '/images/logos/' . $item->retailer->logo,
'label_name' => $item->label->name,
'label_logo' => ($item->label->logo != NULL) ? env('ASSET_URL') . '/images/labels/' . $item->label->logo : env('ASSET_URL') . '/images/globals/null.png',
'current_price' => Number::currency($item->currentPrice->price),
'min_price' => $item->minPrice->price,
'max_price' => $item->maxPrice->price,
'upcoming_price' => $item->upcomingPrice->price,
'delivery_info' => ($item->delivery_date_info != NULL) ? __('global.available_until') . ' ' . new LangCountry()->dateNumbers(Carbon::parse($item->delivery_date_info)) : null,
];
});
});
$sorted_by_price = collect($data)->sortBy('current_price');
$data = $sorted_by_price->toArray();
return [
'data' => $data,
'pagination' => Item::with(['retailer', 'label', 'price', 'weight'])->paginate(4)
];
}
So, if anyone would like to give me a hand on how to refactor the currentPrice() method on the Item Model into the Item Service Class its "getItems()" method I would highly appreciate it.
Thanks, Michael
currentPrice is a relationship, it needs to be declared inside the model class, so you can't move it to a service class.
If you want to move it to another file, you can create a trait and use the trait.
trait MyTrait
{
public function currentPrice()
{
...
}
}
But a trait is mainly used for reusable code into several models / classes.
@vincent15000 thanks for your quick feedback! So, this is not an option. I guess the best option for move it into a service class is leveraging a DB query like:
$users = DB::table('users')
->join('contacts', 'users.id', '=', 'contacts.user_id')
->join('orders', 'users.id', '=', 'orders.user_id')
->select('users.*', 'contacts.phone', 'orders.price')
->get();
this comes very close to a raw sql query - this way, I should be able to move the relation from the model to the query itself, right? The question is how I manage the logic that is in the "currentPrice()" method?
@mkocurek Yes it's effectively a solution, but you will need to pass the item model as a parameter to the function in the service class.
Hmmm ... first I would rename price to prices, because an item has many prices.
Then I would write the function (in a service given that you want it in a service).
public function currentPrice(Item $item)
{
return $item->prices->where('published_at', '<', now())->sortByDesc('published_at')->first();
}
@mkocurek Try to push as much of this into your query, and let the database handle it. Calculations,filtering and sorting is way faster in the database than in php.
return $data->map(function ($item) {
return [
'id' => $item->id,
'name' => $item->article_name,
'info' => $item->short_description,
'image' => env('ASSET_URL') . '/images/articles/' . $item->cat_d_id . '/' . $item->article_image_url,
'weight' => ItemHelper::weightOrAmount($item->weight->last()->weight, $item->weight->last()->amount),
'price_weighted' => ItemHelper::getWeightedPrice($item->weightOrAmount->last()->weight, $item->weightOrAmount->last()->amount, $item->currentPrice->price),
'original_url' => $item->article_url,
'retailer_name' => $item->retailer->name,
'retailer_logo' => env('ASSET_URL') . '/images/logos/' . $item->retailer->logo,
'label_name' => $item->label->name,
'label_logo' => ($item->label->logo != NULL) ? env('ASSET_URL') . '/images/labels/' . $item->label->logo : env('ASSET_URL') . '/images/globals/null.png',
'current_price' => Number::currency($item->currentPrice->price),
'min_price' => $item->minPrice->price,
'max_price' => $item->maxPrice->price,
'upcoming_price' => $item->upcomingPrice->price,
'delivery_info' => ($item->delivery_date_info != NULL) ? __('global.available_until') . ' ' . new LangCountry()->dateNumbers(Carbon::parse($item->delivery_date_info)) : null,
];
});
});
$sorted_by_price = collect($data)->sortBy('current_price');
$data = $sorted_by_price->toArray();
@mkocurek Try not to confuse “services” with “a class that you just lift-and-shift code from your models into”. That’s not really their use case, and you certainly wouldn’t take relation methods out of an Eloquent model and put them in a “service” class.
Service classes are for encapsulating business logic. So if you have an “item” model then I assume it belongs to an order model or similar. You might have this in a controller:
public function store(StoreOrderRequest $request)
{
$order = new Order([
'currency' => $request->input('currency'),
'ip_address' => $request->ip(),
'status' => OrderStatus::Created,
]);
$order->customer()->associate($request->user());
$order->save();
foreach ($request->input('items') as $item) {
$product = Product::query()->findOrFail($item['id']);
// Ensure there is enough inventory to fulfil line item...
$orderItem = $order->items()->make([
'description' => $product->name,
'unit_amount' => $product->unit_price,
'quantity' => $item['quantity'],
]);
$order->items()->save($orderItem);
}
// Create any other order-related models...
return redirect()->route('orders.show', $order)->with('success', 'Order created.');
}
Lots of logic, and lots of models getting queried and created. And this is exactly the kind of thing that would benefit from being moved to an OrderService class. Once it has, your controller would then look something like this:
public function store(StoreOrderRequest $request)
{
$this->orderService->createOrder($request->safe());
return redirect()->route('orders.show', $order)->with('success', 'Order created.');
}
The “business logic” portion of the controller action is now a single line. The actual order creation logic has been moved to a method in a service class, and that service class can now be used elsewhere in your application (i.e. other controllers such as an admin controller or an API controller).
@martinbean What's the difference between $request->safe() and $request->validated() ?
@vincent15000 The safe method returns a Illuminate\Support\ValidatedInput instance, which is just a little nicer to work with than a plain associative array that validated would return.
My previous message was purely for the sake of an example, but if you did want to further enforce data being sent to the OrderService::createOrder method, then you could create some sort of CreateOrderParameters class (which others may refer to as a “DTO”) that you construct from a ValidatedInput instance:
$parameters = CreateOrderParameters::fromValidatedInput($request->safe());
$order = $this->orderService->createOrder($parameters);
Please or to participate in this conversation.