LaravelDaily/Test-Laravel-Eloquent-Basics

Task 7 - update or new record

Closed this issue · 2 comments

Test 7 may be solved with both

$password = bcrypt('password');
$user = User::updateOrCreate(compact('name', 'email'), compact('password'));

and

$password = bcrypt('password');
$user = User::updateOrCreate(compact('name'), compact('email', 'password'));

which should not be accepted, according to the task.

Indeed, the former creates a new record during the test,
whilst the latter updates the password even if the record is found.

I've created this PR #159 to improve the tests (which of course fails the tests on GitHub because it has just the test fix).

P.S. I then noticed that #89 and #119 are related.
Also, in #89 this PR #56 is referenced to fix the issue too (and the are changes very similar).

Neither option is correct because you need to create a password if the user does not exist. In the task, you are instructed to create the user's password (using a random one). After doing your update, the user will not be able to log in.

updateOrCreate

There is an issue with those options as they unexpectedly modify the password of an existing user, which is not the desired behavior. Our requirement is to generate a password only when the user is not found and then update the email. We have two solutions to address this issue - one utilizing updateOrCreate, and the other without it.

First Solution ( With updateOrCreate )

We first search for the user by name. If the user is found, we use the old password and update the email. Otherwise, we generate a new password. However, this solution has a potential issue as it updates the password and email even if they are not changed, which could result in unnecessary update queries. I'm uncertain if the method checks for attribute changes and only updates those.

app/Http/Controllers/UserController.php inside check_update function
$user = User::where('name', $name)->first();
$user = User::updateOrCreate(
    compact('name'),
    [
        'email' => $email,
        'password' => $user ? $user->password : bcrypt(Str::random(8))
    ]
);

Second Solution ( Without updateOrCreate )

My second solution involves using the findOr function, which searches for a user by name. If the user is not found, it falls back to a callback function where a new user is created. Then, I check if the user's email is different from the new email. If there is a difference, I update the email, which resolves the issue of updating the email and password even if the data is unchanged.

app/Http/Controllers/UserController.php inside check_update function
 $user = User::where('name', $name)->firstOr(function () use ($name, $email) {
            return User::create([
                'name' => $name,
                'email' => $email,
                'password' => bcrypt(Str::random(8))
            ]);
        });

if ($user->email != $email) {
          $user->email = $email;
          $user->save();
     }