I am working on a Laravel application (Github repo ) that requires user registration and login.
After registration, the users can change their registration details (except password, for which there is the default password recovery functionality) and add more info.
They also have the possibility to replace the default avatar image with a picture of their choice
In routes\web.php
I have:
use Illuminate\Support\Facades\Route;
Route::get('/', [App\Http\Controllers\Frontend\HomepageController::class, 'index'])->name('homepage');
Auth::routes();
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');
In Controllers\Dashboard\UserProfileController.php
I have:
namespace App\Http\Controllers\Dashboard;
use App\Http\Controllers\Controller;
use Illuminate\Http\Request;
use Auth;
use App\Models\UserProfile;
class UserProfileController extends Controller
{
// Guard this route
public function __construct() {
$this->middleware('auth');
}
public function index(UserProfile $user)
{
return view('dashboard.userprofile',
array('current_user' => Auth::user())
);
}
public function update(Request $request)
{
$current_user = Auth::user();
$request->validate([
'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'],
]);
$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');
// Upload avatar
if (isset($request->avatar)) {
$imageName = md5(time()) . '.' . $request->avatar->extension();
$request->avatar->move(public_path('images/avatars'), $imageName);
$current_user->avatar = $imageName;
}
// Update user
$current_user->update();
return redirect('dashboard/profile')
->with('success', 'User data updated successfully');
}
// Delete avatar
public function deleteavatar($id) {
$current_user = Auth::user();
$current_user->avatar = "default.png";
$current_user->save();
}
}
The update profile form:
<form action="{{ route('profile.update') }}" enctype='multipart/form-data' method="post" novalidate>
{{csrf_field()}}
<div class="form-group">
<input type="text" id="first_name" name="first_name" placeholder="First name" class="form-control" value="{{old('first_name', $current_user->first_name)}}">
@if ($errors->has('first_name'))
<span class="errormsg text-danger">{{ $errors->first('first_name') }}</span>
@endif
</div>
<div class="form-group">
<input type="text" id="last_name" name="last_name" placeholder="Last name" class="form-control" value="{{old('last_name', $current_user->last_name)}}">
@if ($errors->has('first_name'))
<span class="errormsg text-danger">{{ $errors->first('last_name') }}</span>
@endif
</div>
<div class="form-group">
<input type="text" id="email" name="email" placeholder="E-mail address" class="form-control" value="{{old('email', $current_user->email)}}">
@if ($errors->has('email'))
<span class="errormsg text-danger">{{ $errors->first('email') }}</span>
@endif
</div>
<div class="form-group">
<textarea name="bio" id="bio" class="form-control" cols="30" rows="6">{{old('bio', $current_user->bio)}}</textarea>
@if ($errors->has('bio'))
<span class="errormsg text-danger">{{ $errors->first('bio') }}</span>
@endif
</div>
<label for="avatar" class="text-muted">Upload avatar</label>
<div class="form-group d-flex">
<div class="w-75 pr-1">
<input type='file' name='avatar' id="avatar" class="form-control border-0 py-0 pl-0 file-upload-btn" value="{{$current_user->avatar}}">
@if ($errors->has('avatar'))
<span class="errormsg text-danger">{{ $errors->first('avatar') }}</span>
@endif
</div>
<div class="w-25 position-relative" id="avatar-container">
<img class="rounded-circle img-thumbnail avatar-preview" src="{{asset('images/avatars')}}/{{$current_user->avatar}}" alt="{{$current_user->first_name}} {{$current_user->first_name}}">
<span class="avatar-trash">
@if($current_user->avatar !== 'default.png')
<a href="#" class="icon text-light" id="delete-avatar" data-uid="{{$current_user->id}}"><i class="fa fa-trash"></i></a>
@endif
</span>
</div>
</div>
<div class="form-group d-flex mb-0">
<div class="w-50 pr-1">
<input type="submit" name="submit" value="Save" class="btn btn-block btn-primary">
</div>
<div class="w-50 pl-1">
<a href="{{route('profile')}}" class="btn btn-block btn-primary">Cancel</a>
</div>
</div>
</form>
The deleting of the user's picture (reverting to the default avatar, in other words, is done via AJAX):
(function() {
//Delete Avatar
$('#delete-avatar').on('click', function(evt) {
evt.preventDefault();
var $avatar = $('#avatar-container').find('img');
var $topAvatar = $('#top_avatar');
var $trashIcon = $(this);
var defaultAvatar = APP_URL + '/images/avatars/default.png';
//Get user's ID
var id = $(this).data('uid');
if (confirm('Delete the avatar?')) {
var CSRF_TOKEN = $('meta[name="csrf-token"]').attr('content');
$.ajax({
url: APP_URL + '/dashboard/profile/deleteavatar/' + id,
method: 'POST',
data: {
id: id,
_token: CSRF_TOKEN,
},
success: function() {
$avatar.attr('src', defaultAvatar);
$topAvatar.attr('src', defaultAvatar);
$trashIcon.remove();
}
});
}
});
})();
Questions:
- Could the code be significantly "shortened"?
- Are there better alternatives to the means I have chosen to do the various "actions" (inserting/updating the avatar and bio, etc)?
- How can this be improved (in any way)?
2 Answers 2
The suggestions below should allow the code to be shortened and improved.
Update method is a bit long
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.
namespace App\Http\Requests;
use Auth;
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,'. Auth::user()->id],
'avatar' => ['mimes:jpeg, jpg, png, gif', 'max:2048'],
];
}
}
If the validation fails and the request was an XHR request, an HTTP response with a 422 status code will be returned to the user including a JSON representation of the validation errors..
Then that subclass can be injected instead of Illuminate\Http\Request
in the update
method arguments and use $request->all()
to get the fields to pass to $current_user->update()
.
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 can be added to a group of routes
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');
});
Also, for the sake of readability (e.g. see section 2.3 of PSR-12) it would be wise to alias the profile controller using a use
statement.
use App\Http\Controllers\Dashboard\UserProfileController;
Then each reference can simply be UserProfileController
Instead of the fully qualified name.
Resource Controller could be used
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 PUT
or PATCH
.
Remember to add testing
Laravel offers great support for writing feature tests to ensure the routes output what you expect. It appears there is already a factory for the user and a migration for the user table so those could be used with the RefreshDatabase trait in tests.
The tests could use the SQLite database engine for testing- simply by uncommenting the lines 24 and 25 of phpunit.xml.
Feature tests can make great use of the HTTP test functions available- e.g. requesting routes acting as a user (see the example in that section about using a model factory to generate and authenticate a user) and ensuring the status is okay or redirected to a certain route with assertRedirect()
.
If using a formRequest subclass as suggested above an assertion could be made that the response code is 422 for invalid input (e.g. missing required field, wrong type of field, etc).
JavaScript can be simplified slightly with jQuery shortcuts
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();
}
});
-
\$\begingroup\$ Great advice. However, when I go to
/dashboard/profile/update
I am not redirected to the login page... \$\endgroup\$Razvan Zamfir– Razvan Zamfir2021年06月04日 09:28:15 +00:00Commented Jun 4, 2021 at 9:28 -
\$\begingroup\$ I checked out the code suggestion and found that
$current_user
was undefined in that example forUserUpdateRequest
. Updating it works in my sample test and I have updated the example above. \$\endgroup\$2021年06月06日 04:25:28 +00:00Commented Jun 6, 2021 at 4:25
I think I'll give it a shot. :)
1. Could the code be significantly "shortened"?
I think controller wise, it's okay. It's what I've learnt on Laracasts as well, almost the same thing.
Anw, here are my 2 cents :)
userprofile.blade.php
@if ($errors->has('first_name'))
<span class="errormsg text-danger">{{ $errors->first('first_name') }}</span>
@endif
can be done with:
@error ('first_name')
<span class="errormsg text-danger">{{ $message }}</span>
@enderror
Also, the {{csrf_field()}}
can be replaced with @csrf
2. Are there better alternatives to do the various "actions"?
From what I've learnt and experience, I do recommend checking out a few starter packages:
- Laravel Breeze (with Inertia)
- Laravel Jetstream (With Inertia or Livewire)
These little starter kits come with authentication pre-installed. And by looking into it, I'm pretty sure you can make the most use out of these kits. (With smarts like you, I'm pretty sure avatar implementation should be no issue.) :)
3. How can this be improved (in any way)?
May I suggest Vue? I personally think it's beautiful because everything is in a component. Which includes event listeners, html.
It's can also easily be injected into your blade file.
Major plus point: Paired with Inertia, your pages don't have to make a request every time you visit a page. Inertia automatically intercepts a request and makes a partial page reload, which makes your whole blog significantly faster.
This can be included in your blog's navbar, perhaps? Maybe for loading tags etc. Here's a link to their docs
Just a suggestion ̄\_(ツ)_/ ̄ Let's say, at any point you want to make single page applications (SPA) in your blog, you can do so with Vue.