Skip to content

Commit ac39892

Browse files
committed
bug #105 [bug] ensure all requests are removed for user (kbond)
This PR was merged into the master branch. Discussion ---------- [bug] ensure all requests are removed for user A user can have multiple valid requests. This change removes them all when any one of them is used to successfully reset the user's password. I think the BC break is negligible. Only unfortunate thing is I think the method name no longer implies what should happen here. Commits ------- e27a702 [bug] ensure all requests are removed for user
2 parents 0a66f6a + e27a702 commit ac39892

File tree

4 files changed

+45
-7
lines changed

4 files changed

+45
-7
lines changed

src/Persistence/Repository/ResetPasswordRequestRepositoryTrait.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,13 @@ public function getMostRecentNonExpiredRequestDate(object $user): ?\DateTimeInte
6161

6262
public function removeResetPasswordRequest(ResetPasswordRequestInterface $resetPasswordRequest): void
6363
{
64-
$this->getEntityManager()->remove($resetPasswordRequest);
65-
$this->getEntityManager()->flush();
64+
$this->createQueryBuilder('t')
65+
->delete()
66+
->where('t.user = :user')
67+
->setParameter('user', $resetPasswordRequest->getUser())
68+
->getQuery()
69+
->execute()
70+
;
6671
}
6772

6873
public function removeExpiredResetPasswordRequests(): int

src/Persistence/ResetPasswordRequestRepositoryInterface.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,10 @@ public function findResetPasswordRequest(string $selector): ?ResetPasswordReques
5353
public function getMostRecentNonExpiredRequestDate(object $user): ?\DateTimeInterface;
5454

5555
/**
56-
* Remove a single password reset request from persistence.
56+
* Remove this reset password request from persistence and any other for this user.
57+
*
58+
* This method should remove this ResetPasswordRequestInterface and also all
59+
* other ResetPasswordRequestInterface objects in storage for the same user.
5760
*/
5861
public function removeResetPasswordRequest(ResetPasswordRequestInterface $resetPasswordRequest): void;
5962

tests/Fixtures/Entity/ResetPasswordTestFixtureRequest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ final class ResetPasswordTestFixtureRequest implements ResetPasswordRequestInter
2626
* @ORM\GeneratedValue()
2727
* @ORM\Column(type="integer")
2828
*/
29-
private $id;
29+
public $id;
3030

3131
/**
3232
* @ORM\Column(type="string", nullable=true)
@@ -68,5 +68,6 @@ public function getHashedToken(): string
6868

6969
public function getUser(): object
7070
{
71+
return $this->user;
7172
}
7273
}

tests/FunctionalTests/Persistence/ResetPasswordRequestRepositoryTest.php

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,16 +152,45 @@ public function testGetMostRecentNonExpiredRequestDateReturnsNullIfRequestNotFou
152152

153153
public function testRemoveResetPasswordRequestRemovedGivenObjectFromPersistence(): void
154154
{
155-
$fixture = new ResetPasswordTestFixtureRequest();
155+
$userFixture = new ResetPasswordTestFixtureUser();
156+
$requestFixture = new ResetPasswordTestFixtureRequest();
157+
$requestFixture->user = $userFixture;
156158

157-
$this->manager->persist($fixture);
159+
$this->manager->persist($requestFixture);
160+
$this->manager->persist($userFixture);
158161
$this->manager->flush();
159162

160-
$this->repository->removeResetPasswordRequest($fixture);
163+
$this->repository->removeResetPasswordRequest($requestFixture);
161164

162165
$this->assertCount(0, $this->repository->findAll());
163166
}
164167

168+
public function testRemoveResetPasswordRequestRemovesAllRequestsForUser(): void
169+
{
170+
$userFixtureA = new ResetPasswordTestFixtureUser();
171+
$userFixtureB = new ResetPasswordTestFixtureUser();
172+
$requestFixtureA = new ResetPasswordTestFixtureRequest();
173+
$requestFixtureA->user = $userFixtureA;
174+
$requestFixtureB = new ResetPasswordTestFixtureRequest();
175+
$requestFixtureB->user = $userFixtureA;
176+
$requestFixtureC = new ResetPasswordTestFixtureRequest();
177+
$requestFixtureC->user = $userFixtureB;
178+
179+
$this->manager->persist($requestFixtureA);
180+
$this->manager->persist($requestFixtureB);
181+
$this->manager->persist($requestFixtureC);
182+
$this->manager->persist($userFixtureA);
183+
$this->manager->persist($userFixtureB);
184+
$this->manager->flush();
185+
186+
$this->repository->removeResetPasswordRequest($requestFixtureB);
187+
188+
$requests = $this->repository->findAll();
189+
190+
$this->assertCount(1, $requests);
191+
$this->assertSame($requestFixtureC->id, $requests[0]->id);
192+
}
193+
165194
public function testRemovedExpiredResetPasswordRequestsOnlyRemovedExpiredRequestsFromPersistence(): void
166195
{
167196
$expiredFixture = new ResetPasswordTestFixtureRequest();

0 commit comments

Comments
 (0)