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

[7.x] Add withToken() helper method #33075

Merged
merged 5 commits into from
Jun 3, 2020
Merged

[7.x] Add withToken() helper method #33075

merged 5 commits into from
Jun 3, 2020

Conversation

aryehraber
Copy link
Contributor

This PR adds a small helper method for tests using Authorization Bearer tokens as part of request headers. It was inspired by the existing $request->bearerToken() method on the Illuminate\Http\Request class, which is also really useful.

I feel this allows requests inside tests to read a little nicer, as well as helps against typos when writing "Authorization" and "Bearer" over and over, and instead IDEs can autocomplete the method.

$response = $this->withHeader('Authorization', 'Bearer test_token')->postJson('/some-endpoint');

$response = $this->withBearerToken('test_token')->postJson('/some-endpoint');

There are no breaking changes, and I've added a small test to verify everything works as expected.

Happy to make changes if there's any feedback!

@aryehraber
Copy link
Contributor Author

I just noticed that the Illuminate\Http\Client\PendingRequest class uses withToken() which is even better/shorter and allows overriding the default "Bearer" type.

I'll push an update to make this method more dynamic shortly!

@aryehraber aryehraber marked this pull request as draft June 2, 2020 19:34
@GrahamCampbell
Copy link
Member

Please also rebase this PR against the latest 7.x codebase. :)

@aryehraber
Copy link
Contributor Author

Whoops, I think I did something wrong while rebasing! Apologies if this sent out a bunch of notifications 🤦‍♂️

@GrahamCampbell Sorry about this, what's the best way to fix this? Should I create a new PR and ensure my branch is on latest 7.x?

@lcdss
Copy link

lcdss commented Jun 2, 2020

It can be easier just copy the your commits hash sequentially (since it's not many), reset hard the branch to the latest upstream commit, cherry pick your commits and then a push force.

@aryehraber
Copy link
Contributor Author

Thanks @lcdss, I ended up doing something similar -- looks ok now (I think)

@aryehraber
Copy link
Contributor Author

The PR is now ready for review!

The method changed slightly since my original message, so here's an updated snippet to demonstrate its usefulness:

$response = $this->withHeader('Authorization', 'Bearer test_token')->postJson('/some-endpoint');

$response = $this->withToken('test_token')->postJson('/some-endpoint');

Much shorter and more readable I think — hope you agree 😊

Let me know if there's any feedback!

@aryehraber aryehraber marked this pull request as ready for review June 2, 2020 20:44
@aryehraber aryehraber changed the title [7.x] Add withBearerToken() helper method [7.x] Add withToken() helper method Jun 2, 2020
@taylorotwell taylorotwell merged commit 188f89f into laravel:7.x Jun 3, 2020
@aryehraber aryehraber deleted the testing-authorization-helper branch June 3, 2020 13:59
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.

4 participants