There is a school of thought that controllers and actions should be as lean as possible - pushing domain/business logic back into the models as much as possible. There's another school of thought that prefers to be a little more up front and show what's happening in the controller.
Then there's the approach of keeping it simple and only passing functionality to the model when your controller action is starting to get complicated (or, if there are multiple pathways to creating a comment, for example - such as different controller actions, or jobs/queues, or commands).
There's nothing really to any of these is good or bad practice. While "cleaner" code is always a good objective - there's quite a range of preference on what is cleaner.
In your case, it looks like you have a simple relation between an Item
and an ItemComment
using BelongsTo
and HasMany
. I see nothing wrong with keeping this in the controller as it is and moving on to build the rest of your application.
Even when relationships get more complex (for example, in one of my applications I have an Element
which relates to both a parent and a root entity), I still prefer to create the new entity in the controller (without saving) and pass that to the parent's addEntity
, which is responsible for setting relationships and then saving the new entity, rather than pass in (validated) request parameters.
Another advantage to having something like addComment
on your Item
model is that you go one step further and make other models "commentable" by creating a trait that can be reused and extracting the addComment
method to that trait. Although, for this to work out you'll need to use a polymorphic relationship.
I also try to use the standard convention naming and routing for actions.
// ItemCommentsController.php
// action for POST /item/{id}/comments
public function store(Item $item, Request $request)
{
// I'm assuming some basic validation here.
$validated = $request->validate([
'text' => 'required|max:1023',
'isLog' => 'sometimes|boolean',
]);
// $validated will only contain 'text' and 'isLog'.
// also, the validate method will return an error to your user if invalid
// and the following code will not be run.
$comment = $item->addComment(new ItemComment($validated));
return; // you'll want to send a response or redirect here
}
// Comment.php
class Comment extends Model
{
// The attributes that can be mass assigned through constructor.
protected $fillable = ['text', 'isLog'];
// relationship to whatever the comment is attached to.
// your comments table will need a parent_id and parent_type field.
public function parent()
{
$this->morphTo();
}
}
// Item.php
class Item extends Model
{
// use the Commentable trait.
// (I put traits in a Behaviours directory for namespacing).
use \App\Behaviours\Commentable;
}
// Commentable.php
trait Commentable
{
public function addComment(Comment $comment)
{
$comment->parent_id = $this->id;
$comment->parent_type = static::class;
$comment->user_id = auth()->id();
return $comment->save();
}
// this assumes your comments table has parent_id and parent_type fields.
public function comments()
{
$this->morphMany(static::class, 'parent');
}
}
You can further lean out your controller action by creating a Form Request and injecting that instead of the generic Request
object.