Skip to content
This repository was archived by the owner on Jun 29, 2022. It is now read-only.

AuthMiddleware test #140

Merged
merged 4 commits into from
Nov 2, 2019
Merged

AuthMiddleware test #140

merged 4 commits into from
Nov 2, 2019

Conversation

zhukovra
Copy link
Contributor

Q A
Is bugfix?
New feature?
Breaks BC?
Tests pass? ✔️
Fixed issues #105

I've made one change in AuthMiddleware. I think that without the change these lines is useless

private const REQUEST_NAME = 'auth_user';
private $requestName = self::REQUEST_NAME;

$request->withAttribute($this->requestName, $identity);

public function setRequestName(string $name): void
{
$this->requestName = $name;
}

@samdark samdark added type:test Test status:code review The pull request needs review. labels Oct 31, 2019
@demonking
Copy link

$request->withAttribute($this->requestName, $identity);

This seems right, because it doesn't return $this, it's a clone of $this :

https://github.com/Nyholm/psr7/blob/master/src/ServerRequest.php#L143-L149

@zhukovra
Copy link
Contributor Author

zhukovra commented Nov 2, 2019

$request->withAttribute($this->requestName, $identity);

This seems right, because it doesn't return $this, it's a clone of $this :

https://github.com/Nyholm/psr7/blob/master/src/ServerRequest.php#L143-L149

It is part of PSR: https://github.com/php-fig/http-message/blob/efd67d1dc14a7ef4fc4e518e7dee91c271d524e4/src/ServerRequestInterface.php#L235-L237

@samdark samdark merged commit 6328404 into yiisoft:master Nov 2, 2019
@samdark
Copy link
Member

samdark commented Nov 2, 2019

Merged. Thank you!

@samdark samdark removed the status:code review The pull request needs review. label Nov 2, 2019
devanych pushed a commit that referenced this pull request Feb 1, 2021
devanych pushed a commit that referenced this pull request Feb 1, 2021
devanych pushed a commit that referenced this pull request Feb 1, 2021
devanych pushed a commit that referenced this pull request Feb 1, 2021
devanych pushed a commit that referenced this pull request Feb 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants