Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.6] Improve session errors assertions #23055

Merged
merged 1 commit into from
Feb 7, 2018
Merged

[5.6] Improve session errors assertions #23055

merged 1 commit into from
Feb 7, 2018

Conversation

roberto-aguilar
Copy link
Contributor

@roberto-aguilar roberto-aguilar commented Feb 7, 2018

In the previous implementation of TestResponse::assertSessionHasErrors i found that was not possible to find errors that had an integer key, for example:

// If somewhere in your application you did
return redirect()->back()->withErrors('Something wrong happened.')

// You ended up with a message bag similar to this one in the session errors
$errors = new Illuminate\Support\MessageBag('Something wrong happened.');
$errors->get(0); // Returns 'Something wrong happened.'

// And the following assertion was failing, even when the error was there
$this->post('/somewhere')
     ->assertSessionHasErrors(['Something wrong happened.']);

This was happening because if the assertion detected that the key was an integer, it was sending the error value to the MessageBag::has method, which was incorrect because that method uses the key to determine if there is any messages for it.

Also, as part of the refactor i achieved the following assertion API improvements, which now allow to find:

/*
 * All the messages for a given key
 */
$this->post('/somewhere', ['password' => 'abc 123'])
     ->assertSessionHasErrors([
         'password' => [
             'The password may only contain letters, numbers, and dashes.',
             'The password must be at least 8 characters.',
         ],
     ]);

/*
 * A subset of the messages for a given key
 */
$this->post('/somewhere', ['password' => 'abc 123'])
     ->assertSessionHasErrors([
         'password' => [
             'The password must be at least 8 characters.',
         ],
     ]);

/*
 * An specific message for a given key (without the need for a wrapping array).
 */
$this->post('/somewhere', ['password' => 'abc 123'])
     ->assertSessionHasErrors([
         'password' => 'The password must be at least 8 characters.',
     ]);

/*
 * Messages with integer keys
 */
$this->post('/somewhere')
     ->assertSessionHasErrors([
         'Something wrong happened.',
         'And this happened too.',
     ]);

/*
 * A subset of messages with integer keys
 */
$this->post('/somewhere')
     ->assertSessionHasErrors([
         'Something wrong happened.',
     ]);

/*
 * An specific message with an integer key (without the need for a wrapping array).
 */
$this->post('/somewhere')
     ->assertSessionHasErrors('Something wrong happened.');

@roberto-aguilar roberto-aguilar changed the title Improve session errors assertions [5.5] Improve session errors assertions Feb 7, 2018
In the previous implementation of `TestResponse::assertSessionHasErrors` i
found that was not possible to find errors that had an integer key.

This was happening because if the assertion detected that the key was an
integer, it was sending the error value to the `MessageBag::has` method,
which was incorrect because that method uses the key to determine if
there is any messages for it.
@carusogabriel
Copy link
Contributor

👍 for 5.6

@taylorotwell
Copy link
Member

Are there any breaking changes in this PR at all @DojoGeekRA?

@roberto-aguilar
Copy link
Contributor Author

roberto-aguilar commented Feb 7, 2018

Hi @taylorotwell,

No, there are no breaking changes, the previous API still works 😃

@roberto-aguilar
Copy link
Contributor Author

roberto-aguilar commented Feb 7, 2018

@carusogabriel yeah, i have no problem with that, but i though that the LTS users could still benefit from this 😄

Thanks for the suggestion!

@taylorotwell taylorotwell changed the base branch from 5.5 to 5.6 February 7, 2018 21:14
@taylorotwell taylorotwell merged commit 4fbb54b into laravel:5.6 Feb 7, 2018
@roberto-aguilar roberto-aguilar deleted the hotfix/session-errors-assertion branch February 7, 2018 21:25
@taylorotwell
Copy link
Member

Actually broke the most simple use case of this function.

@taylorotwell
Copy link
Member

Feel free to re-submit with tests proving this actually works.

@roberto-aguilar
Copy link
Contributor Author

roberto-aguilar commented Feb 9, 2018

I'm sorry @taylorotwell i tried several scenarios but missed the one with just the error key.

Thanks for the help, i have been working on this and will submit again when is ready 😃

@GrahamCampbell GrahamCampbell changed the title [5.5] Improve session errors assertions [5.6] Improve session errors assertions Feb 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants