framework: Response status should be 200 instead of 201 when returned model was created from within a test before making the request

Laravel Version: 5.7
PHP Version: 7.2
Database Driver & Version: Postgres 9.6

Description:

In a PHPUnit test, when creating a User through a factory and returning Auth::user() from a controller/action, the response status is 201, where it should be 200.

This was addressed in #21107 #25775 and #23270 (and probably others). However, the suggested solution of ->refresh()ing the user before passing it to ->actingAs() doesn’t work for me. Further, i think it should not be necessary to refresh the user in order to work around the framework trying to be smart. It’s not difficult to manually set the response status to 201 in an endpoint where a fresh resource is created. So the “smartness” of wasRecentlyCreated resulting in a 201 status code is not that valuable. Especially if it’s only half smart and doesn’t distinguish between resources created within the request/response lifecycle and resources created by a test before making the request.

I consider the following scenario a valid test that should actually pass. If you agree on that, let’s look for a solution together. If you don’t agree that the test should pass, feel free to close this issue, and sugggest a reliable approach to get the proper status code that is consistent between test and production environment.

Steps to reproduce:

1. Create a test case:

$user = factory(User::class)->create();

$response = $this->actingAs($user, 'api')->json('GET', '/api/v1/users/me');
$response->assertOk();

2. Create the corresponding action:

<?php

namespace App\Http\Actions\User;

use App\Http\Resources\UserResource;
use Illuminate\Support\Facades\Auth;

class ShowMeAction
{
    public function __invoke()
    {
        return new UserResource(Auth::user());
    }
}

3. Run the test

4. Call ->refresh() on the user before passing it to actingAs()

Contrary to what was suggested as a solution in one of the quoted issues, even with the refresh() call, the test fails for me:

Response status code [201] does not match expected 200 status code.
Failed asserting that false is true.

About this issue

Original URL
State: closed
Created 6 years ago
Comments: 24 (14 by maintainers)

Most upvoted comments

Anyway - 2 ways to approach this:

Factory sets wasRecentlyCreated to false - but I dont think that is valid
Anything you pass to actingAs() overrides wasRecentlyCreated to false - because you cant be pre-authenticated against a new resource. I cant think of a time this would ever be valid? Can anyone think of any?

laurencei on Oct 2, 2018

Remember the point of 201 is the request was successful and you created a resource.

But it /test above - I didnt create a resource inside of that request.

The fact I created a $user prior to the test is irrelevant to the purpose of the request lifecycle.

laurencei on Oct 2, 2018

use:

$user = factory(User::class)->create();

Passport::actingAs(User::find($user->id));

This will return 200.

memochou1993 on Mar 13, 2019

So fresh install of Laravel 5.7:

public function testBasicTest()
{
    $user = factory(\App\User::class)->create();
    $response = $this->actingAs($user)->json('GET', '/test');
    $response->assertOk();
}

and

Route::get('/test', function () {
    return Auth::user();
});

gives a 201.

This is a bug - because the user existed before the start of the reqeust. i.e. we did not create a new user and return it inside of /test.

The problem seems to be actingAs() is taking the literal output of $user and using that inside of the request.

laurencei on Oct 2, 2018

@driesvints - but if the model was created inside a factory before the request - then returning 201 is incorrect - because wasRecentlyCreated should only apply to the lifecycle of the request.

I havent fully tested - but this looks like a bug to me.

laurencei on Oct 2, 2018