From 51d1760f7961ae86832e81d6f2ab7c7f7cccc791 Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Tue, 19 Mar 2024 17:15:57 +0100 Subject: [PATCH 1/2] Clean up CancelDonationUseCase Since user can no longer cancel donations, remove all code that distinguishes between users and administrators. Remove reference to mailer interface, since that is no longer needed. This'll help improving the TemplateMailerInterface to accept only objects. Add `allow-plugins` section to composer.json to allow for new code style. The changes in the CancelDonationUseCase and CancelDonationRequest constructor are a BC break. --- composer.json | 3 +- .../CancelDonation/CancelDonationRequest.php | 12 +- .../CancelDonation/CancelDonationUseCase.php | 56 ++------ .../CancelDonationUseCaseTest.php | 128 ++---------------- .../CancelDonationRequestTest.php | 41 ------ 5 files changed, 25 insertions(+), 215 deletions(-) delete mode 100644 tests/Unit/UseCases/CancelDonation/CancelDonationRequestTest.php diff --git a/composer.json b/composer.json index 64b8f91a..a4b579fc 100644 --- a/composer.json +++ b/composer.json @@ -59,7 +59,8 @@ "config": { "discard-changes": true, "allow-plugins": { - "composer/package-versions-deprecated": true + "composer/package-versions-deprecated": true, + "dealerdirect/phpcodesniffer-composer-installer": true } }, "extra": { diff --git a/src/UseCases/CancelDonation/CancelDonationRequest.php b/src/UseCases/CancelDonation/CancelDonationRequest.php index 3cb1e561..e5a4d50c 100644 --- a/src/UseCases/CancelDonation/CancelDonationRequest.php +++ b/src/UseCases/CancelDonation/CancelDonationRequest.php @@ -6,12 +6,7 @@ class CancelDonationRequest { - private int $donationId; - private ?string $authorizedUser; - - public function __construct( int $donationId, ?string $authorizedUser = null ) { - $this->donationId = $donationId; - $this->authorizedUser = $authorizedUser; + public function __construct( private readonly int $donationId, private readonly string $authorizedUser ) { } public function getDonationId(): int { @@ -19,13 +14,10 @@ public function getDonationId(): int { } public function isAuthorizedRequest(): bool { - return $this->authorizedUser !== null; + return true; } public function getUserName(): string { - if ( $this->authorizedUser == null ) { - throw new \LogicException( "Tried to access user name of unauthorized user. Call isAuthorizedRequest() first!" ); - } return $this->authorizedUser; } diff --git a/src/UseCases/CancelDonation/CancelDonationUseCase.php b/src/UseCases/CancelDonation/CancelDonationUseCase.php index 77561c07..ad6f6dfd 100644 --- a/src/UseCases/CancelDonation/CancelDonationUseCase.php +++ b/src/UseCases/CancelDonation/CancelDonationUseCase.php @@ -4,28 +4,24 @@ namespace WMDE\Fundraising\DonationContext\UseCases\CancelDonation; -use WMDE\EmailAddress\EmailAddress; -use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\Domain\Repositories\GetDonationException; use WMDE\Fundraising\DonationContext\Domain\Repositories\StoreDonationException; use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Infrastructure\DonationEventLogger; -use WMDE\Fundraising\DonationContext\Infrastructure\TemplateMailerInterface; use WMDE\Fundraising\PaymentContext\UseCases\CancelPayment\CancelPaymentUseCase; use WMDE\Fundraising\PaymentContext\UseCases\CancelPayment\FailureResponse; class CancelDonationUseCase { - private const LOG_MESSAGE_DONATION_STATUS_CHANGE = 'frontend: storno'; private const LOG_MESSAGE_DONATION_STATUS_CHANGE_BY_ADMIN = 'cancelled by user: %s'; public function __construct( - private DonationRepository $donationRepository, - private TemplateMailerInterface $mailer, - private DonationAuthorizationChecker $authorizationService, - private DonationEventLogger $donationLogger, - private CancelPaymentUseCase $cancelPaymentUseCase ) { + private readonly DonationRepository $donationRepository, + private readonly DonationAuthorizationChecker $authorizationService, + private readonly DonationEventLogger $donationLogger, + private readonly CancelPaymentUseCase $cancelPaymentUseCase + ) { } public function cancelDonation( CancelDonationRequest $cancellationRequest ): CancelDonationResponse { @@ -58,23 +54,11 @@ public function cancelDonation( CancelDonationRequest $cancellationRequest ): Ca $this->donationLogger->log( $donation->getId(), $this->getLogMessage( $cancellationRequest ) ); - try { - $this->sendConfirmationEmail( $cancellationRequest, $donation ); - } catch ( \RuntimeException $ex ) { - return new CancelDonationResponse( - $cancellationRequest->getDonationId(), - CancelDonationResponse::MAIL_DELIVERY_FAILED - ); - } - return $this->newSuccessResponse( $cancellationRequest ); } public function getLogMessage( CancelDonationRequest $cancellationRequest ): string { - if ( $cancellationRequest->isAuthorizedRequest() ) { - return sprintf( self::LOG_MESSAGE_DONATION_STATUS_CHANGE_BY_ADMIN, $cancellationRequest->getUserName() ); - } - return self::LOG_MESSAGE_DONATION_STATUS_CHANGE; + return sprintf( self::LOG_MESSAGE_DONATION_STATUS_CHANGE_BY_ADMIN, $cancellationRequest->getUserName() ); } public function requestIsAllowedToModifyDonation( CancelDonationRequest $cancellationRequest ): bool { @@ -82,7 +66,8 @@ public function requestIsAllowedToModifyDonation( CancelDonationRequest $cancell return $this->authorizationService->systemCanModifyDonation( $cancellationRequest->getDonationId() ); } - return $this->authorizationService->userCanModifyDonation( $cancellationRequest->getDonationId() ); + // Users on the frontend are no longer allowed to cancel donations + return false; } private function newFailureResponse( CancelDonationRequest $cancellationRequest ): CancelDonationResponse { @@ -99,29 +84,4 @@ private function newSuccessResponse( CancelDonationRequest $cancellationRequest ); } - private function sendConfirmationEmail( CancelDonationRequest $cancellationRequest, Donation $donation ): void { - if ( $cancellationRequest->isAuthorizedRequest() ) { - return; - } - if ( !$donation->getDonor()->hasEmailAddress() ) { - return; - } - $this->mailer->sendMail( - new EmailAddress( $donation->getDonor()->getEmailAddress() ), - $this->getConfirmationMailTemplateArguments( $donation ) - ); - } - - /** - * @param Donation $donation - * - * @return array - */ - private function getConfirmationMailTemplateArguments( Donation $donation ): array { - return [ - 'donationId' => $donation->getId(), - 'recipient' => $donation->getDonor()->getName()->toArray(), - ]; - } - } diff --git a/tests/Integration/UseCases/CancelDonation/CancelDonationUseCaseTest.php b/tests/Integration/UseCases/CancelDonation/CancelDonationUseCaseTest.php index c35c6ac8..36a35224 100644 --- a/tests/Integration/UseCases/CancelDonation/CancelDonationUseCaseTest.php +++ b/tests/Integration/UseCases/CancelDonation/CancelDonationUseCaseTest.php @@ -6,18 +6,15 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use WMDE\EmailAddress\EmailAddress; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Infrastructure\DonationEventLogger; -use WMDE\Fundraising\DonationContext\Infrastructure\TemplateMailerInterface; use WMDE\Fundraising\DonationContext\Tests\Data\ValidDonation; use WMDE\Fundraising\DonationContext\Tests\Fixtures\DonationEventLoggerSpy; use WMDE\Fundraising\DonationContext\Tests\Fixtures\DonationRepositorySpy; use WMDE\Fundraising\DonationContext\Tests\Fixtures\FakeDonationRepository; use WMDE\Fundraising\DonationContext\Tests\Fixtures\SucceedingDonationAuthorizerSpy; -use WMDE\Fundraising\DonationContext\Tests\Fixtures\TemplateBasedMailerSpy; use WMDE\Fundraising\DonationContext\UseCases\CancelDonation\CancelDonationRequest; use WMDE\Fundraising\DonationContext\UseCases\CancelDonation\CancelDonationUseCase; use WMDE\Fundraising\PaymentContext\UseCases\CancelPayment\CancelPaymentUseCase; @@ -31,19 +28,19 @@ */ class CancelDonationUseCaseTest extends TestCase { + private const AUTHORIZED_USER = 'admin_adminson'; + private function newCancelDonationUseCase( DonationRepository $repository = null, - TemplateMailerInterface $mailer = null, DonationAuthorizationChecker $authorizer = null, DonationEventLogger $logger = null, CancelPaymentUseCase $cancelPaymentUseCase = null ): CancelDonationUseCase { return new CancelDonationUseCase( $repository ?? new FakeDonationRepository(), - $mailer ?? new TemplateBasedMailerSpy( $this ), $authorizer ?? new SucceedingDonationAuthorizerSpy(), $logger ?? new DonationEventLoggerSpy(), - $cancelPaymentUseCase ?? $this->getSucceedingCancelPaymentUseCase() + $cancelPaymentUseCase ?? $this->getSucceedingCancelPaymentUseCase() ); } @@ -60,13 +57,13 @@ private function getFailingCancelPaymentUseCase(): CancelPaymentUseCase&MockObje } public function testGivenIdOfUnknownDonation_cancellationIsNotSuccessful(): void { - $response = $this->newCancelDonationUseCase()->cancelDonation( new CancelDonationRequest( 1 ) ); + $response = $this->newCancelDonationUseCase()->cancelDonation( new CancelDonationRequest( 1, self::AUTHORIZED_USER ) ); $this->assertFalse( $response->cancellationSucceeded() ); } public function testResponseContainsDonationId(): void { - $response = $this->newCancelDonationUseCase()->cancelDonation( new CancelDonationRequest( 1337 ) ); + $response = $this->newCancelDonationUseCase()->cancelDonation( new CancelDonationRequest( 1337, self::AUTHORIZED_USER ) ); $this->assertSame( 1337, $response->getDonationId() ); } @@ -76,7 +73,7 @@ public function testGivenIdOfCancellableDonation_cancellationIsSuccessful(): voi $repository = new FakeDonationRepository(); $repository->storeDonation( $donation ); - $request = new CancelDonationRequest( $donation->getId() ); + $request = new CancelDonationRequest( $donation->getId(), self::AUTHORIZED_USER ); $response = $this->newCancelDonationUseCase( repository: $repository )->cancelDonation( $request ); $donation = $repository->getDonationById( $donation->getId() ); @@ -91,7 +88,7 @@ public function testGivenIdOfNonCancellableDonation_cancellationIsNotSuccessful( $repository = new FakeDonationRepository(); $repository->storeDonation( $donation ); - $request = new CancelDonationRequest( $donation->getId() ); + $request = new CancelDonationRequest( $donation->getId(), self::AUTHORIZED_USER ); $response = $this->newCancelDonationUseCase( repository: $repository, cancelPaymentUseCase: $this->getFailingCancelPaymentUseCase() @@ -105,50 +102,6 @@ private function newCancelableDonation(): Donation { return ValidDonation::newDirectDebitDonation(); } - public function testWhenDonationGetsCancelled_cancellationConfirmationEmailIsSent(): void { - $donation = $this->newCancelableDonation(); - $mailer = new TemplateBasedMailerSpy( $this ); - $repository = new FakeDonationRepository(); - $repository->storeDonation( $donation ); - - $request = new CancelDonationRequest( $donation->getId() ); - $response = $this->newCancelDonationUseCase( - repository: $repository, - mailer: $mailer - )->cancelDonation( $request ); - - $this->assertTrue( $response->cancellationSucceeded() ); - $mailer->assertCalledOnceWith( - new EmailAddress( $donation->getDonor()->getEmailAddress() ), - [ - 'recipient' => [ - 'salutation' => ValidDonation::DONOR_SALUTATION, - 'title' => ValidDonation::DONOR_TITLE, - 'firstName' => ValidDonation::DONOR_FIRST_NAME, - 'lastName' => ValidDonation::DONOR_LAST_NAME, - ], - 'donationId' => 1 - ] - ); - } - - public function testWhenDonationGetsCancelled_logEntryNeededByBackendIsWritten(): void { - $donation = $this->newCancelableDonation(); - $logger = new DonationEventLoggerSpy(); - $repository = new FakeDonationRepository(); - $repository->storeDonation( $donation ); - - $this->newCancelDonationUseCase( - repository: $repository, - logger: $logger - )->cancelDonation( new CancelDonationRequest( $donation->getId() ) ); - - $this->assertSame( - [ [ $donation->getId(), 'frontend: storno' ] ], - $logger->getLogCalls() - ); - } - public function testWhenDonationGetsCancelledByAdmin_adminUserNameIsWrittenAsLogEntry(): void { $donation = $this->newCancelableDonation(); $logger = new DonationEventLoggerSpy(); @@ -158,51 +111,27 @@ public function testWhenDonationGetsCancelledByAdmin_adminUserNameIsWrittenAsLog $this->newCancelDonationUseCase( repository: $repository, logger: $logger - )->cancelDonation( new CancelDonationRequest( $donation->getId(), "coolAdmin" ) ); + )->cancelDonation( new CancelDonationRequest( $donation->getId(), self::AUTHORIZED_USER ) ); $this->assertSame( - [ [ $donation->getId(), 'cancelled by user: coolAdmin' ] ], + [ [ $donation->getId(), 'cancelled by user: admin_adminson' ] ], $logger->getLogCalls() ); } public function testGivenIdOfNonCancellableDonation_nothingIsWrittenToTheLog(): void { $logger = new DonationEventLoggerSpy(); - $this->newCancelDonationUseCase( logger: $logger )->cancelDonation( new CancelDonationRequest( 1 ) ); + $this->newCancelDonationUseCase( logger: $logger )->cancelDonation( new CancelDonationRequest( 1, self::AUTHORIZED_USER ) ); $this->assertSame( [], $logger->getLogCalls() ); } - public function testWhenConfirmationMailFails_mailDeliveryFailureResponseIsReturned(): void { - $donation = $this->newCancelableDonation(); - $mailer = $this->newThrowingMailer(); - $repository = new FakeDonationRepository(); - $repository->storeDonation( $donation ); - - $request = new CancelDonationRequest( $donation->getId() ); - $response = $this->newCancelDonationUseCase( - repository: $repository, - mailer: $mailer - )->cancelDonation( $request ); - - $this->assertTrue( $response->cancellationSucceeded() ); - $this->assertTrue( $response->mailDeliveryFailed() ); - } - - private function newThrowingMailer(): TemplateMailerInterface { - $mailer = $this->createMock( TemplateMailerInterface::class ); - - $mailer->method( $this->anything() )->willThrowException( new \RuntimeException() ); - - return $mailer; - } - public function testWhenGetDonationFails_cancellationIsNotSuccessful(): void { $donation = $this->newCancelableDonation(); $repository = new FakeDonationRepository(); $repository->throwOnRead(); - $request = new CancelDonationRequest( $donation->getId() ); + $request = new CancelDonationRequest( $donation->getId(), self::AUTHORIZED_USER ); $response = $this->newCancelDonationUseCase( repository: $repository )->cancelDonation( $request ); $this->assertFalse( $response->cancellationSucceeded() ); @@ -214,7 +143,7 @@ public function testWhenDonationSavingFails_cancellationIsNotSuccessful(): void $repository->storeDonation( $donation ); $repository->throwOnWrite(); - $request = new CancelDonationRequest( $donation->getId() ); + $request = new CancelDonationRequest( $donation->getId(), self::AUTHORIZED_USER ); $response = $this->newCancelDonationUseCase( repository: $repository )->cancelDonation( $request ); $this->assertFalse( $response->cancellationSucceeded() ); @@ -226,7 +155,7 @@ public function testWhenAdminUserCancelsDonation_authorizerChecksIfSystemCanModi $repository = new FakeDonationRepository(); $repository->storeDonation( $donation ); - $request = new CancelDonationRequest( $donation->getId(), "coolAdmin" ); + $request = new CancelDonationRequest( $donation->getId(), self::AUTHORIZED_USER ); $this->newCancelDonationUseCase( repository: $repository, authorizer: $authorizer @@ -236,37 +165,6 @@ public function testWhenAdminUserCancelsDonation_authorizerChecksIfSystemCanModi $this->assertFalse( $authorizer->hasAuthorizedAsUser() ); } - public function testWhenDonorCancelsDonation_authorizerUsesFullAuthorizationCheck(): void { - $donation = $this->newCancelableDonation(); - $authorizer = new SucceedingDonationAuthorizerSpy(); - $repository = new FakeDonationRepository(); - $repository->storeDonation( $donation ); - - $request = new CancelDonationRequest( $donation->getId() ); - $this->newCancelDonationUseCase( - repository: $repository, - authorizer: $authorizer - )->cancelDonation( $request ); - - $this->assertFalse( $authorizer->hasAuthorizedAsAdmin() ); - $this->assertTrue( $authorizer->hasAuthorizedAsUser() ); - } - - public function testWhenAdminUserCancelsDonation_emailIsNotSent(): void { - $mailer = $this->createMock( TemplateMailerInterface::class ); - - $mailer->expects( $this->never() )->method( 'sendMail' ); - - $donation = $this->newCancelableDonation(); - $repository = new FakeDonationRepository(); - $repository->storeDonation( $donation ); - $request = new CancelDonationRequest( $donation->getId(), "coolAdmin" ); - $this->newCancelDonationUseCase( - repository: $repository, - mailer: $mailer - )->cancelDonation( $request ); - } - public function testCanceledDonationIsPersisted(): void { $donation = $this->newCancelableDonation(); $repository = new DonationRepositorySpy( $donation ); diff --git a/tests/Unit/UseCases/CancelDonation/CancelDonationRequestTest.php b/tests/Unit/UseCases/CancelDonation/CancelDonationRequestTest.php deleted file mode 100644 index b4306a16..00000000 --- a/tests/Unit/UseCases/CancelDonation/CancelDonationRequestTest.php +++ /dev/null @@ -1,41 +0,0 @@ -assertSame( $donationID, $cancelDonationRequest->getDonationId() ); - $this->assertFalse( $cancelDonationRequest->isAuthorizedRequest() ); - } - - public function testAuthorizedRequest(): void { - $donationID = 2; - $authUser = "adminUserX"; - $cancelDonationRequest = new CancelDonationRequest( $donationID, $authUser ); - - $this->assertSame( $donationID, $cancelDonationRequest->getDonationId() ); - $this->assertTrue( $cancelDonationRequest->isAuthorizedRequest() ); - $this->assertSame( $authUser, $cancelDonationRequest->getUserName() ); - } - - public function testUnauthorizedRequestHasNoUserName(): void { - $donationID = 70; - $cancelDonationRequest = new CancelDonationRequest( $donationID ); - - $this->assertSame( $donationID, $cancelDonationRequest->getDonationId() ); - $this->expectException( \LogicException::class ); - $cancelDonationRequest->getUserName(); - } -} From 95178dc5f4559d10972e27c5861066b08c9a916f Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Tue, 19 Mar 2024 18:41:24 +0100 Subject: [PATCH 2/2] Improve DonationNotifier implementation Split interface for donor and template mailer Use value objects instead of arrays This is a backwards-breaking change that needs new implementations in the Fundraising Application --- .scrutinizer.yml | 2 +- src/Domain/Model/DonorName.php | 4 +- .../AdminNotificationInterface.php | 12 ++ .../TemplateArgumentsAdmin.php | 18 +++ .../TemplateArgumentsDonation.php | 30 +++++ .../TemplateArgumentsDonationConfirmation.php | 16 +++ .../DonorNotificationInterface.php | 13 ++ ...ailer.php => TemplateDonationNotifier.php} | 69 +++++----- .../TemplateMailerInterface.php | 16 --- ...MailerSpy.php => DonorNotificationSpy.php} | 20 ++- tests/Fixtures/ThrowingAdminNotifier.php | 16 +++ tests/Fixtures/ThrowingDonorNotification.php | 17 +++ tests/Fixtures/ThrowingTemplateMailer.php | 16 --- ...t.php => TemplateDonationNotifierTest.php} | 124 +++++++++--------- 14 files changed, 234 insertions(+), 139 deletions(-) create mode 100644 src/Infrastructure/AdminNotificationInterface.php create mode 100644 src/Infrastructure/DonationNotifier/TemplateArgumentsAdmin.php create mode 100644 src/Infrastructure/DonationNotifier/TemplateArgumentsDonation.php create mode 100644 src/Infrastructure/DonationNotifier/TemplateArgumentsDonationConfirmation.php create mode 100644 src/Infrastructure/DonorNotificationInterface.php rename src/Infrastructure/{DonationMailer.php => TemplateDonationNotifier.php} (50%) delete mode 100644 src/Infrastructure/TemplateMailerInterface.php rename tests/Fixtures/{TemplateBasedMailerSpy.php => DonorNotificationSpy.php} (59%) create mode 100644 tests/Fixtures/ThrowingAdminNotifier.php create mode 100644 tests/Fixtures/ThrowingDonorNotification.php delete mode 100644 tests/Fixtures/ThrowingTemplateMailer.php rename tests/Unit/Infrastructure/{DonationConfirmationMailerTest.php => TemplateDonationNotifierTest.php} (58%) diff --git a/.scrutinizer.yml b/.scrutinizer.yml index 95c6751d..d4d5bfa1 100644 --- a/.scrutinizer.yml +++ b/.scrutinizer.yml @@ -17,7 +17,7 @@ filter: build: image: default-bionic environment: - php: 8.1.4 + php: 8.2 nodes: analysis: tests: diff --git a/src/Domain/Model/DonorName.php b/src/Domain/Model/DonorName.php index 8d690e34..7e9684df 100644 --- a/src/Domain/Model/DonorName.php +++ b/src/Domain/Model/DonorName.php @@ -9,7 +9,9 @@ interface DonorName { public function getFullName(): string; /** - * @return string[] + * Get name components for usage in templates + * + * @return array */ public function toArray(): array; diff --git a/src/Infrastructure/AdminNotificationInterface.php b/src/Infrastructure/AdminNotificationInterface.php new file mode 100644 index 00000000..fbbc7c2c --- /dev/null +++ b/src/Infrastructure/AdminNotificationInterface.php @@ -0,0 +1,12 @@ + $moderationFlags Name and state of {@see \WMDE\Fundraising\DonationContext\Domain\Model\ModerationIdentifier} + * @param float $amount + */ + public function __construct( + public readonly int $donationId, + public readonly array $moderationFlags, + public readonly float $amount, + ) { + } +} diff --git a/src/Infrastructure/DonationNotifier/TemplateArgumentsDonation.php b/src/Infrastructure/DonationNotifier/TemplateArgumentsDonation.php new file mode 100644 index 00000000..9222a1a9 --- /dev/null +++ b/src/Infrastructure/DonationNotifier/TemplateArgumentsDonation.php @@ -0,0 +1,30 @@ + $moderationFlags Name and state of {@see \WMDE\Fundraising\DonationContext\Domain\Model\ModerationIdentifier} + * @param string $bankTransferCode + * @param bool $receiptOptIn + */ + public function __construct( + public readonly int $id, + public readonly float $amount, + public readonly int $amountInCents, + public readonly int $interval, + public readonly string $paymentType, + public readonly bool $needsModeration, + public readonly array $moderationFlags, + public readonly string $bankTransferCode, + public readonly bool $receiptOptIn, + ) { + } +} diff --git a/src/Infrastructure/DonationNotifier/TemplateArgumentsDonationConfirmation.php b/src/Infrastructure/DonationNotifier/TemplateArgumentsDonationConfirmation.php new file mode 100644 index 00000000..665eccda --- /dev/null +++ b/src/Infrastructure/DonationNotifier/TemplateArgumentsDonationConfirmation.php @@ -0,0 +1,16 @@ + $recipient Output of Name::toArray() + * @param TemplateArgumentsDonation $donation + */ + public function __construct( + public readonly array $recipient, + public readonly TemplateArgumentsDonation $donation + ) { + } +} diff --git a/src/Infrastructure/DonorNotificationInterface.php b/src/Infrastructure/DonorNotificationInterface.php new file mode 100644 index 00000000..da427fe4 --- /dev/null +++ b/src/Infrastructure/DonorNotificationInterface.php @@ -0,0 +1,13 @@ +confirmationMailer->sendMail( new EmailAddress( $donation->getDonor()->getEmailAddress() ), - $this->getTemplateArguments( $donation ) + $this->getTemplateArgumentsForDonorConfirmation( $donation ) ); } - /** - * @param Donation $donation - * - * @return array{recipient:array,donation:array} - */ - private function getTemplateArguments( Donation $donation ): array { + private function getTemplateArgumentsForDonorConfirmation( Donation $donation ): TemplateArgumentsDonationConfirmation { $paymentInfo = $this->getPaymentService->getPaymentDataArray( $donation->getPaymentId() ); - return [ - 'recipient' => $donation->getDonor()->getName()->toArray(), - 'donation' => [ - 'id' => $donation->getId(), - 'amount' => Euro::newFromCents( (int)$paymentInfo['amount'] )->getEuroFloat(), - 'amountInCents' => $paymentInfo['amount'], - 'interval' => $paymentInfo['interval'], - 'paymentType' => $paymentInfo['paymentType'], - 'needsModeration' => $donation->isMarkedForModeration(), - 'moderationFlags' => $this->getModerationFlags( ...$donation->getModerationReasons() ), - 'bankTransferCode' => $paymentInfo['paymentReferenceCode'] ?? '', - 'receiptOptIn' => $donation->getDonor()->wantsReceipt(), - ] - ]; + return new TemplateArgumentsDonationConfirmation( + $donation->getDonor()->getName()->toArray(), + new TemplateArgumentsDonation( + id: $donation->getId(), + amount: Euro::newFromCents( (int)$paymentInfo['amount'] )->getEuroFloat(), + amountInCents: intval( $paymentInfo['amount'] ), + interval: intval( $paymentInfo['interval'] ), + paymentType: strval( $paymentInfo['paymentType'] ), + needsModeration: $donation->isMarkedForModeration(), + moderationFlags: $this->getModerationFlags( ...$donation->getModerationReasons() ), + bankTransferCode: strval( $paymentInfo['paymentReferenceCode'] ?? '' ), + receiptOptIn: $donation->getDonor()->wantsReceipt(), + ) + ); } /** @@ -82,17 +87,13 @@ public function sendModerationNotificationToAdmin( Donation $donation ): void { ); } - /** - * @param Donation $donation - * @return array{id:int,moderationFlags:array,amount:float} - */ - private function getAdminTemplateArguments( Donation $donation ): array { + private function getAdminTemplateArguments( Donation $donation ): TemplateArgumentsAdmin { $paymentInfo = $this->getPaymentService->getPaymentDataArray( $donation->getPaymentId() ); - return [ - 'id' => $donation->getId(), - 'moderationFlags' => $this->getModerationFlags( ...$donation->getModerationReasons() ), - 'amount' => Euro::newFromCents( (int)$paymentInfo['amount'] )->getEuroFloat() - ]; + return new TemplateArgumentsAdmin( + donationId: $donation->getId(), + moderationFlags: $this->getModerationFlags( ...$donation->getModerationReasons() ), + amount: Euro::newFromCents( (int)$paymentInfo['amount'] )->getEuroFloat() + ); } } diff --git a/src/Infrastructure/TemplateMailerInterface.php b/src/Infrastructure/TemplateMailerInterface.php deleted file mode 100644 index 53d012bb..00000000 --- a/src/Infrastructure/TemplateMailerInterface.php +++ /dev/null @@ -1,16 +0,0 @@ - $templateArguments Context parameters to use while rendering the template - */ - public function sendMail( EmailAddress $recipient, array $templateArguments = [] ): void; -} diff --git a/tests/Fixtures/TemplateBasedMailerSpy.php b/tests/Fixtures/DonorNotificationSpy.php similarity index 59% rename from tests/Fixtures/TemplateBasedMailerSpy.php rename to tests/Fixtures/DonorNotificationSpy.php index 5c0e5fbf..cfa402bd 100644 --- a/tests/Fixtures/TemplateBasedMailerSpy.php +++ b/tests/Fixtures/DonorNotificationSpy.php @@ -6,13 +6,15 @@ use PHPUnit\Framework\TestCase; use WMDE\EmailAddress\EmailAddress; -use WMDE\Fundraising\DonationContext\Infrastructure\TemplateMailerInterface; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationNotifier\TemplateArgumentsDonationConfirmation; +use WMDE\Fundraising\DonationContext\Infrastructure\DonorNotificationInterface; -class TemplateBasedMailerSpy implements TemplateMailerInterface { +class DonorNotificationSpy implements DonorNotificationInterface { private TestCase $testCase; + /** - * @var array}> + * @var array{EmailAddress,TemplateArgumentsDonationConfirmation}[] */ private array $sendMailCalls = []; @@ -20,24 +22,18 @@ public function __construct( TestCase $testCase ) { $this->testCase = $testCase; } - public function sendMail( EmailAddress $recipient, array $templateArguments = [] ): void { + public function sendMail( EmailAddress $recipient, TemplateArgumentsDonationConfirmation $templateArguments ): void { $this->sendMailCalls[] = [ $recipient, $templateArguments ]; } /** - * @return array}> + * @return array{EmailAddress,TemplateArgumentsDonationConfirmation}[] */ public function getSendMailCalls(): array { return $this->sendMailCalls; } - /** - * @param EmailAddress $expectedEmail - * @param array $expectedArguments - * - * @return void - */ - public function assertCalledOnceWith( EmailAddress $expectedEmail, array $expectedArguments ): void { + public function assertCalledOnceWith( EmailAddress $expectedEmail, TemplateArgumentsDonationConfirmation $expectedArguments ): void { $this->assertCalledOnce(); $this->testCase->assertEquals( diff --git a/tests/Fixtures/ThrowingAdminNotifier.php b/tests/Fixtures/ThrowingAdminNotifier.php new file mode 100644 index 00000000..669d4f09 --- /dev/null +++ b/tests/Fixtures/ThrowingAdminNotifier.php @@ -0,0 +1,16 @@ +getMockPaymentService( $donation->getPaymentId(), ValidPayments::newBankTransferPayment()->getDisplayValues() ); - - $confirmationMailer = new DonationMailer( $mailerSpy, new ThrowingTemplateMailer(), $paymentService, self::ADMIN_EMAIL ); - + $confirmationMailer = new TemplateDonationNotifier( $mailerSpy, new ThrowingAdminNotifier(), $paymentService, self::ADMIN_EMAIL ); $confirmationMailer->sendConfirmationFor( $donation ); - $mailerSpy->assertCalledOnceWith( new EmailAddress( ValidDonation::DONOR_EMAIL_ADDRESS ), [ - 'recipient' => [ - 'firstName' => ValidDonation::DONOR_FIRST_NAME, - 'lastName' => ValidDonation::DONOR_LAST_NAME, - 'salutation' => ValidDonation::DONOR_SALUTATION, - 'title' => ValidDonation::DONOR_TITLE - ], - 'donation' => [ - 'id' => $donation->getId(), - 'amount' => ValidDonation::DONATION_AMOUNT, - 'amountInCents' => intval( ValidDonation::DONATION_AMOUNT * 100 ), - 'interval' => ValidDonation::PAYMENT_INTERVAL_IN_MONTHS, - 'needsModeration' => false, - 'paymentType' => 'UEB', - 'bankTransferCode' => ValidPayments::PAYMENT_BANK_TRANSFER_CODE, - 'receiptOptIn' => true, - 'moderationFlags' => [], - ] - ] ); + $mailerSpy->assertCalledOnceWith( + new EmailAddress( ValidDonation::DONOR_EMAIL_ADDRESS ), + new TemplateArgumentsDonationConfirmation( + [ + 'firstName' => ValidDonation::DONOR_FIRST_NAME, + 'lastName' => ValidDonation::DONOR_LAST_NAME, + 'salutation' => ValidDonation::DONOR_SALUTATION, + 'title' => ValidDonation::DONOR_TITLE + ], + new TemplateArgumentsDonation( + id: $donation->getId(), + amount: ValidDonation::DONATION_AMOUNT, + amountInCents: intval( ValidDonation::DONATION_AMOUNT * 100 ), + interval: ValidDonation::PAYMENT_INTERVAL_IN_MONTHS, + paymentType: 'UEB', + needsModeration: false, + moderationFlags: [], + bankTransferCode: ValidPayments::PAYMENT_BANK_TRANSFER_CODE, + receiptOptIn: true, + ) + ) + ); } public function testTemplateDataContainsModerationInformation(): void { - $mailerSpy = new TemplateBasedMailerSpy( $this ); + $mailerSpy = new DonorNotificationSpy( $this ); $donation = ValidDonation::newBankTransferDonation(); $donation->markForModeration( new ModerationReason( ModerationIdentifier::AMOUNT_TOO_HIGH, 'amount' ), new ModerationReason( ModerationIdentifier::ADDRESS_CONTENT_VIOLATION, 'email' ), new ModerationReason( ModerationIdentifier::ADDRESS_CONTENT_VIOLATION, 'street' ), ); - $confirmationMailer = new DonationMailer( + $confirmationMailer = new TemplateDonationNotifier( $mailerSpy, - new ThrowingTemplateMailer(), + new ThrowingAdminNotifier(), $this->getMockPaymentService( $donation->getPaymentId(), ValidPayments::newBankTransferPayment()->getDisplayValues() ), self::ADMIN_EMAIL ); @@ -70,23 +76,22 @@ public function testTemplateDataContainsModerationInformation(): void { $confirmationMailer->sendConfirmationFor( $donation ); [ , $templateArguments ] = $mailerSpy->getSendMailCalls()[0]; - if ( !is_array( $templateArguments['donation'] ) ) { - $templateArguments['donation'] = []; - } - $this->assertTrue( $templateArguments['donation']['needsModeration'] ); + /** @var TemplateArgumentsDonation $donation */ + $donation = $templateArguments->donation; + $this->assertTrue( $donation->needsModeration ); $this->assertSame( [ 'AMOUNT_TOO_HIGH' => true, 'ADDRESS_CONTENT_VIOLATION' => true ], - $templateArguments['donation']['moderationFlags'] + $donation->moderationFlags ); } public function testGivenAnonymousDonationMailerDoesNothing(): void { - $mailerSpy = new TemplateBasedMailerSpy( $this ); + $mailerSpy = new DonorNotificationSpy( $this ); $donation = ValidDonation::newBookedAnonymousPayPalDonation(); - $confirmationMailer = new DonationMailer( $mailerSpy, new ThrowingTemplateMailer(), $this->createStub( GetPaymentUseCase::class ), self::ADMIN_EMAIL ); + $confirmationMailer = new TemplateDonationNotifier( $mailerSpy, new ThrowingAdminNotifier(), $this->createStub( GetPaymentUseCase::class ), self::ADMIN_EMAIL ); $confirmationMailer->sendConfirmationFor( $donation ); @@ -94,13 +99,13 @@ public function testGivenAnonymousDonationMailerDoesNothing(): void { } public function testGivenUnmoderatedDonation_adminIsNotNotified(): void { - $mailerSpy = new TemplateBasedMailerSpy( $this ); - $confirmationMailer = new DonationMailer( new ThrowingTemplateMailer(), $mailerSpy, $this->createStub( GetPaymentUseCase::class ), self::ADMIN_EMAIL ); + $mailerSpy = $this->createMock( AdminNotificationInterface::class ); + $mailerSpy->expects( $this->never() )->method( 'sendMail' ); + + $confirmationMailer = new TemplateDonationNotifier( new ThrowingDonorNotification(), $mailerSpy, $this->createStub( GetPaymentUseCase::class ), self::ADMIN_EMAIL ); $donation = ValidDonation::newDirectDebitDonation(); $confirmationMailer->sendModerationNotificationToAdmin( $donation ); - - $this->assertCount( 0, $mailerSpy->getSendMailCalls() ); } /** @@ -108,25 +113,24 @@ public function testGivenUnmoderatedDonation_adminIsNotNotified(): void { * @param int $expectedMailCount * * @return void - * @throws \PHPUnit\Framework\MockObject\Exception * @dataProvider moderationReasonProvider */ public function testGivenModeratedDonation_adminIsNotNotifiedOfAnyModerationExceptAmountTooHigh( array $moderationReasons, int $expectedMailCount ): void { - $mailerSpy = new TemplateBasedMailerSpy( $this ); + $mailerSpy = $this->createMock( AdminNotificationInterface::class ); + $mailerSpy->expects( $this->exactly( $expectedMailCount ) )->method( 'sendMail' ); + $donation = ValidDonation::newDirectDebitDonation(); $donation->markForModeration( ...$moderationReasons ); $paymentService = $this->createStub( GetPaymentUseCase::class ); $paymentService->method( 'getPaymentDataArray' )->willReturn( ValidPayments::newDirectDebitPayment()->getDisplayValues() ); - $confirmationMailer = new DonationMailer( - new ThrowingTemplateMailer(), + $confirmationMailer = new TemplateDonationNotifier( + new ThrowingDonorNotification(), $mailerSpy, $paymentService, self::ADMIN_EMAIL ); $confirmationMailer->sendModerationNotificationToAdmin( $donation ); - - $this->assertCount( $expectedMailCount, $mailerSpy->getSendMailCalls() ); } /** @@ -146,25 +150,27 @@ public static function moderationReasonProvider(): iterable { } public function testTemplateDataForAdminContainsAllNecessaryDonationInformation(): void { - $mailerSpy = new TemplateBasedMailerSpy( $this ); $donation = ValidDonation::newBankTransferDonation(); $donation->markForModeration( new ModerationReason( ModerationIdentifier::AMOUNT_TOO_HIGH, 'amount' ) ); - $confirmationMailer = new DonationMailer( - new ThrowingTemplateMailer(), + $expectedTemplateArguments = new TemplateArgumentsAdmin( + donationId: $donation->getId(), + moderationFlags: [ + 'AMOUNT_TOO_HIGH' => true, + ], + amount: ValidDonation::DONATION_AMOUNT, + ); + $expectedEmail = new EmailAddress( self::ADMIN_EMAIL ); + $mailerSpy = $this->createMock( AdminNotificationInterface::class ); + $mailerSpy->expects( $this->once() )->method( 'sendMail' )->with( $expectedEmail, $expectedTemplateArguments ); + + $confirmationMailer = new TemplateDonationNotifier( + new ThrowingDonorNotification(), $mailerSpy, $this->getMockPaymentService( $donation->getPaymentId(), ValidPayments::newBankTransferPayment()->getDisplayValues() ), self::ADMIN_EMAIL ); $confirmationMailer->sendModerationNotificationToAdmin( $donation ); - - $mailerSpy->assertCalledOnceWith( new EmailAddress( self::ADMIN_EMAIL ), [ - 'id' => $donation->getId(), - 'amount' => ValidDonation::DONATION_AMOUNT, - 'moderationFlags' => [ - 'AMOUNT_TOO_HIGH' => true, - ], - ] ); } /**