Skip to main content
1 of 10

The suggestions below should allow the code to be shortened and improved.

Update method

The UserProfileController::update() method is somewhat long. The sections below should allow it to be simplified.

Pass fields to update to model method update()

The UserProfileController::update() method is somewhat long. Presuming that the model UserProfile is a sub-class of Illuminate\Database\Eloquent\Model then the update() method can be passed an array of attributes to update. Instead of these lines:

        $current_user->first_name = $request->get('first_name');
        $current_user->last_name = $request->get('last_name');
        $current_user->email = $request->get('email');
        $current_user->bio = $request->get('bio');

Get an array of fields to update from $request->all(), then set the avatar on that array if the avatar needs to be updated.

Make a form request class for handling the validation

The validation rules could be moved out to a FormRequest subclass.

use Illuminate\Foundation\Http\FormRequest;

class UserUpdateRequest extends FormRequest
{
    public function rules()
    {
        return [
            'first_name' => ['required', 'string', 'max:255'],
            'last_name' => ['required', 'string', 'max:255'],
            'email' => ['required', 'email', 'max:100', 'unique:users,email,'. $current_user->id],
            'avatar' => ['mimes:jpeg, jpg, png, gif', 'max:2048'],
        ];
    }
}

Then inject that subclass instead of Illuminate\Http\Request in the update method arguments and use $request->all() to get the fields to pass to $current_user->update(). I haven't tested this but it should get you closer:

  public function update(UserUpdateRequest $request)
  {
      $current_user = Auth::user();
      $fieldsToUpdate = $request->all()
      // Upload avatar
      if (isset($request->avatar)) {
          $imageName = md5(time()) . '.' . $request->avatar->extension();
          $request->avatar->move(public_path('images/avatars'), $imageName);
          $fieldsToUpdate['avatar'] = $imageName;
      }
    
      // Update user
      $current_user->update($fieldsToUpdate);
      return redirect('dashboard/profile')
          ->with('success', 'User data updated successfully');
  }

Middleware

Instead of setting the middleware in the controller, a Middleware Group could be added to routes\web.php - e.g.

Route::group(['middleware' => ['auth']], function() {
    Route::get('/dashboard', [App\Http\Controllers\Dashboard\DashboardController::class, 'index'])->name('dashboard');

    Route::get('/dashboard/profile', [App\Http\Controllers\Dashboard\UserProfileController::class, 'index'])->name('profile');

    Route::post('/dashboard/profile/update', [App\Http\Controllers\Dashboard\UserProfileController::class, 'update'])->name('profile.update');

    Route::post('/dashboard/profile/deleteavatar/{id}', [App\Http\Controllers\Dashboard\UserProfileController::class, 'deleteavatar'])->name('profile.deleteavatar'); 
}

Resource Controller

While it may not save many lines and would likely require updating the route paths, consider using a Resource controller. The Update route would instead be /dashboard/profile with the verb of PUT or PATCH.

JavaScript

The call to $.ajax() can be replaced with a call to $.post(). Then there is no need to specify the method, and the keys can be removed from the options:

        $.post(
            APP_URL + '/dashboard/profile/deleteavatar/' + id,
            {
                id: id,
                _token: CSRF_TOKEN,
            },
            function() {
                $avatar.attr('src', defaultAvatar);
                $topAvatar.attr('src', defaultAvatar);
                $trashIcon.remove();
            }
        });