Laravel UserController Design

Question

<?php

namespace App\Http\Controllers;

use Illuminate\Http\Request;
use App\User;
use Auth;

class ProfileController extends Controller
{

    public function __construct()
    {

        $this->middleware('auth');

    }

    public function index($id)
    {

        $user = User::find($id);

        $photos = $user->getPhotos();

        $interests = $user->getInterests();

        $gifts = $user->getGifts();

        $visits = $user->totalVisits();

        $likes = $user->totalLikes();

        if ($user->id == Auth::user()->id) {

            $page_name = 'My Profile';

            return view('profile.my_profile', compact('page_name', 'user', 'photos', 'interests', 'likes', 'visits', 'gifts'));

        } else {

            $page_name = $user->name;

            return view('profile.profile', compact('page_name', 'user', 'photos', 'interests', 'likes', 'visits', 'gifts'));

        }

    }

}

I have the following controller, which manages the "Profile" page in my web app. I feel like in my current code, there is a separation in a way that photos, interests, gifts, visits and likes look like they do not belong to the User.

How would I go to refactor all these different functions into one? Should I change the User Model's __construct to execute all of the functions? Ideally, I would like to pass only two variables to my view ("user" and "page_name" variables)

Here is my model, for reference:

<?php

namespace App;

use Illuminate\Foundation\Auth\User as Authenticatable;
use Illuminate\Notifications\Notifiable;
use Illuminate\Support\Facades\DB;

class User extends Authenticatable
{
    use Notifiable;

    protected $table = 'users';

    public $timestamps = false;

    /**
     * The attributes that are mass assignable.
     *
     * @var array
     */

    protected $fillable = [
        'name', 'email', 'password',
    ];

    /**
     * The attributes that should be hidden for arrays.
     *
     * @var array
     */

    protected $hidden = [
        'password', 'remember_token',
    ];

    public function getPhotos()
    {

        return DB::table('photos')
            ->where('user_id', $this->id)
            ->get();

    }

    public function getInterests()
    {

        return DB::table('interests')
            ->where('user_id', $this->id)
            ->get();

    }

    public function getGifts()
    {

        return DB::table('gifts')
            ->join('sent_gifts', function ($join) {
                $join->on('gifts.id', '=', 'sent_gifts.gift_id')
                    ->where('user_2', $this->id);
            })
            ->join('users', function ($join) {
                $join->on('users.id', '=', 'sent_gifts.user_1');
            })
            ->get();

    }

    public function getVisits()
    {

        return DB::table('user_reactions')
            ->join('users', function ($join) {
                $join->on('users.id', '=', 'user_reactions.user_1')
                    ->where('user_2', $this->id)
                    ->where('reaction', 'visit');
            })
            ->get();

    }

    public function getLikes()
    {

        return DB::table('user_reactions')
            ->join('users', function ($join) {
                $join->on('users.id', '=', 'user_reactions.user_1')
                    ->where('user_2', $this->id)
                    ->where('reaction', 'like');
            })
            ->get();

    }

    public function getFilter()
    {

        return DB::table('filters')
            ->where('user_id', $this->id)
            ->limit(1)
            ->get();

    }

    public function totalVisits()
    {

        return DB::table('user_reactions')
            ->where('reaction', 'visit')
            ->where('user_2', $this->id)
            ->get()
            ->count();

    }

    public function totalLikes()
    {

        return DB::table('user_reactions')
            ->where('reaction', 'like')
            ->where('user_2', $this->id)
            ->get()
            ->count();

    }

}

Answer 1

First of all do not use Laravel 5.4 even if you've written some^* code already, use latest 5.7, and try to always update.

_{*: You really need to rewrite the application if you have code like you presented everywhere, so just take your time and use L5.7. Idea is already there you just need to improve it - pure coding, no thinking.}

Model(s)

Standards

You are sticking to the standard (I guess because User model is the stock one) and using class name User for a model means that table is plural which renders $table property useless and you save one line.

Each table shall have its own Model.

Note that we did not tell Eloquent which table to use for our Flight model. By convention, the "snake case", plural name of the class will be used as the table name unless another name is explicitly specified.

---

Relationships

Main reason you have read about Laravel when you were googling around which framework to use is because its ORM Eloquent^*. Eloquent is a abstraction so you do not even need to know how each relationship work under the hood, it does all the magic for you. Firstly you'll need to understand what relationships are so google around, and when you are done use superb documentation of Laravel^** and get all the info from there.

_{*,**: In early days of Laravel it was the selling point for many developers.}

Defining the relationships is hard part of RDBMS design, so take your time, do it right, use pen and paper and then code it - easy part if design is right.

Note: For user_reactions use polymorphic relations.

Invest time to learning relationships it will pay off soon :)

Note: I am not writing about how I would define relations you have in code because of lack of context and mainly because of helping too much and I consider it as giving a fish instead of teaching how to fish.

Controller

Routing / Middleware / Route-Model-Binding

Forget about defining middlewares in controller itself, use routes file for that, so delete the __construct() and just add it back to the routes file. You may ask why, well because of DRY, you will end up with 5-x controllers and each of them will contain $this->middleware('auth'); in constructor, repeating...^*

_{*: Unless you make a "master" class like AuthController and extend from there - IMO that just adds more complications than good if you want to define middleware only.}

Use groups in route file, and define middleware on the groups part, see below for route example.

---

Route Model Binding another super useful feature of Laravel. Example vs bunch of words:

Route:

Route::middleware(['auth'])->group(function () {
    Route::get('profile/{user}', 'ProfileController@index'); <-- Route Model Binding
    Route::get('profile/edit', 'ProfileController@edit');
});

Signature of ProfileController@index method should be index(App\User $user), you see behind the scenes Laravel does query to the database and fetches User instance for you.

---

After defining all the relationships on User model you will most likely end up with using a oneliner to get the $user:

$user = User::with(['photos', 'interests', 'gifts', 'visits', 'likes'])->whereId($id)->get(); // if you stick with not using Route Model Binding
$user->load(['photos', 'interests', 'gifts', 'visits', 'likes']); // if you use Route Model Binding

// in view you will access interests for example $user->interests

Now what about edge case if user is looking at their own profile, as I understand the only difference is to change variable $page_name, do it right in the view (I would do it that way):

@if($user->id == Auth::user()->id)
My Profile
@else
{{ $user->name }}
@endif

So proper controller after suggestions would look like:

<?php
namespace App\Http\Controllers;

use App\User;
use Auth;

class ProfileController extends Controller
{

    public function index(User $user)
    {
        $user->load(['photos', 'interests', 'gifts', 'visits', 'likes']);

        return view('profile.my_profile', compact('user'));
    }

}

Packages

Do not reinvent the wheel, for example for handling attachments / images use solution that is tested https://github.com/spatie/laravel-medialibrary and used by many!

During development debugbar is a must.