Laravel controller for form validation skinny

Question

I'm trying to make controllers skinny as possible. I use repositories for accessing the database and I use Services to do other stuff. In this case, I use it to insert a new post to the database. For validation I use Jeffery Way's validation package for Laravel.

PostController.php

<?php
use Laracasts\Validation\FormValidationException;
use Dnianas\Post\PostCreationService;
use Dnianas\Post\PostRepository;
use Dnianas\Forms\PostForm;
class PostController extends BaseController
{
 /*
 * Dnianas\Services\PostCreationService
 */
 protected $post;
 /*
 * The post repository
 */
 protected $posts;
 /**
 * @param PostForm $postForm
 * @param PostCreationService $post
 * @param PostRepository $postRepo
 */
 public function __construct(PostForm $postForm, PostCreationService $post, PostRepository $postRepo)
 {
 $this->posts = $postRepo;
 $this->postForm = $postForm;
 $this->post = $post;
 }
 public function index()
 {
 $this->beforeFilter('auth');
 }
 /**
 * @return \Illuminate\Http\JsonResponse
 */
 public function create()
 {
 // Get the input
 $input = Input::all();
 // Validate it
 try {
 $this->postForm->validate($input);
 } catch(FormValidationException $e) {
 return Response::json([
 'success' => 'false',
 'message' => 'You didn\'t enter anything, Post cannot be empty.'
 ]);
 }
 // Insert it to the database
 $post = $this->post->create($input, Auth::user()->id);
 // Get the html content from the view
 $html = View::make('posts.post', ['post_id' => $post->id])->render();
 // Return a message along with the html content
 return Response::json([
 'success' => 'true',
 'message' => 'Your post has been successfuly posted!',
 'post_html' => $html
 ]);
 }

PostForm.php

<?php namespace Dnianas\Forms;
use Laracasts\Validation\FormValidator;
class PostForm extends FormValidator
{
 /**
 * Validation rules for when the user creates a post
 * @var array
 */
 protected $rules = [
 'post_content' => 'required'
 ];
}

PostCreationService.php

namespace Dnianas\Post;
class PostCreationService 
{
 /**
 * @param $input
 * @param $user_id
 * @return \Post
 */
 public function create($input, $user_id)
 {
 // Validation passed to we insert it to our database
 $post = new \Post;
 $post->post_content = $input['post_content'];
 $post->posted_date = \Carbon::now();
 $post->user_id = $user_id;
 $post->save();
 return $post;
 }
}

Is this controller skinny enough? Can it get any skinnier? If so, then how?

• Bishal Paudel: You can use Laravel events while saving posts. This way you don't need to check validation in the controller.

Answer 1

• You can rewrite your entire variable section and constructor with PHP 8 constructor promotion:

class PostController extends BaseController
{
 /** Description */
 public function __construct(protected PostForm $postForm, 
 protected PostCreationService $post, 
 protected PostRepository $postRepo) {}
 // rest of controller
}

• In PostForm, I would put the namespace on its own line.
• Instead of using the FormValidator package, you can use FormRequests which are built into Laravel and do the same thing.
• Is it really just 'post_content' => 'required'? The value may be longer than you can store in the database. It may be a .php file or some junk for all you know. Maybe 'required|string|max:255'?
• The comments "Get the input" and "Validate it" are too obvious to be useful
• You could have used the helper now() in $post->posted_date = \Carbon::now(); but actually I question if you need posted_date at all because it will equal created_at from timestamps which is done automatically (and can be updated at a later date).
• Why do you pass Auth::user()->id to PostCreationService's create method when you can just move it inside there? (Remember, you can use actingAs($user) when testing.)
• Your methods should all declare their parameter and return types

Answer 2

Check out Dwight Watson's Validating Trait. I like to use it with:

protected $throwValidationExceptions = true;

Validation of Eloquent models occurs automatically according to the rules defined on the model and if it fails, a ValidatingException is thrown that you can catch further up the application stack. For example, I might put the following exception handler somewhere like app/global/start.php

App::error(function (\Watson\Validating\ValidationException $exception)
{
 Log::error($exception);
 // I prefer 'success' => 'false' to be expressed in the response code
 return Response::json([
 'message' => $exception->getErrors()->first(),
 ], 422); // Unprocessable entity
});

Them my controllers end up looking more like this:

public function create()
{
 $input = Input::all();
 // Validation occurs automatically during object creation
 $post = $this->post->create($input, Auth::user()->id);
 $html = View::make('posts.post', ['post_id' => $post->id])->render();
 // I prefer "success" to be expressed by the 200 OK response code
 return Response::json([
 'message' => 'Your post has been successfully posted!',
 'post_html' => $html
 ]); // not specifying a response code implies 200 OK
}

But it's really just a matter of personal preference at this point. Your code looks good to me.

tl;dr +1 would pull & merge