From eb7d10b4df62fb89769699ea66dd5a364bb28daf Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Fri, 14 Jul 2023 11:43:47 +0200 Subject: [PATCH 01/10] Use new API-based PayPal payments Rename DonationAuthorizer to DonationAuthorizationChecker. Replace TokenFetcher with DonationAuthorizer, a new class that can generate and store tokens Adapt call to CreatePaymentUseCase, adding all necessary instances and value objects to DomainSpecificPaymentCreationRequest. Remove tokens from AddDonationResponse - they should be queried in the calling code, not returned, because they don't belong in the donation bounded context. Tickets: https://phabricator.wikimedia.org/T341795 https://phabricator.wikimedia.org/T329159 --- composer.json | 2 +- .../DonationAuthorizationChecker.php | 40 ++++++++++ src/Authorization/DonationAuthorizer.php | 38 +--------- ... DoctrineDonationAuthorizationChecker.php} | 7 +- .../RepositoryPaypalParentFinder.php | 2 +- src/DonationAcceptedEventHandler.php | 6 +- src/UseCases/AddComment/AddCommentUseCase.php | 4 +- .../AddDonation/AddDonationResponse.php | 16 +--- .../AddDonation/AddDonationUseCase.php | 68 +++++++---------- .../AddDonation/CreatePaymentService.php | 4 +- .../AddDonation/CreatePaymentWithUseCase.php | 4 +- .../BookDonationUseCase.php | 4 +- .../CancelDonation/CancelDonationUseCase.php | 4 +- .../GetDonation/GetDonationUseCase.php | 4 +- .../UpdateDonor/UpdateDonorUseCase.php | 6 +- tests/Fixtures/CreatePaymentServiceSpy.php | 8 +- tests/Fixtures/FailingDonationAuthorizer.php | 4 +- .../Fixtures/SucceedingDonationAuthorizer.php | 4 +- .../SucceedingDonationAuthorizerSpy.php | 4 +- .../Fixtures/SucceedingPaymentServiceStub.php | 7 +- tests/Fixtures/UrlGeneratorSpy.php | 27 ------- ...trineDonationAuthorizationCheckerTest.php} | 20 +++-- .../DonationAcceptedEventHandlerTest.php | 4 +- .../AddComment/AddCommentUseCaseTest.php | 4 +- .../AddDonation/AddDonationUseCaseTest.php | 76 +++++++++---------- .../CancelDonationUseCaseTest.php | 4 +- .../UpdateDonor/UpdateDonorUseCaseTest.php | 6 +- .../CreditCardNotificationUseCaseTest.php | 4 +- ...ymentCompletionNotificationUseCaseTest.php | 4 +- 29 files changed, 168 insertions(+), 217 deletions(-) create mode 100644 src/Authorization/DonationAuthorizationChecker.php rename src/DataAccess/{DoctrineDonationAuthorizer.php => DoctrineDonationAuthorizationChecker.php} (89%) delete mode 100644 tests/Fixtures/UrlGeneratorSpy.php rename tests/Integration/DataAccess/{DoctrineDonationAuthorizerTest.php => DoctrineDonationAuthorizationCheckerTest.php} (94%) diff --git a/composer.json b/composer.json index 4e5df27e..027c1409 100644 --- a/composer.json +++ b/composer.json @@ -16,7 +16,7 @@ "wmde/euro": "~1.0", "wmde/freezable-value-object": "~2.0", "wmde/fun-validators": "~4.0", - "wmde/fundraising-payments": "~6.0" + "wmde/fundraising-payments": "dev-url-generation-refactor" }, "repositories": [ { diff --git a/src/Authorization/DonationAuthorizationChecker.php b/src/Authorization/DonationAuthorizationChecker.php new file mode 100644 index 00000000..6a814731 --- /dev/null +++ b/src/Authorization/DonationAuthorizationChecker.php @@ -0,0 +1,40 @@ + + */ +interface DonationAuthorizationChecker { + + /** + * Should return false on infrastructure failure. + * + * @param int $donationId + * + * @return bool + */ + public function userCanModifyDonation( int $donationId ): bool; + + /** + * Should return false on infrastructure failure. + * + * @param int $donationId + * + * @return bool + */ + public function systemCanModifyDonation( int $donationId ): bool; + + /** + * Should return false on infrastructure failure. + * + * @param int $donationId + * + * @return bool + */ + public function canAccessDonation( int $donationId ): bool; + +} diff --git a/src/Authorization/DonationAuthorizer.php b/src/Authorization/DonationAuthorizer.php index 580c74bc..b61743fd 100644 --- a/src/Authorization/DonationAuthorizer.php +++ b/src/Authorization/DonationAuthorizer.php @@ -1,40 +1,10 @@ - */ -interface DonationAuthorizer { - - /** - * Should return false on infrastructure failure. - * - * @param int $donationId - * - * @return bool - */ - public function userCanModifyDonation( int $donationId ): bool; - - /** - * Should return false on infrastructure failure. - * - * @param int $donationId - * - * @return bool - */ - public function systemCanModifyDonation( int $donationId ): bool; - - /** - * Should return false on infrastructure failure. - * - * @param int $donationId - * - * @return bool - */ - public function canAccessDonation( int $donationId ): bool; +use WMDE\Fundraising\PaymentContext\Services\URLAuthenticator; +interface DonationAuthorizer { + public function authorizeDonationAccess( int $donationId ): URLAuthenticator; } diff --git a/src/DataAccess/DoctrineDonationAuthorizer.php b/src/DataAccess/DoctrineDonationAuthorizationChecker.php similarity index 89% rename from src/DataAccess/DoctrineDonationAuthorizer.php rename to src/DataAccess/DoctrineDonationAuthorizationChecker.php index c4b0ff08..42492fde 100644 --- a/src/DataAccess/DoctrineDonationAuthorizer.php +++ b/src/DataAccess/DoctrineDonationAuthorizationChecker.php @@ -6,14 +6,15 @@ use Doctrine\ORM\EntityManager; use Doctrine\ORM\ORMException; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\DataAccess\DoctrineEntities\Donation; use WMDE\Fundraising\DonationContext\Domain\Repositories\GetDonationException; /** - * @license GPL-2.0-or-later + * This is only for checking legacy donation authorizations. + * New donations should use an implementation of DonationAuthorizationChecker that uses tokens stored outside the bounded context. */ -class DoctrineDonationAuthorizer implements DonationAuthorizer { +class DoctrineDonationAuthorizationChecker implements DonationAuthorizationChecker { private EntityManager $entityManager; private string $updateToken; diff --git a/src/DataAccess/PaymentMigration/RepositoryPaypalParentFinder.php b/src/DataAccess/PaymentMigration/RepositoryPaypalParentFinder.php index 8cad3281..2a8ca1f0 100644 --- a/src/DataAccess/PaymentMigration/RepositoryPaypalParentFinder.php +++ b/src/DataAccess/PaymentMigration/RepositoryPaypalParentFinder.php @@ -5,7 +5,7 @@ use Doctrine\ORM\EntityManager; use WMDE\Fundraising\PaymentContext\DataAccess\DoctrinePaymentRepository; -use WMDE\Fundraising\PaymentContext\DataAccess\PaymentNotFoundException; +use WMDE\Fundraising\PaymentContext\Domain\Exception\PaymentNotFoundException; use WMDE\Fundraising\PaymentContext\Domain\Model\PayPalPayment; class RepositoryPaypalParentFinder implements PaypalParentFinder { diff --git a/src/DonationAcceptedEventHandler.php b/src/DonationAcceptedEventHandler.php index 3f074c7c..44b499c0 100644 --- a/src/DonationAcceptedEventHandler.php +++ b/src/DonationAcceptedEventHandler.php @@ -4,7 +4,7 @@ namespace WMDE\Fundraising\DonationContext; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\Domain\Repositories\GetDonationException; use WMDE\Fundraising\DonationContext\UseCases\DonationNotifier; @@ -16,11 +16,11 @@ class DonationAcceptedEventHandler { public const DATABASE_ERROR_OCCURRED = 'Database error occurred'; public const SUCCESS = null; - private DonationAuthorizer $authorizer; + private DonationAuthorizationChecker $authorizer; private DonationRepository $repository; private DonationNotifier $notifier; - public function __construct( DonationAuthorizer $authorizer, DonationRepository $repository, DonationNotifier $notifier ) { + public function __construct( DonationAuthorizationChecker $authorizer, DonationRepository $repository, DonationNotifier $notifier ) { $this->authorizer = $authorizer; $this->repository = $repository; $this->notifier = $notifier; diff --git a/src/UseCases/AddComment/AddCommentUseCase.php b/src/UseCases/AddComment/AddCommentUseCase.php index db854257..743092c5 100644 --- a/src/UseCases/AddComment/AddCommentUseCase.php +++ b/src/UseCases/AddComment/AddCommentUseCase.php @@ -4,7 +4,7 @@ namespace WMDE\Fundraising\DonationContext\UseCases\AddComment; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Model\DonationComment; use WMDE\Fundraising\DonationContext\Domain\Model\Donor\Name\NoName; @@ -24,7 +24,7 @@ class AddCommentUseCase { public function __construct( private readonly DonationRepository $donationRepository, - private readonly DonationAuthorizer $authorizationService, + private readonly DonationAuthorizationChecker $authorizationService, private readonly TextPolicyValidator $textPolicyValidator, private readonly AddCommentValidator $commentValidator ) { } diff --git a/src/UseCases/AddDonation/AddDonationResponse.php b/src/UseCases/AddDonation/AddDonationResponse.php index bef08da3..6d5b451e 100644 --- a/src/UseCases/AddDonation/AddDonationResponse.php +++ b/src/UseCases/AddDonation/AddDonationResponse.php @@ -16,17 +16,11 @@ class AddDonationResponse { private ?Donation $donation = null; - private ?string $updateToken = null; - - private ?string $accessToken = null; - private ?string $paymentProviderRedirectUrl = null; - public static function newSuccessResponse( Donation $donation, string $updateToken, string $accessToken, ?string $paymentProviderRedirectUrl ): self { + public static function newSuccessResponse( Donation $donation, ?string $paymentProviderRedirectUrl ): self { $response = new self(); $response->donation = $donation; - $response->updateToken = $updateToken; - $response->accessToken = $accessToken; $response->paymentProviderRedirectUrl = $paymentProviderRedirectUrl; return $response; } @@ -66,14 +60,6 @@ public function getDonation(): ?Donation { return $this->donation; } - public function getUpdateToken(): ?string { - return $this->updateToken; - } - - public function getAccessToken(): ?string { - return $this->accessToken; - } - public function getPaymentProviderRedirectUrl(): ?string { return $this->paymentProviderRedirectUrl; } diff --git a/src/UseCases/AddDonation/AddDonationUseCase.php b/src/UseCases/AddDonation/AddDonationUseCase.php index d356b469..2c96a785 100644 --- a/src/UseCases/AddDonation/AddDonationUseCase.php +++ b/src/UseCases/AddDonation/AddDonationUseCase.php @@ -4,8 +4,7 @@ namespace WMDE\Fundraising\DonationContext\UseCases\AddDonation; -use WMDE\Fundraising\DonationContext\Authorization\DonationTokenFetcher; -use WMDE\Fundraising\DonationContext\Authorization\DonationTokens; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; use WMDE\Fundraising\DonationContext\Domain\Event\DonationCreatedEvent; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Model\DonationTrackingInfo; @@ -22,10 +21,9 @@ use WMDE\Fundraising\DonationContext\EventEmitter; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\Moderation\ModerationService; use WMDE\Fundraising\DonationContext\UseCases\DonationNotifier; -use WMDE\Fundraising\PaymentContext\Domain\PaymentUrlGenerator\PaymentProviderURLGenerator; -use WMDE\Fundraising\PaymentContext\Domain\PaymentUrlGenerator\RequestContext; +use WMDE\Fundraising\PaymentContext\Domain\UrlGenerator\DomainSpecificContext; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\DomainSpecificPaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\FailureResponse; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; use WMDE\FunValidators\ConstraintViolation; /** @@ -42,7 +40,7 @@ public function __construct( private readonly AddDonationValidator $donationValidator, private readonly ModerationService $policyValidator, private readonly DonationNotifier $notifier, - private readonly DonationTokenFetcher $tokenFetcher, + private readonly DonationAuthorizer $donationAuthorizer, private readonly EventEmitter $eventEmitter, private readonly CreatePaymentService $paymentService ) { @@ -55,13 +53,14 @@ public function addDonation( AddDonationRequest $donationRequest ): AddDonationR return AddDonationResponse::newFailureResponse( $validationResult->getViolations() ); } - $paymentResult = $this->paymentService->createPayment( $this->getPaymentRequestForDonor( $donationRequest ) ); + $donationId = $this->idGenerator->getNewId(); + $paymentResult = $this->paymentService->createPayment( $this->getPaymentRequestForDonor( $donationRequest, $donationId ) ); if ( $paymentResult instanceof FailureResponse ) { return AddDonationResponse::newFailureResponse( [ new ConstraintViolation( $donationRequest->getPaymentCreationRequest(), $paymentResult->errorMessage, 'payment' ) ] ); } - $donation = $this->newDonationFromRequest( $donationRequest, $paymentResult->paymentId ); + $donation = $this->newDonationFromRequest( $donationRequest, $donationId, $paymentResult->paymentId ); $moderationResult = $this->policyValidator->moderateDonationRequest( $donationRequest ); if ( $moderationResult->needsModeration() ) { @@ -74,27 +73,20 @@ public function addDonation( AddDonationRequest $donationRequest ): AddDonationR $this->donationRepository->storeDonation( $donation ); - $tokens = $this->tokenFetcher->getTokens( $donation->getId() ); - $this->eventEmitter->emit( new DonationCreatedEvent( $donation->getId(), $donation->getDonor() ) ); $this->sendDonationConfirmationEmail( $donation, $paymentResult->paymentComplete ); // The notifier checks if a notification is really needed (e.g. amount too high) $this->notifier->sendModerationNotificationToAdmin( $donation ); - return AddDonationResponse::newSuccessResponse( - $donation, - $tokens->getUpdateToken(), - $tokens->getAccessToken(), - $this->generatePaymentProviderUrl( $paymentResult->paymentProviderURLGenerator, $donation, $tokens ) - ); + return AddDonationResponse::newSuccessResponse( $donation, $paymentResult->externalPaymentCompletionUrl ); } - private function newDonationFromRequest( AddDonationRequest $donationRequest, int $paymentId ): Donation { + private function newDonationFromRequest( AddDonationRequest $donationRequest, int $donationId, int $paymentId ): Donation { $donor = $this->getPersonalInfoFromRequest( $donationRequest ); $this->processNewsletterAndReceiptOptions( $donationRequest, $donor ); return new Donation( - $this->idGenerator->getNewId(), + $donationId, $donor, $paymentId, $this->newTrackingInfoFromRequest( $donationRequest ) @@ -167,45 +159,39 @@ private function sendDonationConfirmationEmail( Donation $donation, bool $paymen * * We need to add donor-type specific properties (bank transfer code and validation) * to the original request. - * - * @param AddDonationRequest $request - * @return PaymentCreationRequest */ - private function getPaymentRequestForDonor( AddDonationRequest $request ): PaymentCreationRequest { + private function getPaymentRequestForDonor( AddDonationRequest $request, int $donationId ): DomainSpecificPaymentCreationRequest { $paymentRequest = $request->getPaymentCreationRequest(); $paymentReferenceCodePrefix = self::PREFIX_BANK_TRANSACTION_KNOWN_DONOR; if ( $request->donorIsAnonymous() ) { $paymentReferenceCodePrefix = self::PREFIX_BANK_TRANSACTION_ANONYMOUS_DONOR; } - $paymentRequest = PaymentRequestBuilder::fromExistingRequest( $paymentRequest ) ->withPaymentReferenceCodePrefix( $paymentReferenceCodePrefix ) ->getPaymentCreationRequest(); - $paymentRequest->setDomainSpecificPaymentValidator( $this->paymentService->createPaymentValidator() ); - return $paymentRequest; - } - private function generatePaymentProviderUrl( PaymentProviderURLGenerator $paymentProviderURLGenerator, Donation $donation, DonationTokens $tokens ): string { - $name = $donation->getDonor()->getName()->toArray(); - return $paymentProviderURLGenerator->generateURL( new RequestContext( - $donation->getId(), - $this->generatePayPalInvoiceId( $donation ), - $tokens->getUpdateToken(), - $tokens->getAccessToken(), - $name['firstName'] ?? '', - $name['lastName'] ?? '', - ) ); + $urlAuthenticator = $this->donationAuthorizer->authorizeDonationAccess( $donationId ); + $context = new DomainSpecificContext( + $donationId, + null, + $this->generatePayPalInvoiceId( $donationId ), + $request->getDonorFirstName(), + $request->getDonorLastName() + ); + return DomainSpecificPaymentCreationRequest::newFromBaseRequest( + $paymentRequest, + $this->paymentService->createPaymentValidator(), + $context, + $urlAuthenticator + ); } /** * We use the donation primary key as the InvoiceId because they're unique * But we prepend a letter to make sure they don't clash with memberships - * - * @param Donation $donation - * @return string */ - private function generatePayPalInvoiceId( Donation $donation ): string { - return 'D' . $donation->getId(); + private function generatePayPalInvoiceId( int $donationId ): string { + return 'D' . $donationId; } private function processNewsletterAndReceiptOptions( AddDonationRequest $donationRequest, Donor $donor ): void { diff --git a/src/UseCases/AddDonation/CreatePaymentService.php b/src/UseCases/AddDonation/CreatePaymentService.php index 63df6f8d..41880513 100644 --- a/src/UseCases/AddDonation/CreatePaymentService.php +++ b/src/UseCases/AddDonation/CreatePaymentService.php @@ -3,12 +3,12 @@ namespace WMDE\Fundraising\DonationContext\UseCases\AddDonation; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\DomainSpecificPaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\FailureResponse as PaymentCreationFailed; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\SuccessResponse as PaymentCreationSucceeded; interface CreatePaymentService { - public function createPayment( PaymentCreationRequest $request ): PaymentCreationFailed|PaymentCreationSucceeded; + public function createPayment( DomainSpecificPaymentCreationRequest $request ): PaymentCreationFailed|PaymentCreationSucceeded; public function createPaymentValidator(): DonationPaymentValidator; } diff --git a/src/UseCases/AddDonation/CreatePaymentWithUseCase.php b/src/UseCases/AddDonation/CreatePaymentWithUseCase.php index 628f2882..c973d56e 100644 --- a/src/UseCases/AddDonation/CreatePaymentWithUseCase.php +++ b/src/UseCases/AddDonation/CreatePaymentWithUseCase.php @@ -5,8 +5,8 @@ use WMDE\Fundraising\PaymentContext\Domain\PaymentType; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\CreatePaymentUseCase; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\DomainSpecificPaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\FailureResponse as PaymentCreationFailed; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\SuccessResponse as PaymentCreationSucceeded; class CreatePaymentWithUseCase implements CreatePaymentService { @@ -21,7 +21,7 @@ public function __construct( ) { } - public function createPayment( PaymentCreationRequest $request ): PaymentCreationFailed|PaymentCreationSucceeded { + public function createPayment( DomainSpecificPaymentCreationRequest $request ): PaymentCreationFailed|PaymentCreationSucceeded { return $this->createPaymentUseCase->createPayment( $request ); } diff --git a/src/UseCases/BookDonationUseCase/BookDonationUseCase.php b/src/UseCases/BookDonationUseCase/BookDonationUseCase.php index e628b8ca..abfb7051 100644 --- a/src/UseCases/BookDonationUseCase/BookDonationUseCase.php +++ b/src/UseCases/BookDonationUseCase/BookDonationUseCase.php @@ -4,7 +4,7 @@ namespace WMDE\Fundraising\DonationContext\UseCases\BookDonationUseCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationIdRepository; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; @@ -21,7 +21,7 @@ class BookDonationUseCase { public function __construct( private readonly DonationIdRepository $idGenerator, private readonly DonationRepository $repository, - private readonly DonationAuthorizer $authorizationService, + private readonly DonationAuthorizationChecker $authorizationService, private readonly DonationNotifier $notifier, private readonly PaymentBookingService $paymentBookingService, private readonly DonationEventLogger $eventLogger diff --git a/src/UseCases/CancelDonation/CancelDonationUseCase.php b/src/UseCases/CancelDonation/CancelDonationUseCase.php index 72bb266a..b0ee46bf 100644 --- a/src/UseCases/CancelDonation/CancelDonationUseCase.php +++ b/src/UseCases/CancelDonation/CancelDonationUseCase.php @@ -5,7 +5,7 @@ namespace WMDE\Fundraising\DonationContext\UseCases\CancelDonation; use WMDE\EmailAddress\EmailAddress; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\Domain\Repositories\GetDonationException; @@ -23,7 +23,7 @@ class CancelDonationUseCase { public function __construct( private DonationRepository $donationRepository, private TemplateMailerInterface $mailer, - private DonationAuthorizer $authorizationService, + private DonationAuthorizationChecker $authorizationService, private DonationEventLogger $donationLogger, private CancelPaymentUseCase $cancelPaymentUseCase ) { } diff --git a/src/UseCases/GetDonation/GetDonationUseCase.php b/src/UseCases/GetDonation/GetDonationUseCase.php index 127b8e3a..4e4882b2 100644 --- a/src/UseCases/GetDonation/GetDonationUseCase.php +++ b/src/UseCases/GetDonation/GetDonationUseCase.php @@ -4,7 +4,7 @@ namespace WMDE\Fundraising\DonationContext\UseCases\GetDonation; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Authorization\DonationTokenFetcher; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; @@ -13,7 +13,7 @@ class GetDonationUseCase { public function __construct( - private readonly DonationAuthorizer $authorizer, + private readonly DonationAuthorizationChecker $authorizer, private readonly DonationTokenFetcher $tokenFetcher, private readonly DonationRepository $donationRepository ) { } diff --git a/src/UseCases/UpdateDonor/UpdateDonorUseCase.php b/src/UseCases/UpdateDonor/UpdateDonorUseCase.php index 2a970bed..a4f3013f 100644 --- a/src/UseCases/UpdateDonor/UpdateDonorUseCase.php +++ b/src/UseCases/UpdateDonor/UpdateDonorUseCase.php @@ -4,7 +4,7 @@ namespace WMDE\Fundraising\DonationContext\UseCases\UpdateDonor; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Event\DonorUpdatedEvent; use WMDE\Fundraising\DonationContext\Domain\Model\Donor; use WMDE\Fundraising\DonationContext\Domain\Model\Donor\Address\PostalAddress; @@ -26,14 +26,14 @@ */ class UpdateDonorUseCase { - private DonationAuthorizer $authorizationService; + private DonationAuthorizationChecker $authorizationService; private DonationRepository $donationRepository; private UpdateDonorValidator $updateDonorValidator; private DonationNotifier $donationConfirmationMailer; private EventEmitter $eventEmitter; public function __construct( - DonationAuthorizer $authorizationService, + DonationAuthorizationChecker $authorizationService, UpdateDonorValidator $updateDonorValidator, DonationRepository $donationRepository, DonationNotifier $donationConfirmationMailer, diff --git a/tests/Fixtures/CreatePaymentServiceSpy.php b/tests/Fixtures/CreatePaymentServiceSpy.php index f55ba844..bcac561c 100644 --- a/tests/Fixtures/CreatePaymentServiceSpy.php +++ b/tests/Fixtures/CreatePaymentServiceSpy.php @@ -5,16 +5,16 @@ use WMDE\Fundraising\DonationContext\UseCases\AddDonation\DonationPaymentValidator; use WMDE\Fundraising\PaymentContext\Domain\PaymentType; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\DomainSpecificPaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\SuccessResponse as PaymentCreationSucceeded; class CreatePaymentServiceSpy extends SucceedingPaymentServiceStub { /** - * @var PaymentCreationRequest[] + * @var DomainSpecificPaymentCreationRequest[] */ private array $requests = []; - public function createPayment( PaymentCreationRequest $request ): PaymentCreationSucceeded { + public function createPayment( DomainSpecificPaymentCreationRequest $request ): PaymentCreationSucceeded { $this->requests[] = $request; return parent::createPayment( $request ); } @@ -23,7 +23,7 @@ public function createPaymentValidator(): DonationPaymentValidator { return new DonationPaymentValidator( PaymentType::cases() ); } - public function getLastRequest(): PaymentCreationRequest { + public function getLastRequest(): DomainSpecificPaymentCreationRequest { return $this->requests[0]; } diff --git a/tests/Fixtures/FailingDonationAuthorizer.php b/tests/Fixtures/FailingDonationAuthorizer.php index 117d43f4..f6a6f49a 100644 --- a/tests/Fixtures/FailingDonationAuthorizer.php +++ b/tests/Fixtures/FailingDonationAuthorizer.php @@ -4,12 +4,12 @@ namespace WMDE\Fundraising\DonationContext\Tests\Fixtures; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; /** * @license GPL-2.0-or-later */ -class FailingDonationAuthorizer implements DonationAuthorizer { +class FailingDonationAuthorizer implements DonationAuthorizationChecker { public function userCanModifyDonation( int $donationId ): bool { return false; diff --git a/tests/Fixtures/SucceedingDonationAuthorizer.php b/tests/Fixtures/SucceedingDonationAuthorizer.php index e2490e3a..a1556572 100644 --- a/tests/Fixtures/SucceedingDonationAuthorizer.php +++ b/tests/Fixtures/SucceedingDonationAuthorizer.php @@ -4,12 +4,12 @@ namespace WMDE\Fundraising\DonationContext\Tests\Fixtures; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; /** * @license GPL-2.0-or-later */ -class SucceedingDonationAuthorizer implements DonationAuthorizer { +class SucceedingDonationAuthorizer implements DonationAuthorizationChecker { public function userCanModifyDonation( int $donationId ): bool { return true; diff --git a/tests/Fixtures/SucceedingDonationAuthorizerSpy.php b/tests/Fixtures/SucceedingDonationAuthorizerSpy.php index 646363bf..8b74ae7f 100644 --- a/tests/Fixtures/SucceedingDonationAuthorizerSpy.php +++ b/tests/Fixtures/SucceedingDonationAuthorizerSpy.php @@ -4,12 +4,12 @@ namespace WMDE\Fundraising\DonationContext\Tests\Fixtures; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; /** * @license GPL-2.0-or-later */ -class SucceedingDonationAuthorizerSpy implements DonationAuthorizer { +class SucceedingDonationAuthorizerSpy implements DonationAuthorizationChecker { private bool $authorizedAsUser = false; private bool $authorizedAsAdmin = false; diff --git a/tests/Fixtures/SucceedingPaymentServiceStub.php b/tests/Fixtures/SucceedingPaymentServiceStub.php index 960cc8f6..86188770 100644 --- a/tests/Fixtures/SucceedingPaymentServiceStub.php +++ b/tests/Fixtures/SucceedingPaymentServiceStub.php @@ -7,8 +7,7 @@ use WMDE\Fundraising\DonationContext\UseCases\AddDonation\CreatePaymentService; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\DonationPaymentValidator; use WMDE\Fundraising\PaymentContext\Domain\PaymentType; -use WMDE\Fundraising\PaymentContext\Domain\PaymentUrlGenerator\NullGenerator; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\DomainSpecificPaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\SuccessResponse as PaymentCreationSucceeded; class SucceedingPaymentServiceStub implements CreatePaymentService { @@ -18,12 +17,12 @@ class SucceedingPaymentServiceStub implements CreatePaymentService { public function __construct( ?PaymentCreationSucceeded $successResponse = null ) { $this->successResponse = $successResponse ?? new PaymentCreationSucceeded( ValidPayments::ID_BANK_TRANSFER, - new NullGenerator(), + '', true ); } - public function createPayment( PaymentCreationRequest $request ): PaymentCreationSucceeded { + public function createPayment( DomainSpecificPaymentCreationRequest $request ): PaymentCreationSucceeded { return $this->successResponse; } diff --git a/tests/Fixtures/UrlGeneratorSpy.php b/tests/Fixtures/UrlGeneratorSpy.php deleted file mode 100644 index 7541580b..00000000 --- a/tests/Fixtures/UrlGeneratorSpy.php +++ /dev/null @@ -1,27 +0,0 @@ -lastContext !== null ) { - throw new \LogicException( 'Url generator must only be called once' ); - } - $this->lastContext = $requestContext; - // Adding context just to make the url parameter look impressive and different from a stub - return 'https://example.com/?context=' . urlencode( var_export( $requestContext, true ) ); - } - - public function getLastContext(): RequestContext { - if ( $this->lastContext === null ) { - throw new \LogicException( 'Url generator should have been called' ); - } - return $this->lastContext; - } -} diff --git a/tests/Integration/DataAccess/DoctrineDonationAuthorizerTest.php b/tests/Integration/DataAccess/DoctrineDonationAuthorizationCheckerTest.php similarity index 94% rename from tests/Integration/DataAccess/DoctrineDonationAuthorizerTest.php rename to tests/Integration/DataAccess/DoctrineDonationAuthorizationCheckerTest.php index 2d9407e3..a502adbd 100644 --- a/tests/Integration/DataAccess/DoctrineDonationAuthorizerTest.php +++ b/tests/Integration/DataAccess/DoctrineDonationAuthorizationCheckerTest.php @@ -7,18 +7,16 @@ use Doctrine\ORM\EntityManager; use Doctrine\ORM\Exception\ORMException; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; -use WMDE\Fundraising\DonationContext\DataAccess\DoctrineDonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; +use WMDE\Fundraising\DonationContext\DataAccess\DoctrineDonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\DataAccess\DoctrineEntities\Donation; use WMDE\Fundraising\DonationContext\Domain\Repositories\GetDonationException; use WMDE\Fundraising\DonationContext\Tests\TestEnvironment; /** - * @covers \WMDE\Fundraising\DonationContext\DataAccess\DoctrineDonationAuthorizer - * - * @license GPL-2.0-or-later + * @covers \WMDE\Fundraising\DonationContext\DataAccess\DoctrineDonationAuthorizationChecker */ -class DoctrineDonationAuthorizerTest extends TestCase { +class DoctrineDonationAuthorizationCheckerTest extends TestCase { private const CORRECT_UPDATE_TOKEN = 'CorrectUpdateToken'; private const CORRECT_ACCESS_TOKEN = 'CorrectAccessToken'; @@ -36,8 +34,8 @@ protected function setUp(): void { $this->entityManager = TestEnvironment::newInstance()->getFactory()->getEntityManager(); } - private function newAuthorizationService( string $updateToken = '', string $accessToken = '' ): DonationAuthorizer { - return new DoctrineDonationAuthorizer( $this->entityManager, $updateToken, $accessToken ); + private function newAuthorizationService( string $updateToken = '', string $accessToken = '' ): DonationAuthorizationChecker { + return new DoctrineDonationAuthorizationChecker( $this->entityManager, $updateToken, $accessToken ); } public function testGivenNoDonation_authorizationFails(): void { @@ -129,7 +127,7 @@ public function testWhenUpdateTokenIsExpiredUpdateCheckSucceedsForSystem(): void } public function testGivenExceptionFromEntityManager_authorizerWrapsExceptionForUserModification(): void { - $authorizer = new DoctrineDonationAuthorizer( + $authorizer = new DoctrineDonationAuthorizationChecker( $this->getThrowingEntityManager(), self::CORRECT_UPDATE_TOKEN, self::CORRECT_ACCESS_TOKEN @@ -141,7 +139,7 @@ public function testGivenExceptionFromEntityManager_authorizerWrapsExceptionForU } public function testGivenExceptionFromEntityManager_authorizerWrapsExceptionForSystemModification(): void { - $authorizer = new DoctrineDonationAuthorizer( + $authorizer = new DoctrineDonationAuthorizationChecker( $this->getThrowingEntityManager(), self::CORRECT_UPDATE_TOKEN, self::CORRECT_ACCESS_TOKEN @@ -153,7 +151,7 @@ public function testGivenExceptionFromEntityManager_authorizerWrapsExceptionForS } public function testGivenExceptionFromEntityManager_authorizerWrapsExceptionForAccessCheck(): void { - $authorizer = new DoctrineDonationAuthorizer( + $authorizer = new DoctrineDonationAuthorizationChecker( $this->getThrowingEntityManager(), self::CORRECT_UPDATE_TOKEN, self::CORRECT_ACCESS_TOKEN diff --git a/tests/Integration/DonationAcceptedEventHandlerTest.php b/tests/Integration/DonationAcceptedEventHandlerTest.php index 1c9c81ad..d2cef3a0 100644 --- a/tests/Integration/DonationAcceptedEventHandlerTest.php +++ b/tests/Integration/DonationAcceptedEventHandlerTest.php @@ -6,7 +6,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\DonationAcceptedEventHandler; @@ -24,7 +24,7 @@ class DonationAcceptedEventHandlerTest extends TestCase { private const UNKNOWN_ID = 32202; private const KNOWN_ID = 31337; - private DonationAuthorizer $authorizer; + private DonationAuthorizationChecker $authorizer; private DonationRepository $repository; private MockObject&DonationNotifier $mailer; diff --git a/tests/Integration/UseCases/AddComment/AddCommentUseCaseTest.php b/tests/Integration/UseCases/AddComment/AddCommentUseCaseTest.php index fa5e910a..30a60d80 100644 --- a/tests/Integration/UseCases/AddComment/AddCommentUseCaseTest.php +++ b/tests/Integration/UseCases/AddComment/AddCommentUseCaseTest.php @@ -5,7 +5,7 @@ namespace WMDE\Fundraising\DonationContext\Tests\Integration\UseCases\AddComment; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Model\DonationComment; use WMDE\Fundraising\DonationContext\Domain\Model\ModerationIdentifier; use WMDE\Fundraising\DonationContext\Domain\Model\ModerationReason; @@ -31,7 +31,7 @@ class AddCommentUseCaseTest extends TestCase { private const COMMENT_IS_PUBLIC = true; private DonationRepository $donationRepository; - private DonationAuthorizer $authorizer; + private DonationAuthorizationChecker $authorizer; private TextPolicyValidator $textPolicyValidator; private AddCommentValidator $commentValidator; diff --git a/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php b/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php index 0091eece..f5cb1629 100644 --- a/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php +++ b/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php @@ -5,8 +5,7 @@ namespace WMDE\Fundraising\DonationContext\Tests\Integration\UseCases\AddDonation; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationTokenFetcher; -use WMDE\Fundraising\DonationContext\Authorization\DonationTokens; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; use WMDE\Fundraising\DonationContext\Domain\Event\DonationCreatedEvent; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Model\Donor\Name\CompanyName; @@ -20,10 +19,8 @@ use WMDE\Fundraising\DonationContext\Tests\Fixtures\CreatePaymentServiceSpy; use WMDE\Fundraising\DonationContext\Tests\Fixtures\EventEmitterSpy; use WMDE\Fundraising\DonationContext\Tests\Fixtures\FakeDonationRepository; -use WMDE\Fundraising\DonationContext\Tests\Fixtures\FixedDonationTokenFetcher; use WMDE\Fundraising\DonationContext\Tests\Fixtures\StaticDonationIdRepository; use WMDE\Fundraising\DonationContext\Tests\Fixtures\SucceedingPaymentServiceStub; -use WMDE\Fundraising\DonationContext\Tests\Fixtures\UrlGeneratorSpy; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\AddDonationRequest; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\AddDonationUseCase; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\AddDonationValidationResult; @@ -33,8 +30,7 @@ use WMDE\Fundraising\DonationContext\UseCases\AddDonation\Moderation\ModerationService; use WMDE\Fundraising\DonationContext\UseCases\DonationNotifier; use WMDE\Fundraising\PaymentContext\Domain\Model\PaymentInterval; -use WMDE\Fundraising\PaymentContext\Domain\PaymentUrlGenerator\NullGenerator; -use WMDE\Fundraising\PaymentContext\Domain\PaymentUrlGenerator\PaymentProviderURLGenerator; +use WMDE\Fundraising\PaymentContext\Services\URLAuthenticator; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\FailureResponse as PaymentCreationFailed; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\SuccessResponse as PaymentCreationSucceeded; @@ -46,8 +42,6 @@ */ class AddDonationUseCaseTest extends TestCase { - private const UPDATE_TOKEN = 'a very nice token'; - private const ACCESS_TOKEN = 'kindly allow me access'; private const PAYMENT_PROVIDER_URL = 'https://paypal.example.com/'; public function testWhenValidationSucceeds_successResponseIsCreated(): void { @@ -169,7 +163,7 @@ public function testGivenValidRequest_moderationEmailIsSent(): void { public function testGivenValidRequest_withIncompletePayment_confirmationEmailIsNotSent(): void { $paymentService = new SucceedingPaymentServiceStub( new PaymentCreationSucceeded( paymentId: 1, - paymentProviderURLGenerator: new NullGenerator(), + externalPaymentCompletionUrl: '', paymentComplete: false ) ); $mockNotifier = $this->createMock( DonationNotifier::class ); @@ -227,23 +221,24 @@ private function newValidCompanyDonationRequest(): AddDonationRequest { return $request; } - public function testSuccessResponseContainsTokens(): void { - $returnedTokens = new DonationTokens( 'a110-acce55', 'a110-00d8e' ); + public function testUrlAuthenticatorIsPassedToPaymentCreationRequest(): void { + $urlAuthenticator = $this->makeUrlAuthenticatorStub(); + $donationAuthorizer = $this->makeDonationAuthorizerStub( $urlAuthenticator ); + $paymentService = new CreatePaymentServiceSpy(); $useCase = $this->makeUseCase( - tokenFetcher: $this->makeFakeTokenFetcher( $returnedTokens ) + donationAuthorizer: $donationAuthorizer, + paymentService: $paymentService ); - $response = $useCase->addDonation( $this->newMinimumDonationRequest() ); + $useCase->addDonation( $this->newMinimumDonationRequest() ); - $this->assertSame( 'a110-acce55', $response->getAccessToken() ); - $this->assertSame( 'a110-00d8e', $response->getUpdateToken() ); + $lastRequest = $paymentService->getLastRequest(); + $this->assertSame( $urlAuthenticator, $lastRequest->urlAuthenticator ); } public function testSuccessResponseContainsGeneratedUrl(): void { - $urlGeneratorStub = $this->createStub( PaymentProviderURLGenerator::class ); - $urlGeneratorStub->method( 'generateURL' )->willReturn( self::PAYMENT_PROVIDER_URL ); $useCase = $this->makeUseCase( - paymentService: $this->makeSuccessfulPaymentServiceWithUrlGenerator( $urlGeneratorStub ) + paymentService: $this->makeSuccessfulPaymentServiceWithUrl() ); $response = $useCase->addDonation( $this->newMinimumDonationRequest() ); @@ -252,32 +247,30 @@ public function testSuccessResponseContainsGeneratedUrl(): void { } public function testUrlGeneratorGetsDonationData(): void { - $urlGenerator = new UrlGeneratorSpy(); + $paymentService = new CreatePaymentServiceSpy(); $useCase = $this->makeUseCase( idGenerator: new StaticDonationIdRepository(), - paymentService: $this->makeSuccessfulPaymentServiceWithUrlGenerator( $urlGenerator ) + paymentService: $paymentService ); - $response = $useCase->addDonation( $this->newValidAddDonationRequestWithEmail( 'irrelevant@example.com' ) ); + $useCase->addDonation( $this->newValidAddDonationRequestWithEmail( 'irrelevant@example.com' ) ); - $context = $urlGenerator->getLastContext(); + $context = $paymentService->getLastRequest()->domainSpecificContext; $this->assertSame( StaticDonationIdRepository::DONATION_ID, $context->itemId ); $this->assertSame( 'D' . StaticDonationIdRepository::DONATION_ID, $context->invoiceId ); - $this->assertSame( $response->getAccessToken(), $context->accessToken ); - $this->assertSame( $response->getUpdateToken(), $context->updateToken ); $this->assertSame( ValidDonation::DONOR_FIRST_NAME, $context->firstName ); $this->assertSame( ValidDonation::DONOR_LAST_NAME, $context->lastName ); } public function testGivenAnonymousDonation_UrlGeneratorGetsEmptyNames(): void { - $urlGenerator = new UrlGeneratorSpy(); + $paymentService = new CreatePaymentServiceSpy(); $useCase = $this->makeUseCase( - paymentService: $this->makeSuccessfulPaymentServiceWithUrlGenerator( $urlGenerator ) + paymentService: $paymentService ); $useCase->addDonation( $this->newMinimumDonationRequest() ); - $context = $urlGenerator->getLastContext(); + $context = $paymentService->getLastRequest()->domainSpecificContext; $this->assertSame( '', $context->firstName ); $this->assertSame( '', $context->lastName ); } @@ -387,7 +380,7 @@ private function makeUseCase( ?AddDonationValidator $donationValidator = null, ?ModerationService $policyValidator = null, ?DonationNotifier $notifier = null, - ?DonationTokenFetcher $tokenFetcher = null, + ?DonationAuthorizer $donationAuthorizer = null, ?EventEmitter $eventEmitter = null, ?CreatePaymentService $paymentService = null, ): AddDonationUseCase { @@ -397,7 +390,7 @@ private function makeUseCase( $donationValidator ?? $this->makeFakeSucceedingDonationValidator(), $policyValidator ?? $this->makeFakeSucceedingModerationService(), $notifier ?? $this->makeNotifierStub(), - $tokenFetcher ?? $this->makeFakeTokenFetcher(), + $donationAuthorizer ?? $this->makeDonationAuthorizerStub(), $eventEmitter ?? new EventEmitterSpy(), $paymentService ?? new SucceedingPaymentServiceStub() ); @@ -444,19 +437,11 @@ private function makeNotifierStub(): DonationNotifier { return $this->createStub( DonationNotifier::class ); } - private function makeFakeTokenFetcher( ?DonationTokens $tokens = null ): DonationTokenFetcher { - $tokens = $tokens ?? new DonationTokens( - self::ACCESS_TOKEN, - self::UPDATE_TOKEN - ); - return new FixedDonationTokenFetcher( $tokens ); - } - - private function makeSuccessfulPaymentServiceWithUrlGenerator( PaymentProviderURLGenerator $urlGeneratorStub ): CreatePaymentService { + private function makeSuccessfulPaymentServiceWithUrl(): CreatePaymentService { $paymentService = $this->createStub( CreatePaymentService::class ); $paymentService->method( 'createPayment' )->willReturn( new PaymentCreationSucceeded( 1, - $urlGeneratorStub, + self::PAYMENT_PROVIDER_URL, true ) ); return $paymentService; @@ -468,4 +453,17 @@ private function makeFailingPaymentService( string $message ): CreatePaymentServ return $paymentService; } + private function makeDonationAuthorizerStub( ?URLAuthenticator $authenticator = null ): DonationAuthorizer { + $authorizer = $this->createStub( DonationAuthorizer::class ); + $authorizer->method( 'authorizeDonationAccess' )->willReturn( $authenticator ?? $this->makeUrlAuthenticatorStub() ); + return $authorizer; + } + + private function makeUrlAuthenticatorStub(): URLAuthenticator { + $authenticator = $this->createStub( URLAuthenticator::class ); + $authenticator->method( 'addAuthenticationTokensToApplicationUrl' )->willReturnArgument( 0 ); + $authenticator->method( 'getAuthenticationTokensForPaymentProviderUrl' )->willReturn( [] ); + return $authenticator; + } + } diff --git a/tests/Integration/UseCases/CancelDonation/CancelDonationUseCaseTest.php b/tests/Integration/UseCases/CancelDonation/CancelDonationUseCaseTest.php index f545cfc6..154d1b21 100644 --- a/tests/Integration/UseCases/CancelDonation/CancelDonationUseCaseTest.php +++ b/tests/Integration/UseCases/CancelDonation/CancelDonationUseCaseTest.php @@ -7,7 +7,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use WMDE\EmailAddress\EmailAddress; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\Infrastructure\DonationEventLogger; @@ -36,7 +36,7 @@ class CancelDonationUseCaseTest extends TestCase { private function newCancelDonationUseCase( DonationRepository $repository = null, TemplateMailerInterface $mailer = null, - DonationAuthorizer $authorizer = null, + DonationAuthorizationChecker $authorizer = null, DonationEventLogger $logger = null, CancelPaymentUseCase $cancelPaymentUseCase = null ): CancelDonationUseCase { diff --git a/tests/Integration/UseCases/UpdateDonor/UpdateDonorUseCaseTest.php b/tests/Integration/UseCases/UpdateDonor/UpdateDonorUseCaseTest.php index ec7ed314..e7d837fb 100644 --- a/tests/Integration/UseCases/UpdateDonor/UpdateDonorUseCaseTest.php +++ b/tests/Integration/UseCases/UpdateDonor/UpdateDonorUseCaseTest.php @@ -5,7 +5,7 @@ namespace WMDE\Fundraising\DonationContext\Tests\Integration\UseCases\UpdateDonor; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Event\DonorUpdatedEvent; use WMDE\Fundraising\DonationContext\Domain\Model\Donor\AnonymousDonor; use WMDE\Fundraising\DonationContext\Domain\Model\DonorType; @@ -173,7 +173,7 @@ private function newRepository(): DonationRepository { return new FakeDonationRepository(); } - private function newDonationAuthorizer(): DonationAuthorizer { + private function newDonationAuthorizer(): DonationAuthorizationChecker { return new SucceedingDonationAuthorizer(); } @@ -183,7 +183,7 @@ private function newDonorValidator(): UpdateDonorValidator { return $validator; } - private function newUpdateDonorUseCase( DonationRepository $repository, ?DonationNotifier $confirmationMailer = null, ?EventEmitter $eventEmitter = null, ?UpdateDonorValidator $donorValidator = null, ?DonationAuthorizer $donationAuthorizer = null ): UpdateDonorUseCase { + private function newUpdateDonorUseCase( DonationRepository $repository, ?DonationNotifier $confirmationMailer = null, ?EventEmitter $eventEmitter = null, ?UpdateDonorValidator $donorValidator = null, ?DonationAuthorizationChecker $donationAuthorizer = null ): UpdateDonorUseCase { return new UpdateDonorUseCase( $donationAuthorizer ?? $this->newDonationAuthorizer(), $donorValidator ?? $this->newDonorValidator(), diff --git a/tests/Unit/UseCases/CreditCardPaymentNotification/CreditCardNotificationUseCaseTest.php b/tests/Unit/UseCases/CreditCardPaymentNotification/CreditCardNotificationUseCaseTest.php index 256ee1c2..960bb766 100644 --- a/tests/Unit/UseCases/CreditCardPaymentNotification/CreditCardNotificationUseCaseTest.php +++ b/tests/Unit/UseCases/CreditCardPaymentNotification/CreditCardNotificationUseCaseTest.php @@ -5,7 +5,7 @@ namespace WMDE\Fundraising\DonationContext\Tests\Unit\UseCases\CreditCardPaymentNotification; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationIdRepository; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\Infrastructure\DonationEventLogger; @@ -145,7 +145,7 @@ public function testWhenPaymentBookingServiceFails_handlerReturnsFailure(): void private function newCreditCardNotificationUseCase( ?DonationIdRepository $idGenerator = null, ?DonationRepository $donationRepository = null, - ?DonationAuthorizer $authorizer = null, + ?DonationAuthorizationChecker $authorizer = null, ?DonationNotifier $notifier = null, ?PaymentBookingService $paymentBookingService = null, ?DonationEventLogger $eventLogger = null diff --git a/tests/Unit/UseCases/HandlePayPalPaymentNotification/HandlePayPalPaymentCompletionNotificationUseCaseTest.php b/tests/Unit/UseCases/HandlePayPalPaymentNotification/HandlePayPalPaymentCompletionNotificationUseCaseTest.php index fe889e60..d957710f 100644 --- a/tests/Unit/UseCases/HandlePayPalPaymentNotification/HandlePayPalPaymentCompletionNotificationUseCaseTest.php +++ b/tests/Unit/UseCases/HandlePayPalPaymentNotification/HandlePayPalPaymentCompletionNotificationUseCaseTest.php @@ -6,7 +6,7 @@ use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; +use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationIdRepository; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\Infrastructure\DonationEventLogger; @@ -248,7 +248,7 @@ public function testWhenPaymentIsAlreadyBooked_returnsPaymentAlreadyBookedRespon private function givenNewUseCase( ?DonationIdRepository $idGenerator = null, ?DonationRepository $repository = null, - ?DonationAuthorizer $authorizer = null, + ?DonationAuthorizationChecker $authorizer = null, ?DonationNotifier $notifier = null, ?PaymentBookingService $paymentBookingService = null, ?DonationEventLogger $eventLogger = null, From cde5ce459e84de9e212e194e7df9afc39adfe6d4 Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Tue, 22 Aug 2023 11:29:54 +0200 Subject: [PATCH 02/10] Deprecate classes The DonationFetcher class should be phased out We also don't want the DoctrineDonationPrePersistSubscriber https://phabricator.wikimedia.org/T341795 --- src/DataAccess/DoctrineDonationPrePersistSubscriber.php | 3 +++ src/DataAccess/DoctrineDonationTokenFetcher.php | 2 +- src/DonationContextFactory.php | 4 ++++ src/Infrastructure/HttpDonationNotifier.php | 4 ++++ src/UseCases/GetDonation/GetDonationResponse.php | 7 +++++++ src/UseCases/GetDonation/GetDonationUseCase.php | 5 +++++ 6 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/DataAccess/DoctrineDonationPrePersistSubscriber.php b/src/DataAccess/DoctrineDonationPrePersistSubscriber.php index bd0b6333..4f5a6b5a 100644 --- a/src/DataAccess/DoctrineDonationPrePersistSubscriber.php +++ b/src/DataAccess/DoctrineDonationPrePersistSubscriber.php @@ -10,6 +10,9 @@ use WMDE\Fundraising\DonationContext\Authorization\TokenGenerator; use WMDE\Fundraising\DonationContext\DataAccess\DoctrineEntities\Donation; +/** + * @deprecated We don't want to use Doctrine event subscribers any more. + */ class DoctrineDonationPrePersistSubscriber implements EventSubscriber { private const DATE_TIME_FORMAT = 'Y-m-d H:i:s'; diff --git a/src/DataAccess/DoctrineDonationTokenFetcher.php b/src/DataAccess/DoctrineDonationTokenFetcher.php index 9c5f90b4..2b970f59 100644 --- a/src/DataAccess/DoctrineDonationTokenFetcher.php +++ b/src/DataAccess/DoctrineDonationTokenFetcher.php @@ -11,7 +11,7 @@ use WMDE\Fundraising\DonationContext\DataAccess\DoctrineEntities\Donation; /** - * @license GPL-2.0-or-later + * @deprecated The HttpDonationNotifier should get a different implementation of DonationTokenFetcher and GetDonationUseCase should not return tokens */ class DoctrineDonationTokenFetcher implements DonationTokenFetcher { diff --git a/src/DonationContextFactory.php b/src/DonationContextFactory.php index 5123e17d..73f44d06 100644 --- a/src/DonationContextFactory.php +++ b/src/DonationContextFactory.php @@ -36,6 +36,7 @@ public function __construct( array $config ) { /** * @return EventSubscriber[] + * @deprecated Use we don't want to use Doctrine event subscribers any more. */ public function newEventSubscribers(): array { return [ @@ -50,6 +51,9 @@ public function getDoctrineMappingPaths(): array { return [ self::DOCTRINE_CLASS_MAPPING_DIRECTORY ]; } + /** + * @deprecated Use we don't want to use Doctrine event subscribers any more. + */ private function newDoctrineDonationPrePersistSubscriber(): DoctrineDonationPrePersistSubscriber { $tokenGenerator = $this->getTokenGenerator(); return new DoctrineDonationPrePersistSubscriber( diff --git a/src/Infrastructure/HttpDonationNotifier.php b/src/Infrastructure/HttpDonationNotifier.php index 76ef4b19..49387741 100644 --- a/src/Infrastructure/HttpDonationNotifier.php +++ b/src/Infrastructure/HttpDonationNotifier.php @@ -19,6 +19,10 @@ */ class HttpDonationNotifier implements DonationNotifier { + /** + * @var DonationTokenFetcher + * @deprecated The FOC should use a special class to get donation tokens for this + */ private DonationTokenFetcher $tokenFetcher; private HttpClientInterface $httpClient; private string $endpointUrl; diff --git a/src/UseCases/GetDonation/GetDonationResponse.php b/src/UseCases/GetDonation/GetDonationResponse.php index 36a70f1d..9be19d81 100644 --- a/src/UseCases/GetDonation/GetDonationResponse.php +++ b/src/UseCases/GetDonation/GetDonationResponse.php @@ -9,6 +9,10 @@ class GetDonationResponse { private ?Donation $donation; + /** + * @var string|null Token to be used for updating the donation + * @deprecated The calling code should get the tokens for the URL elsewhere + */ private ?string $updateToken; public static function newNotAllowedResponse(): self { @@ -41,6 +45,9 @@ public function accessIsPermitted(): bool { return $this->donation !== null; } + /** + * @deprecated The calling code should get the tokens for the URL elsewhere + */ public function getUpdateToken(): ?string { return $this->updateToken; } diff --git a/src/UseCases/GetDonation/GetDonationUseCase.php b/src/UseCases/GetDonation/GetDonationUseCase.php index 4e4882b2..c30a53b3 100644 --- a/src/UseCases/GetDonation/GetDonationUseCase.php +++ b/src/UseCases/GetDonation/GetDonationUseCase.php @@ -12,6 +12,11 @@ class GetDonationUseCase { + /** + * @param DonationAuthorizationChecker $authorizer + * @param DonationTokenFetcher $tokenFetcher (deprecated) + * @param DonationRepository $donationRepository + */ public function __construct( private readonly DonationAuthorizationChecker $authorizer, private readonly DonationTokenFetcher $tokenFetcher, From 9832beeab9017eca49662934993a23f70307bf93 Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Wed, 23 Aug 2023 15:24:09 +0200 Subject: [PATCH 03/10] Deprecate and remove more classes With https://github.com/wmde/fundraising-application/pull/2825 being prepared, we can now remove and deprecate more outdated dependencies. This is another BC breaking change. https://phabricator.wikimedia.org/T341795 --- src/Authorization/DonationTokenFetcher.php | 3 +- .../DonationTokenFetchingException.php | 3 +- src/Authorization/DonationTokens.php | 2 +- src/Authorization/RandomTokenGenerator.php | 3 +- src/Authorization/TokenGenerator.php | 3 +- .../DoctrineDonationAuthorizationChecker.php | 5 +- src/DonationAcceptedEventHandler.php | 2 +- src/DonationContextFactory.php | 48 ------------------- .../DonationAuthorizationChecker.php | 6 +-- .../DonationAuthorizer.php | 2 +- src/Infrastructure/HttpDonationNotifier.php | 26 ++++------ .../HttpDonationNotifierUrlAuthorizer.php | 14 ++++++ src/UseCases/AddComment/AddCommentUseCase.php | 2 +- .../AddDonation/AddDonationUseCase.php | 2 +- .../BookDonationUseCase.php | 2 +- .../CancelDonation/CancelDonationUseCase.php | 2 +- .../GetDonation/GetDonationResponse.php | 19 ++------ .../GetDonation/GetDonationUseCase.php | 8 +--- .../UpdateDonor/UpdateDonorUseCase.php | 2 +- tests/Fixtures/FailingDonationAuthorizer.php | 2 +- tests/Fixtures/FixedDonationTokenFetcher.php | 2 +- tests/Fixtures/FixedTokenGenerator.php | 3 ++ .../Fixtures/SucceedingDonationAuthorizer.php | 2 +- .../SucceedingDonationAuthorizerSpy.php | 2 +- ...ctrineDonationAuthorizationCheckerTest.php | 3 +- .../DoctrineDonationRepositoryTest.php | 8 ---- .../DonationAcceptedEventHandlerTest.php | 2 +- .../AddComment/AddCommentUseCaseTest.php | 2 +- .../AddDonation/AddDonationUseCaseTest.php | 2 +- .../CancelDonationUseCaseTest.php | 2 +- .../GetDonation/GetDonationUseCaseTest.php | 18 +------ .../UpdateDonor/UpdateDonorUseCaseTest.php | 2 +- tests/TestDonationContextFactory.php | 28 ++--------- .../HttpDonationConfirmationNotifierTest.php | 10 ++-- .../CreditCardNotificationUseCaseTest.php | 2 +- ...ymentCompletionNotificationUseCaseTest.php | 2 +- 36 files changed, 67 insertions(+), 179 deletions(-) rename src/{Authorization => Infrastructure}/DonationAuthorizationChecker.php (80%) rename src/{Authorization => Infrastructure}/DonationAuthorizer.php (78%) create mode 100644 src/Infrastructure/HttpDonationNotifierUrlAuthorizer.php diff --git a/src/Authorization/DonationTokenFetcher.php b/src/Authorization/DonationTokenFetcher.php index d7e31045..9b9b237a 100644 --- a/src/Authorization/DonationTokenFetcher.php +++ b/src/Authorization/DonationTokenFetcher.php @@ -5,8 +5,7 @@ namespace WMDE\Fundraising\DonationContext\Authorization; /** - * @license GPL-2.0-or-later - * @author Jeroen De Dauw < jeroendedauw@gmail.com > + * @deprecated */ interface DonationTokenFetcher { diff --git a/src/Authorization/DonationTokenFetchingException.php b/src/Authorization/DonationTokenFetchingException.php index 6552ce4a..6ebbd422 100644 --- a/src/Authorization/DonationTokenFetchingException.php +++ b/src/Authorization/DonationTokenFetchingException.php @@ -5,8 +5,7 @@ namespace WMDE\Fundraising\DonationContext\Authorization; /** - * @license GPL-2.0-or-later - * @author Jeroen De Dauw < jeroendedauw@gmail.com > + * @deprecated */ class DonationTokenFetchingException extends \RuntimeException { diff --git a/src/Authorization/DonationTokens.php b/src/Authorization/DonationTokens.php index 72110f45..c895a028 100644 --- a/src/Authorization/DonationTokens.php +++ b/src/Authorization/DonationTokens.php @@ -5,7 +5,7 @@ namespace WMDE\Fundraising\DonationContext\Authorization; /** - * @license GPL-2.0-or-later + * @deprecated */ class DonationTokens { diff --git a/src/Authorization/RandomTokenGenerator.php b/src/Authorization/RandomTokenGenerator.php index 609e0cee..05ff8f18 100644 --- a/src/Authorization/RandomTokenGenerator.php +++ b/src/Authorization/RandomTokenGenerator.php @@ -5,8 +5,7 @@ namespace WMDE\Fundraising\DonationContext\Authorization; /** - * @license GPL-2.0-or-later - * @author Kai Nissen < kai.nissen@wikimedia.de > + * @deprecated Tokens should be generated outside the bounded context */ class RandomTokenGenerator implements TokenGenerator { diff --git a/src/Authorization/TokenGenerator.php b/src/Authorization/TokenGenerator.php index bd7bcddb..d540d3c5 100644 --- a/src/Authorization/TokenGenerator.php +++ b/src/Authorization/TokenGenerator.php @@ -5,8 +5,7 @@ namespace WMDE\Fundraising\DonationContext\Authorization; /** - * @license GPL-2.0-or-later - * @author Kai Nissen < kai.nissen@wikimedia.de > + * @deprecated Tokens should be generated outside the bounded context */ interface TokenGenerator { diff --git a/src/DataAccess/DoctrineDonationAuthorizationChecker.php b/src/DataAccess/DoctrineDonationAuthorizationChecker.php index 42492fde..015b3459 100644 --- a/src/DataAccess/DoctrineDonationAuthorizationChecker.php +++ b/src/DataAccess/DoctrineDonationAuthorizationChecker.php @@ -6,13 +6,12 @@ use Doctrine\ORM\EntityManager; use Doctrine\ORM\ORMException; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\DataAccess\DoctrineEntities\Donation; use WMDE\Fundraising\DonationContext\Domain\Repositories\GetDonationException; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; /** - * This is only for checking legacy donation authorizations. - * New donations should use an implementation of DonationAuthorizationChecker that uses tokens stored outside the bounded context. + * @deprecated Use the AuthorizationChecker from the Application layer instead */ class DoctrineDonationAuthorizationChecker implements DonationAuthorizationChecker { diff --git a/src/DonationAcceptedEventHandler.php b/src/DonationAcceptedEventHandler.php index 44b499c0..cbfcd8fa 100644 --- a/src/DonationAcceptedEventHandler.php +++ b/src/DonationAcceptedEventHandler.php @@ -4,9 +4,9 @@ namespace WMDE\Fundraising\DonationContext; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\Domain\Repositories\GetDonationException; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\UseCases\DonationNotifier; class DonationAcceptedEventHandler { diff --git a/src/DonationContextFactory.php b/src/DonationContextFactory.php index 73f44d06..eb564878 100644 --- a/src/DonationContextFactory.php +++ b/src/DonationContextFactory.php @@ -4,13 +4,8 @@ namespace WMDE\Fundraising\DonationContext; -use DateInterval; -use Doctrine\Common\EventSubscriber; use Doctrine\DBAL\Connection; use Doctrine\DBAL\Types\Type; -use WMDE\Fundraising\DonationContext\Authorization\RandomTokenGenerator; -use WMDE\Fundraising\DonationContext\Authorization\TokenGenerator; -use WMDE\Fundraising\DonationContext\DataAccess\DoctrineDonationPrePersistSubscriber; /** * @license GPL-2.0-or-later @@ -24,24 +19,11 @@ class DonationContextFactory { */ protected array $config; - protected ?TokenGenerator $tokenGenerator; - /** * @param array{token-length:int,token-validity-timestamp:string} $config */ public function __construct( array $config ) { $this->config = $config; - $this->tokenGenerator = null; - } - - /** - * @return EventSubscriber[] - * @deprecated Use we don't want to use Doctrine event subscribers any more. - */ - public function newEventSubscribers(): array { - return [ - DoctrineDonationPrePersistSubscriber::class => $this->newDoctrineDonationPrePersistSubscriber() - ]; } /** @@ -51,36 +33,6 @@ public function getDoctrineMappingPaths(): array { return [ self::DOCTRINE_CLASS_MAPPING_DIRECTORY ]; } - /** - * @deprecated Use we don't want to use Doctrine event subscribers any more. - */ - private function newDoctrineDonationPrePersistSubscriber(): DoctrineDonationPrePersistSubscriber { - $tokenGenerator = $this->getTokenGenerator(); - return new DoctrineDonationPrePersistSubscriber( - $tokenGenerator, - $tokenGenerator - ); - } - - private function getTokenGenerator(): TokenGenerator { - if ( $this->tokenGenerator === null ) { - $this->tokenGenerator = new RandomTokenGenerator( - $this->config['token-length'], - new DateInterval( $this->config['token-validity-timestamp'] ) - ); - } - return $this->tokenGenerator; - } - - /** - * Should only be called in tests for switching out the default implementation - * - * @param TokenGenerator|null $tokenGenerator - */ - public function setTokenGenerator( ?TokenGenerator $tokenGenerator ): void { - $this->tokenGenerator = $tokenGenerator; - } - public function registerCustomTypes( Connection $connection ): void { $this->registerDoctrineModerationIdentifierType( $connection ); } diff --git a/src/Authorization/DonationAuthorizationChecker.php b/src/Infrastructure/DonationAuthorizationChecker.php similarity index 80% rename from src/Authorization/DonationAuthorizationChecker.php rename to src/Infrastructure/DonationAuthorizationChecker.php index 6a814731..d2644ebe 100644 --- a/src/Authorization/DonationAuthorizationChecker.php +++ b/src/Infrastructure/DonationAuthorizationChecker.php @@ -2,12 +2,8 @@ declare( strict_types = 1 ); -namespace WMDE\Fundraising\DonationContext\Authorization; +namespace WMDE\Fundraising\DonationContext\Infrastructure; -/** - * @license GPL-2.0-or-later - * @author Jeroen De Dauw < jeroendedauw@gmail.com > - */ interface DonationAuthorizationChecker { /** diff --git a/src/Authorization/DonationAuthorizer.php b/src/Infrastructure/DonationAuthorizer.php similarity index 78% rename from src/Authorization/DonationAuthorizer.php rename to src/Infrastructure/DonationAuthorizer.php index b61743fd..92b66b33 100644 --- a/src/Authorization/DonationAuthorizer.php +++ b/src/Infrastructure/DonationAuthorizer.php @@ -1,7 +1,7 @@ tokenFetcher = $authorizer; + public function __construct( HttpDonationNotifierUrlAuthorizer $authorizer, HttpClientInterface $httpClient, string $endpointUrl ) { + $this->urlAuthorizer = $authorizer; $this->httpClient = $httpClient; $this->endpointUrl = $endpointUrl; } @@ -36,17 +30,13 @@ public function __construct( DonationTokenFetcher $authorizer, HttpClientInterfa /** * @param Donation $donation * @throws \Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface - * @throws DonationTokenFetchingException */ public function sendConfirmationFor( Donation $donation ): void { - $this->httpClient->request( - 'GET', - $this->endpointUrl, - [ 'query' => [ - 'donation_id' => $donation->getId(), - 'update_token' => $this->tokenFetcher->getTokens( $donation->getId() )->getUpdateToken() - ] ] - ); + $query = [ + 'donation_id' => $donation->getId(), + ]; + $query = $this->urlAuthorizer->addAuthorizationParameters( $donation->getId(), $query ); + $this->httpClient->request( 'GET', $this->endpointUrl, [ 'query' => $query ] ); } public function sendModerationNotificationToAdmin( Donation $donation ): void { diff --git a/src/Infrastructure/HttpDonationNotifierUrlAuthorizer.php b/src/Infrastructure/HttpDonationNotifierUrlAuthorizer.php new file mode 100644 index 00000000..69619717 --- /dev/null +++ b/src/Infrastructure/HttpDonationNotifierUrlAuthorizer.php @@ -0,0 +1,14 @@ + $parameters + * @return array + */ + public function addAuthorizationParameters( int $donationId, array $parameters ): array; +} diff --git a/src/UseCases/AddComment/AddCommentUseCase.php b/src/UseCases/AddComment/AddCommentUseCase.php index 743092c5..4da1a7b6 100644 --- a/src/UseCases/AddComment/AddCommentUseCase.php +++ b/src/UseCases/AddComment/AddCommentUseCase.php @@ -4,7 +4,6 @@ namespace WMDE\Fundraising\DonationContext\UseCases\AddComment; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Model\DonationComment; use WMDE\Fundraising\DonationContext\Domain\Model\Donor\Name\NoName; @@ -13,6 +12,7 @@ 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\FunValidators\Validators\TextPolicyValidator; /** diff --git a/src/UseCases/AddDonation/AddDonationUseCase.php b/src/UseCases/AddDonation/AddDonationUseCase.php index 2c96a785..17ea66a5 100644 --- a/src/UseCases/AddDonation/AddDonationUseCase.php +++ b/src/UseCases/AddDonation/AddDonationUseCase.php @@ -4,7 +4,6 @@ namespace WMDE\Fundraising\DonationContext\UseCases\AddDonation; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; use WMDE\Fundraising\DonationContext\Domain\Event\DonationCreatedEvent; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Model\DonationTrackingInfo; @@ -19,6 +18,7 @@ use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationIdRepository; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\EventEmitter; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizer; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\Moderation\ModerationService; use WMDE\Fundraising\DonationContext\UseCases\DonationNotifier; use WMDE\Fundraising\PaymentContext\Domain\UrlGenerator\DomainSpecificContext; diff --git a/src/UseCases/BookDonationUseCase/BookDonationUseCase.php b/src/UseCases/BookDonationUseCase/BookDonationUseCase.php index abfb7051..a4e9cdff 100644 --- a/src/UseCases/BookDonationUseCase/BookDonationUseCase.php +++ b/src/UseCases/BookDonationUseCase/BookDonationUseCase.php @@ -4,10 +4,10 @@ namespace WMDE\Fundraising\DonationContext\UseCases\BookDonationUseCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationIdRepository; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Infrastructure\DonationEventLogger; use WMDE\Fundraising\DonationContext\Services\PaymentBookingService; use WMDE\Fundraising\DonationContext\UseCases\DonationNotifier; diff --git a/src/UseCases/CancelDonation/CancelDonationUseCase.php b/src/UseCases/CancelDonation/CancelDonationUseCase.php index b0ee46bf..77561c07 100644 --- a/src/UseCases/CancelDonation/CancelDonationUseCase.php +++ b/src/UseCases/CancelDonation/CancelDonationUseCase.php @@ -5,11 +5,11 @@ namespace WMDE\Fundraising\DonationContext\UseCases\CancelDonation; use WMDE\EmailAddress\EmailAddress; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; 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; diff --git a/src/UseCases/GetDonation/GetDonationResponse.php b/src/UseCases/GetDonation/GetDonationResponse.php index 9be19d81..fccb1ec5 100644 --- a/src/UseCases/GetDonation/GetDonationResponse.php +++ b/src/UseCases/GetDonation/GetDonationResponse.php @@ -9,23 +9,17 @@ class GetDonationResponse { private ?Donation $donation; - /** - * @var string|null Token to be used for updating the donation - * @deprecated The calling code should get the tokens for the URL elsewhere - */ - private ?string $updateToken; public static function newNotAllowedResponse(): self { return new self( null ); } - public static function newValidResponse( Donation $donation, string $updateToken ): self { - return new self( $donation, $updateToken ); + public static function newValidResponse( Donation $donation ): self { + return new self( $donation ); } - private function __construct( Donation $donation = null, string $updateToken = null ) { + private function __construct( Donation $donation = null ) { $this->donation = $donation; - $this->updateToken = $updateToken; } /** @@ -45,11 +39,4 @@ public function accessIsPermitted(): bool { return $this->donation !== null; } - /** - * @deprecated The calling code should get the tokens for the URL elsewhere - */ - public function getUpdateToken(): ?string { - return $this->updateToken; - } - } diff --git a/src/UseCases/GetDonation/GetDonationUseCase.php b/src/UseCases/GetDonation/GetDonationUseCase.php index c30a53b3..c769a20a 100644 --- a/src/UseCases/GetDonation/GetDonationUseCase.php +++ b/src/UseCases/GetDonation/GetDonationUseCase.php @@ -4,22 +4,19 @@ namespace WMDE\Fundraising\DonationContext\UseCases\GetDonation; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; -use WMDE\Fundraising\DonationContext\Authorization\DonationTokenFetcher; 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\Infrastructure\DonationAuthorizationChecker; class GetDonationUseCase { /** * @param DonationAuthorizationChecker $authorizer - * @param DonationTokenFetcher $tokenFetcher (deprecated) * @param DonationRepository $donationRepository */ public function __construct( private readonly DonationAuthorizationChecker $authorizer, - private readonly DonationTokenFetcher $tokenFetcher, private readonly DonationRepository $donationRepository ) { } @@ -36,8 +33,7 @@ public function showConfirmation( GetDonationRequest $request ): GetDonationResp return GetDonationResponse::newValidResponse( // TODO: create a DTO to not expose the Donation Entity beyond the UC layer - $donation, - $this->tokenFetcher->getTokens( $request->getDonationId() )->getUpdateToken() + $donation ); } diff --git a/src/UseCases/UpdateDonor/UpdateDonorUseCase.php b/src/UseCases/UpdateDonor/UpdateDonorUseCase.php index a4f3013f..d695c265 100644 --- a/src/UseCases/UpdateDonor/UpdateDonorUseCase.php +++ b/src/UseCases/UpdateDonor/UpdateDonorUseCase.php @@ -4,7 +4,6 @@ namespace WMDE\Fundraising\DonationContext\UseCases\UpdateDonor; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Event\DonorUpdatedEvent; use WMDE\Fundraising\DonationContext\Domain\Model\Donor; use WMDE\Fundraising\DonationContext\Domain\Model\Donor\Address\PostalAddress; @@ -15,6 +14,7 @@ use WMDE\Fundraising\DonationContext\Domain\Model\DonorType; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\EventEmitter; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\UseCases\DonationNotifier; /** diff --git a/tests/Fixtures/FailingDonationAuthorizer.php b/tests/Fixtures/FailingDonationAuthorizer.php index f6a6f49a..92855942 100644 --- a/tests/Fixtures/FailingDonationAuthorizer.php +++ b/tests/Fixtures/FailingDonationAuthorizer.php @@ -4,7 +4,7 @@ namespace WMDE\Fundraising\DonationContext\Tests\Fixtures; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; /** * @license GPL-2.0-or-later diff --git a/tests/Fixtures/FixedDonationTokenFetcher.php b/tests/Fixtures/FixedDonationTokenFetcher.php index 52800c4e..3e008617 100644 --- a/tests/Fixtures/FixedDonationTokenFetcher.php +++ b/tests/Fixtures/FixedDonationTokenFetcher.php @@ -8,7 +8,7 @@ use WMDE\Fundraising\DonationContext\Authorization\DonationTokens; /** - * @license GPL-2.0-or-later + * @deprecated */ class FixedDonationTokenFetcher implements DonationTokenFetcher { diff --git a/tests/Fixtures/FixedTokenGenerator.php b/tests/Fixtures/FixedTokenGenerator.php index cfc40f31..1e31dbb0 100644 --- a/tests/Fixtures/FixedTokenGenerator.php +++ b/tests/Fixtures/FixedTokenGenerator.php @@ -6,6 +6,9 @@ use WMDE\Fundraising\DonationContext\Authorization\TokenGenerator; +/** + * @deprecated + */ class FixedTokenGenerator implements TokenGenerator { public const TOKEN = 'fixed_token'; diff --git a/tests/Fixtures/SucceedingDonationAuthorizer.php b/tests/Fixtures/SucceedingDonationAuthorizer.php index a1556572..f3236d9a 100644 --- a/tests/Fixtures/SucceedingDonationAuthorizer.php +++ b/tests/Fixtures/SucceedingDonationAuthorizer.php @@ -4,7 +4,7 @@ namespace WMDE\Fundraising\DonationContext\Tests\Fixtures; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; /** * @license GPL-2.0-or-later diff --git a/tests/Fixtures/SucceedingDonationAuthorizerSpy.php b/tests/Fixtures/SucceedingDonationAuthorizerSpy.php index 8b74ae7f..6f390655 100644 --- a/tests/Fixtures/SucceedingDonationAuthorizerSpy.php +++ b/tests/Fixtures/SucceedingDonationAuthorizerSpy.php @@ -4,7 +4,7 @@ namespace WMDE\Fundraising\DonationContext\Tests\Fixtures; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; /** * @license GPL-2.0-or-later diff --git a/tests/Integration/DataAccess/DoctrineDonationAuthorizationCheckerTest.php b/tests/Integration/DataAccess/DoctrineDonationAuthorizationCheckerTest.php index a502adbd..f9017068 100644 --- a/tests/Integration/DataAccess/DoctrineDonationAuthorizationCheckerTest.php +++ b/tests/Integration/DataAccess/DoctrineDonationAuthorizationCheckerTest.php @@ -7,14 +7,15 @@ use Doctrine\ORM\EntityManager; use Doctrine\ORM\Exception\ORMException; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\DataAccess\DoctrineDonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\DataAccess\DoctrineEntities\Donation; use WMDE\Fundraising\DonationContext\Domain\Repositories\GetDonationException; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Tests\TestEnvironment; /** * @covers \WMDE\Fundraising\DonationContext\DataAccess\DoctrineDonationAuthorizationChecker + * @deprecated Delete when deleting DoctrineDonationAuthorizationChecker */ class DoctrineDonationAuthorizationCheckerTest extends TestCase { diff --git a/tests/Integration/DataAccess/DoctrineDonationRepositoryTest.php b/tests/Integration/DataAccess/DoctrineDonationRepositoryTest.php index 79351387..4bb06dc1 100644 --- a/tests/Integration/DataAccess/DoctrineDonationRepositoryTest.php +++ b/tests/Integration/DataAccess/DoctrineDonationRepositoryTest.php @@ -18,7 +18,6 @@ use WMDE\Fundraising\DonationContext\Tests\Data\ValidDonation; use WMDE\Fundraising\DonationContext\Tests\Data\ValidPayments; use WMDE\Fundraising\DonationContext\Tests\Fixtures\FailingDonationExistsChecker; -use WMDE\Fundraising\DonationContext\Tests\Fixtures\FixedTokenGenerator; use WMDE\Fundraising\DonationContext\Tests\Fixtures\SucceedingDonationExistsChecker; use WMDE\Fundraising\DonationContext\Tests\Fixtures\ThrowingEntityManager; use WMDE\Fundraising\DonationContext\Tests\TestEnvironment; @@ -81,13 +80,6 @@ private function assertDoctrineEntityIsInDatabase( DoctrineDonation $expected ): $this->assertNotNull( $actual->getCreationTime() ); $expected->setCreationTime( $actual->getCreationTime() ); - // pre-persist subscriber automatically access and update tokens. We're using fixed values in the test - $expected->encodeAndSetData( array_merge( $expected->getDecodedData(), [ - 'token' => FixedTokenGenerator::TOKEN, - 'utoken' => FixedTokenGenerator::TOKEN, - 'uexpiry' => FixedTokenGenerator::EXPIRY_DATE - ] ) ); - $this->assertEquals( $expected->getModerationReasons()->toArray(), $actual->getModerationReasons()->toArray() ); $this->assertEquals( $expected->getDecodedData(), $actual->getDecodedData() ); diff --git a/tests/Integration/DonationAcceptedEventHandlerTest.php b/tests/Integration/DonationAcceptedEventHandlerTest.php index d2cef3a0..7bb2aeb8 100644 --- a/tests/Integration/DonationAcceptedEventHandlerTest.php +++ b/tests/Integration/DonationAcceptedEventHandlerTest.php @@ -6,10 +6,10 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\DonationAcceptedEventHandler; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Tests\Data\ValidDonation; use WMDE\Fundraising\DonationContext\Tests\Fixtures\FailingDonationAuthorizer; use WMDE\Fundraising\DonationContext\Tests\Fixtures\FakeDonationRepository; diff --git a/tests/Integration/UseCases/AddComment/AddCommentUseCaseTest.php b/tests/Integration/UseCases/AddComment/AddCommentUseCaseTest.php index 30a60d80..56e938ed 100644 --- a/tests/Integration/UseCases/AddComment/AddCommentUseCaseTest.php +++ b/tests/Integration/UseCases/AddComment/AddCommentUseCaseTest.php @@ -5,11 +5,11 @@ namespace WMDE\Fundraising\DonationContext\Tests\Integration\UseCases\AddComment; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Model\DonationComment; use WMDE\Fundraising\DonationContext\Domain\Model\ModerationIdentifier; use WMDE\Fundraising\DonationContext\Domain\Model\ModerationReason; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Tests\Data\ValidDonation; use WMDE\Fundraising\DonationContext\Tests\Fixtures\FailingDonationAuthorizer; use WMDE\Fundraising\DonationContext\Tests\Fixtures\FakeDonationRepository; diff --git a/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php b/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php index f5cb1629..c6950368 100644 --- a/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php +++ b/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php @@ -5,7 +5,6 @@ namespace WMDE\Fundraising\DonationContext\Tests\Integration\UseCases\AddDonation; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizer; use WMDE\Fundraising\DonationContext\Domain\Event\DonationCreatedEvent; use WMDE\Fundraising\DonationContext\Domain\Model\Donation; use WMDE\Fundraising\DonationContext\Domain\Model\Donor\Name\CompanyName; @@ -15,6 +14,7 @@ use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationIdRepository; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\EventEmitter; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizer; use WMDE\Fundraising\DonationContext\Tests\Data\ValidDonation; use WMDE\Fundraising\DonationContext\Tests\Fixtures\CreatePaymentServiceSpy; use WMDE\Fundraising\DonationContext\Tests\Fixtures\EventEmitterSpy; diff --git a/tests/Integration/UseCases/CancelDonation/CancelDonationUseCaseTest.php b/tests/Integration/UseCases/CancelDonation/CancelDonationUseCaseTest.php index 154d1b21..ddc2a141 100644 --- a/tests/Integration/UseCases/CancelDonation/CancelDonationUseCaseTest.php +++ b/tests/Integration/UseCases/CancelDonation/CancelDonationUseCaseTest.php @@ -7,9 +7,9 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use WMDE\EmailAddress\EmailAddress; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; 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; diff --git a/tests/Integration/UseCases/GetDonation/GetDonationUseCaseTest.php b/tests/Integration/UseCases/GetDonation/GetDonationUseCaseTest.php index 1c95df4e..1b84604a 100644 --- a/tests/Integration/UseCases/GetDonation/GetDonationUseCaseTest.php +++ b/tests/Integration/UseCases/GetDonation/GetDonationUseCaseTest.php @@ -4,31 +4,24 @@ namespace WMDE\Fundraising\DonationContext\Tests\Integration\UseCases\GetDonation; -use WMDE\Fundraising\DonationContext\Authorization\DonationTokens; +use PHPUnit\Framework\TestCase; use WMDE\Fundraising\DonationContext\Tests\Data\ValidDonation; use WMDE\Fundraising\DonationContext\Tests\Fixtures\FailingDonationAuthorizer; use WMDE\Fundraising\DonationContext\Tests\Fixtures\FakeDonationRepository; -use WMDE\Fundraising\DonationContext\Tests\Fixtures\FixedDonationTokenFetcher; use WMDE\Fundraising\DonationContext\Tests\Fixtures\SucceedingDonationAuthorizer; use WMDE\Fundraising\DonationContext\UseCases\GetDonation\GetDonationRequest; use WMDE\Fundraising\DonationContext\UseCases\GetDonation\GetDonationUseCase; /** * @covers \WMDE\Fundraising\DonationContext\UseCases\GetDonation\GetDonationUseCase - * - * @license GPL-2.0-or-later - * @author Jeroen De Dauw < jeroendedauw@gmail.com > */ -class GetDonationUseCaseTest extends \PHPUnit\Framework\TestCase { +class GetDonationUseCaseTest extends TestCase { private const CORRECT_DONATION_ID = 1; - private const ACCESS_TOKEN = 'some token'; - private const UPDATE_TOKEN = 'some other token'; public function testWhenAuthorizerSaysNoCanHaz_accessIsNotPermitted(): void { $useCase = new GetDonationUseCase( new FailingDonationAuthorizer(), - $this->newFixedTokenFetcher(), new FakeDonationRepository( ValidDonation::newDirectDebitDonation() ) ); @@ -45,7 +38,6 @@ public function testWhenAuthorizerSaysNoCanHaz_accessIsNotPermitted(): void { public function testWhenAuthorizerSaysSureThingBro_accessIsPermitted(): void { $useCase = new GetDonationUseCase( new SucceedingDonationAuthorizer(), - $this->newFixedTokenFetcher(), new FakeDonationRepository( ValidDonation::newDirectDebitDonation() ) ); @@ -61,7 +53,6 @@ public function testWhenAuthorizerSaysSureThingBro_accessIsPermitted(): void { public function testWhenDonationDoesNotExist_accessIsNotPermitted(): void { $useCase = new GetDonationUseCase( new SucceedingDonationAuthorizer(), - $this->newFixedTokenFetcher(), new FakeDonationRepository() ); @@ -80,7 +71,6 @@ public function testWhenDonationExistsAndAccessIsAllowed_donationIsReturned(): v $useCase = new GetDonationUseCase( new SucceedingDonationAuthorizer(), - $this->newFixedTokenFetcher(), new FakeDonationRepository( $donation ) ); @@ -92,8 +82,4 @@ public function testWhenDonationExistsAndAccessIsAllowed_donationIsReturned(): v $this->assertEquals( $donation, $response->getDonation() ); } - - private function newFixedTokenFetcher(): FixedDonationTokenFetcher { - return new FixedDonationTokenFetcher( new DonationTokens( self::ACCESS_TOKEN, self::UPDATE_TOKEN ) ); - } } diff --git a/tests/Integration/UseCases/UpdateDonor/UpdateDonorUseCaseTest.php b/tests/Integration/UseCases/UpdateDonor/UpdateDonorUseCaseTest.php index e7d837fb..913a08f6 100644 --- a/tests/Integration/UseCases/UpdateDonor/UpdateDonorUseCaseTest.php +++ b/tests/Integration/UseCases/UpdateDonor/UpdateDonorUseCaseTest.php @@ -5,12 +5,12 @@ namespace WMDE\Fundraising\DonationContext\Tests\Integration\UseCases\UpdateDonor; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Event\DonorUpdatedEvent; use WMDE\Fundraising\DonationContext\Domain\Model\Donor\AnonymousDonor; use WMDE\Fundraising\DonationContext\Domain\Model\DonorType; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; use WMDE\Fundraising\DonationContext\EventEmitter; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Tests\Data\ValidDonation; use WMDE\Fundraising\DonationContext\Tests\Fixtures\EventEmitterSpy; use WMDE\Fundraising\DonationContext\Tests\Fixtures\FailingDonationAuthorizer; diff --git a/tests/TestDonationContextFactory.php b/tests/TestDonationContextFactory.php index 93de4027..987d9726 100644 --- a/tests/TestDonationContextFactory.php +++ b/tests/TestDonationContextFactory.php @@ -4,14 +4,11 @@ namespace WMDE\Fundraising\DonationContext\Tests; -use Doctrine\Common\EventManager; -use Doctrine\Common\EventSubscriber; use Doctrine\DBAL\Connection; use Doctrine\DBAL\DriverManager; use Doctrine\ORM\EntityManager; use Doctrine\ORM\ORMSetup; use WMDE\Fundraising\DonationContext\DonationContextFactory; -use WMDE\Fundraising\DonationContext\Tests\Fixtures\FixedTokenGenerator; use WMDE\Fundraising\PaymentContext\PaymentContextFactory; /** @@ -36,7 +33,6 @@ public function __construct( array $config ) { $this->contextFactory = new DonationContextFactory( $config, ); - $this->contextFactory->setTokenGenerator( new FixedTokenGenerator() ); $this->entityManager = null; $this->connection = null; } @@ -51,42 +47,24 @@ public function getConnection(): Connection { public function getEntityManager(): EntityManager { if ( $this->entityManager === null ) { - $this->entityManager = $this->newEntityManager( $this->contextFactory->newEventSubscribers() ); + $this->entityManager = $this->newEntityManager(); } return $this->entityManager; } /** - * @param array $eventSubscribers - * * @return EntityManager * @throws \Doctrine\ORM\Exception\ORMException */ - private function newEntityManager( array $eventSubscribers = [] ): EntityManager { + private function newEntityManager(): EntityManager { $conn = $this->getConnection(); $paymentContext = new PaymentContextFactory(); $paymentContext->registerCustomTypes( $conn ); $paths = array_merge( $this->contextFactory->getDoctrineMappingPaths(), $paymentContext->getDoctrineMappingPaths() ); - $entityManager = EntityManager::create( + return new EntityManager( $conn, ORMSetup::createXMLMetadataConfiguration( $paths ) ); - - $this->setupEventSubscribers( $entityManager->getEventManager(), $eventSubscribers ); - - return $entityManager; - } - - /** - * @param EventManager $eventManager - * @param array $eventSubscribers - * - * @return void - */ - private function setupEventSubscribers( EventManager $eventManager, array $eventSubscribers ): void { - foreach ( $eventSubscribers as $eventSubscriber ) { - $eventManager->addEventSubscriber( $eventSubscriber ); - } } public function newSchemaCreator(): SchemaCreator { diff --git a/tests/Unit/Infrastructure/HttpDonationConfirmationNotifierTest.php b/tests/Unit/Infrastructure/HttpDonationConfirmationNotifierTest.php index cc592331..fa6692fa 100644 --- a/tests/Unit/Infrastructure/HttpDonationConfirmationNotifierTest.php +++ b/tests/Unit/Infrastructure/HttpDonationConfirmationNotifierTest.php @@ -4,10 +4,9 @@ use PHPUnit\Framework\TestCase; use Symfony\Contracts\HttpClient\HttpClientInterface; -use WMDE\Fundraising\DonationContext\Authorization\DonationTokens; use WMDE\Fundraising\DonationContext\Infrastructure\HttpDonationNotifier; +use WMDE\Fundraising\DonationContext\Infrastructure\HttpDonationNotifierUrlAuthorizer; use WMDE\Fundraising\DonationContext\Tests\Data\ValidDonation; -use WMDE\Fundraising\DonationContext\Tests\Fixtures\FixedDonationTokenFetcher; /** * @covers \WMDE\Fundraising\DonationContext\Infrastructure\HttpDonationNotifier @@ -16,9 +15,9 @@ class HttpDonationConfirmationNotifierTest extends TestCase { public function testSendConfirmationFor(): void { $donation = ValidDonation::newBookedAnonymousPayPalDonationUpdate( 1 ); - $testToken = 'blabla'; + $urlAuthorizer = $this->createStub( HttpDonationNotifierUrlAuthorizer::class ); + $urlAuthorizer->method( 'addAuthorizationParameters' )->willReturnArgument( 1 ); $httpClient = $this->createMock( HttpClientInterface::class ); - $fetcher = new FixedDonationTokenFetcher( new DonationTokens( 'some access token', $testToken ) ); $endpointUrl = 'https://somefancyendpoint.xyz/'; $httpClient->expects( $this->once() )->method( 'request' )->with( @@ -26,11 +25,10 @@ public function testSendConfirmationFor(): void { $endpointUrl, [ 'query' => [ 'donation_id' => $donation->getId(), - 'update_token' => $testToken ] ] ); - $notifier = new HttpDonationNotifier( $fetcher, $httpClient, $endpointUrl ); + $notifier = new HttpDonationNotifier( $urlAuthorizer, $httpClient, $endpointUrl ); $notifier->sendConfirmationFor( $donation ); } } diff --git a/tests/Unit/UseCases/CreditCardPaymentNotification/CreditCardNotificationUseCaseTest.php b/tests/Unit/UseCases/CreditCardPaymentNotification/CreditCardNotificationUseCaseTest.php index 960bb766..ea5c83a4 100644 --- a/tests/Unit/UseCases/CreditCardPaymentNotification/CreditCardNotificationUseCaseTest.php +++ b/tests/Unit/UseCases/CreditCardPaymentNotification/CreditCardNotificationUseCaseTest.php @@ -5,9 +5,9 @@ namespace WMDE\Fundraising\DonationContext\Tests\Unit\UseCases\CreditCardPaymentNotification; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationIdRepository; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Infrastructure\DonationEventLogger; use WMDE\Fundraising\DonationContext\Services\PaymentBookingService; use WMDE\Fundraising\DonationContext\Tests\Data\ValidCreditCardNotificationRequest; diff --git a/tests/Unit/UseCases/HandlePayPalPaymentNotification/HandlePayPalPaymentCompletionNotificationUseCaseTest.php b/tests/Unit/UseCases/HandlePayPalPaymentNotification/HandlePayPalPaymentCompletionNotificationUseCaseTest.php index d957710f..afbf7623 100644 --- a/tests/Unit/UseCases/HandlePayPalPaymentNotification/HandlePayPalPaymentCompletionNotificationUseCaseTest.php +++ b/tests/Unit/UseCases/HandlePayPalPaymentNotification/HandlePayPalPaymentCompletionNotificationUseCaseTest.php @@ -6,9 +6,9 @@ use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; -use WMDE\Fundraising\DonationContext\Authorization\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationIdRepository; use WMDE\Fundraising\DonationContext\Domain\Repositories\DonationRepository; +use WMDE\Fundraising\DonationContext\Infrastructure\DonationAuthorizationChecker; use WMDE\Fundraising\DonationContext\Infrastructure\DonationEventLogger; use WMDE\Fundraising\DonationContext\Services\PaymentBookingService; use WMDE\Fundraising\DonationContext\Tests\Data\ValidDonation; From 9aa9023e2e845de00b620a682dc92fd94d5f6cfc Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Mon, 9 Oct 2023 18:12:47 +0200 Subject: [PATCH 04/10] Use PaymentParameters Replace PaymentCreationRequest with PaymentParameters. The actual request to the CreatePaymentUseCase was formerly known as DomainSpecificPaymentCreationRequest and is now PaymentCreationRequest. --- .../AddDonation/AddDonationRequest.php | 51 +++++----- .../AddDonation/AddDonationUseCase.php | 14 +-- .../AddDonation/AddDonationValidator.php | 2 +- .../AddDonation/CreatePaymentService.php | 4 +- .../AddDonation/CreatePaymentWithUseCase.php | 4 +- .../Moderation/ModerationService.php | 10 +- .../AddDonation/PaymentParameterBuilder.php | 94 +++++++++++++++++++ .../AddDonation/PaymentRequestBuilder.php | 94 ------------------- tests/Data/ValidAddDonationRequest.php | 8 +- tests/Fixtures/CreatePaymentServiceSpy.php | 8 +- .../Fixtures/SucceedingPaymentServiceStub.php | 4 +- .../AddDonation/AddDonationUseCaseTest.php | 10 +- .../AddDonation/AddDonationValidatorTest.php | 6 +- .../AddDonation/ModerationServiceTest.php | 4 +- .../AddDonation/AddDonationRequestTest.php | 22 ++--- 15 files changed, 166 insertions(+), 169 deletions(-) create mode 100644 src/UseCases/AddDonation/PaymentParameterBuilder.php delete mode 100644 src/UseCases/AddDonation/PaymentRequestBuilder.php diff --git a/src/UseCases/AddDonation/AddDonationRequest.php b/src/UseCases/AddDonation/AddDonationRequest.php index cb2599a1..4af55926 100644 --- a/src/UseCases/AddDonation/AddDonationRequest.php +++ b/src/UseCases/AddDonation/AddDonationRequest.php @@ -6,11 +6,8 @@ use WMDE\Euro\Euro; use WMDE\Fundraising\DonationContext\Domain\Model\DonorType; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentParameters; -/** - * @license GPL-2.0-or-later - */ class AddDonationRequest { private DonorType $donorType; @@ -27,9 +24,9 @@ class AddDonationRequest { private bool $optsInToNewsletter = false; - private PaymentRequestBuilder $paymentCreationRequestBuilder; + private PaymentParameterBuilder $paymentParameterBuilder; - private PaymentCreationRequest $paymentCreationRequest; + private PaymentParameters $paymentParameters; private string $tracking = ''; private int $totalImpressionCount = 0; @@ -61,7 +58,7 @@ class AddDonationRequest { */ public function __construct() { $this->donorType = DonorType::ANONYMOUS(); - $this->paymentCreationRequestBuilder = new PaymentRequestBuilder(); + $this->paymentParameterBuilder = new PaymentParameterBuilder(); } /** @@ -75,57 +72,57 @@ public function setOptIn( string $optIn ): void { /** * @return Euro - * @deprecated Use {@see $paymentCreationRequest} + * @deprecated Use {@see $paymentParameters} */ public function getAmount(): Euro { - return Euro::newFromCents( $this->paymentCreationRequest->amountInEuroCents ); + return Euro::newFromCents( $this->paymentParameters->amountInEuroCents ); } /** * @param Euro $amount * @return void - * @deprecated Use {@see $paymentCreationRequest} + * @deprecated Use {@see $paymentParameters} */ public function setAmount( Euro $amount ): void { - $this->paymentCreationRequestBuilder->withAmount( $amount->getEuroCents() ); - $this->paymentCreationRequest = $this->paymentCreationRequestBuilder->getPaymentCreationRequest(); + $this->paymentParameterBuilder->withAmount( $amount->getEuroCents() ); + $this->paymentParameters = $this->paymentParameterBuilder->getPaymentParameters(); } /** * @param string $paymentType * @return void - * @deprecated Use {@see $paymentCreationRequest} + * @deprecated Use {@see $paymentParameters} */ public function setPaymentType( string $paymentType ): void { - $this->paymentCreationRequestBuilder->withPaymentType( $paymentType ); - $this->paymentCreationRequest = $this->paymentCreationRequestBuilder->getPaymentCreationRequest(); + $this->paymentParameterBuilder->withPaymentType( $paymentType ); + $this->paymentParameters = $this->paymentParameterBuilder->getPaymentParameters(); } /** * @return int - * @deprecated Use {@see $paymentCreationRequest} + * @deprecated Use {@see $paymentParameters} */ public function getInterval(): int { - return $this->getPaymentCreationRequest()->interval; + return $this->getPaymentParameters()->interval; } /** * @param string $iban - * @deprecated Use {@see $paymentCreationRequest} + * @deprecated Use {@see $paymentParameters} */ public function setIban( string $iban ): void { - $this->paymentCreationRequestBuilder->withBankData( $iban, $this->paymentCreationRequest->bic ); - $this->paymentCreationRequest = $this->paymentCreationRequestBuilder->getPaymentCreationRequest(); + $this->paymentParameterBuilder->withBankData( $iban, $this->paymentParameters->bic ); + $this->paymentParameters = $this->paymentParameterBuilder->getPaymentParameters(); } /** * @param string $bic * @return void - * @deprecated Use {@see $paymentCreationRequest} + * @deprecated Use {@see $paymentParameters} */ public function setBic( string $bic ): void { - $this->paymentCreationRequestBuilder->withBankData( $this->paymentCreationRequest->iban, $bic ); - $this->paymentCreationRequest = $this->paymentCreationRequestBuilder->getPaymentCreationRequest(); + $this->paymentParameterBuilder->withBankData( $this->paymentParameters->iban, $bic ); + $this->paymentParameters = $this->paymentParameterBuilder->getPaymentParameters(); } public function getTracking(): string { @@ -285,11 +282,11 @@ public function setOptsIntoNewsletter( bool $optIn ): void { $this->optsInToNewsletter = $optIn; } - public function getPaymentCreationRequest(): PaymentCreationRequest { - return $this->paymentCreationRequest ?? $this->paymentCreationRequestBuilder->build(); + public function getPaymentParameters(): PaymentParameters { + return $this->paymentParameters ?? $this->paymentParameterBuilder->build(); } - public function setPaymentCreationRequest( PaymentCreationRequest $paymentCreationRequest ): void { - $this->paymentCreationRequest = $paymentCreationRequest; + public function setPaymentParameters( PaymentParameters $paymentParameters ): void { + $this->paymentParameters = $paymentParameters; } } diff --git a/src/UseCases/AddDonation/AddDonationUseCase.php b/src/UseCases/AddDonation/AddDonationUseCase.php index 17ea66a5..5e6b4c3f 100644 --- a/src/UseCases/AddDonation/AddDonationUseCase.php +++ b/src/UseCases/AddDonation/AddDonationUseCase.php @@ -22,8 +22,8 @@ use WMDE\Fundraising\DonationContext\UseCases\AddDonation\Moderation\ModerationService; use WMDE\Fundraising\DonationContext\UseCases\DonationNotifier; use WMDE\Fundraising\PaymentContext\Domain\UrlGenerator\DomainSpecificContext; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\DomainSpecificPaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\FailureResponse; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; use WMDE\FunValidators\ConstraintViolation; /** @@ -57,7 +57,7 @@ public function addDonation( AddDonationRequest $donationRequest ): AddDonationR $paymentResult = $this->paymentService->createPayment( $this->getPaymentRequestForDonor( $donationRequest, $donationId ) ); if ( $paymentResult instanceof FailureResponse ) { return AddDonationResponse::newFailureResponse( [ - new ConstraintViolation( $donationRequest->getPaymentCreationRequest(), $paymentResult->errorMessage, 'payment' ) + new ConstraintViolation( $donationRequest->getPaymentParameters(), $paymentResult->errorMessage, 'payment' ) ] ); } $donation = $this->newDonationFromRequest( $donationRequest, $donationId, $paymentResult->paymentId ); @@ -160,15 +160,15 @@ private function sendDonationConfirmationEmail( Donation $donation, bool $paymen * We need to add donor-type specific properties (bank transfer code and validation) * to the original request. */ - private function getPaymentRequestForDonor( AddDonationRequest $request, int $donationId ): DomainSpecificPaymentCreationRequest { - $paymentRequest = $request->getPaymentCreationRequest(); + private function getPaymentRequestForDonor( AddDonationRequest $request, int $donationId ): PaymentCreationRequest { + $paymentRequest = $request->getPaymentParameters(); $paymentReferenceCodePrefix = self::PREFIX_BANK_TRANSACTION_KNOWN_DONOR; if ( $request->donorIsAnonymous() ) { $paymentReferenceCodePrefix = self::PREFIX_BANK_TRANSACTION_ANONYMOUS_DONOR; } - $paymentRequest = PaymentRequestBuilder::fromExistingRequest( $paymentRequest ) + $paymentRequest = PaymentParameterBuilder::fromExistingParameters( $paymentRequest ) ->withPaymentReferenceCodePrefix( $paymentReferenceCodePrefix ) - ->getPaymentCreationRequest(); + ->getPaymentParameters(); $urlAuthenticator = $this->donationAuthorizer->authorizeDonationAccess( $donationId ); $context = new DomainSpecificContext( @@ -178,7 +178,7 @@ private function getPaymentRequestForDonor( AddDonationRequest $request, int $do $request->getDonorFirstName(), $request->getDonorLastName() ); - return DomainSpecificPaymentCreationRequest::newFromBaseRequest( + return PaymentCreationRequest::newFromParameters( $paymentRequest, $this->paymentService->createPaymentValidator(), $context, diff --git a/src/UseCases/AddDonation/AddDonationValidator.php b/src/UseCases/AddDonation/AddDonationValidator.php index bdc32da3..0617d1d0 100644 --- a/src/UseCases/AddDonation/AddDonationValidator.php +++ b/src/UseCases/AddDonation/AddDonationValidator.php @@ -128,7 +128,7 @@ private function validateEmail(): ValidationResult { private function validateDonorAndPaymentTypeCombination(): void { $donorType = $this->request->getDonorType(); - $paymentType = $this->request->getPaymentCreationRequest()->paymentType; + $paymentType = $this->request->getPaymentParameters()->paymentType; if ( $donorType->is( DonorType::ANONYMOUS() ) && $paymentType === PaymentType::DirectDebit->value ) { $this->violations[] = new ConstraintViolation( $paymentType, diff --git a/src/UseCases/AddDonation/CreatePaymentService.php b/src/UseCases/AddDonation/CreatePaymentService.php index 41880513..63df6f8d 100644 --- a/src/UseCases/AddDonation/CreatePaymentService.php +++ b/src/UseCases/AddDonation/CreatePaymentService.php @@ -3,12 +3,12 @@ namespace WMDE\Fundraising\DonationContext\UseCases\AddDonation; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\DomainSpecificPaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\FailureResponse as PaymentCreationFailed; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\SuccessResponse as PaymentCreationSucceeded; interface CreatePaymentService { - public function createPayment( DomainSpecificPaymentCreationRequest $request ): PaymentCreationFailed|PaymentCreationSucceeded; + public function createPayment( PaymentCreationRequest $request ): PaymentCreationFailed|PaymentCreationSucceeded; public function createPaymentValidator(): DonationPaymentValidator; } diff --git a/src/UseCases/AddDonation/CreatePaymentWithUseCase.php b/src/UseCases/AddDonation/CreatePaymentWithUseCase.php index c973d56e..628f2882 100644 --- a/src/UseCases/AddDonation/CreatePaymentWithUseCase.php +++ b/src/UseCases/AddDonation/CreatePaymentWithUseCase.php @@ -5,8 +5,8 @@ use WMDE\Fundraising\PaymentContext\Domain\PaymentType; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\CreatePaymentUseCase; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\DomainSpecificPaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\FailureResponse as PaymentCreationFailed; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\SuccessResponse as PaymentCreationSucceeded; class CreatePaymentWithUseCase implements CreatePaymentService { @@ -21,7 +21,7 @@ public function __construct( ) { } - public function createPayment( DomainSpecificPaymentCreationRequest $request ): PaymentCreationFailed|PaymentCreationSucceeded { + public function createPayment( PaymentCreationRequest $request ): PaymentCreationFailed|PaymentCreationSucceeded { return $this->createPaymentUseCase->createPayment( $request ); } diff --git a/src/UseCases/AddDonation/Moderation/ModerationService.php b/src/UseCases/AddDonation/Moderation/ModerationService.php index d74f91d7..3130897b 100644 --- a/src/UseCases/AddDonation/Moderation/ModerationService.php +++ b/src/UseCases/AddDonation/Moderation/ModerationService.php @@ -9,7 +9,7 @@ use WMDE\Fundraising\DonationContext\Domain\Model\ModerationReason; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\AddDonationRequest; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\AddDonationValidationResult as Result; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentParameters; use WMDE\FunValidators\Validators\AmountPolicyValidator; use WMDE\FunValidators\Validators\TextPolicyValidator; @@ -68,12 +68,12 @@ public function __construct( AmountPolicyValidator $amountPolicyValidator, TextP */ public function moderateDonationRequest( AddDonationRequest $request ): ModerationResult { $this->result = new ModerationResult(); - $paymentCreationRequest = $request->getPaymentCreationRequest(); - if ( $this->paymentTypeBypassesModeration( $paymentCreationRequest->paymentType ) ) { + $paymentParameters = $request->getPaymentParameters(); + if ( $this->paymentTypeBypassesModeration( $paymentParameters->paymentType ) ) { return $this->result; } - $this->getAmountViolations( $paymentCreationRequest ); + $this->getAmountViolations( $paymentParameters ); $this->getBadWordViolations( $request ); return $this->result; @@ -141,7 +141,7 @@ private function getPolicyViolationsForField( string $fieldContent, string $fiel $this->result->addModerationReason( new ModerationReason( ModerationIdentifier::ADDRESS_CONTENT_VIOLATION, $fieldName ) ); } - private function getAmountViolations( PaymentCreationRequest $request ): void { + private function getAmountViolations( PaymentParameters $request ): void { $amountViolations = $this->amountPolicyValidator->validate( Euro::newFromCents( $request->amountInEuroCents )->getEuroFloat(), $request->interval diff --git a/src/UseCases/AddDonation/PaymentParameterBuilder.php b/src/UseCases/AddDonation/PaymentParameterBuilder.php new file mode 100644 index 00000000..75caf003 --- /dev/null +++ b/src/UseCases/AddDonation/PaymentParameterBuilder.php @@ -0,0 +1,94 @@ +paymentParameters = new PaymentParameters( + 0, + PaymentInterval::OneTime->value, + '', + transferCodePrefix: '' + ); + } + + public static function fromExistingParameters( PaymentParameters $request ): self { + $builder = new self(); + $builder->paymentParameters = $request; + return $builder; + } + + public function getPaymentParameters(): PaymentParameters { + return $this->paymentParameters; + } + + public function build(): PaymentParameters { + return $this->paymentParameters; + } + + public function withAmount( int $amount ): self { + $this->paymentParameters = new PaymentParameters( + $amount, + $this->paymentParameters->interval, + $this->paymentParameters->paymentType, + $this->paymentParameters->iban, + $this->paymentParameters->bic, + $this->paymentParameters->transferCodePrefix, + ); + return $this; + } + + public function withInterval( int $interval ): self { + $this->paymentParameters = new PaymentParameters( + $this->paymentParameters->amountInEuroCents, + $interval, + $this->paymentParameters->paymentType, + $this->paymentParameters->iban, + $this->paymentParameters->bic, + $this->paymentParameters->transferCodePrefix, + ); + return $this; + } + + public function withPaymentType( string $paymentType ): self { + $this->paymentParameters = new PaymentParameters( + $this->paymentParameters->amountInEuroCents, + $this->paymentParameters->interval, + $paymentType, + $this->paymentParameters->iban, + $this->paymentParameters->bic, + $this->paymentParameters->transferCodePrefix + ); + return $this; + } + + public function withBankData( string $iban, string $bic ): self { + $this->paymentParameters = new PaymentParameters( + $this->paymentParameters->amountInEuroCents, + $this->paymentParameters->interval, + $this->paymentParameters->paymentType, + $iban, + $bic, + $this->paymentParameters->transferCodePrefix, + ); + return $this; + } + + public function withPaymentReferenceCodePrefix( string $transferCodePrefix ): self { + $this->paymentParameters = new PaymentParameters( + $this->paymentParameters->amountInEuroCents, + $this->paymentParameters->interval, + $this->paymentParameters->paymentType, + $this->paymentParameters->iban, + $this->paymentParameters->bic, + $transferCodePrefix + ); + return $this; + } +} diff --git a/src/UseCases/AddDonation/PaymentRequestBuilder.php b/src/UseCases/AddDonation/PaymentRequestBuilder.php deleted file mode 100644 index 011cd8cc..00000000 --- a/src/UseCases/AddDonation/PaymentRequestBuilder.php +++ /dev/null @@ -1,94 +0,0 @@ -paymentCreationRequest = new PaymentCreationRequest( - 0, - PaymentInterval::OneTime->value, - '', - transferCodePrefix: '' - ); - } - - public static function fromExistingRequest( PaymentCreationRequest $request ): self { - $builder = new self(); - $builder->paymentCreationRequest = $request; - return $builder; - } - - public function getPaymentCreationRequest(): PaymentCreationRequest { - return $this->paymentCreationRequest; - } - - public function build(): PaymentCreationRequest { - return $this->paymentCreationRequest; - } - - public function withAmount( int $amount ): self { - $this->paymentCreationRequest = new PaymentCreationRequest( - $amount, - $this->paymentCreationRequest->interval, - $this->paymentCreationRequest->paymentType, - $this->paymentCreationRequest->iban, - $this->paymentCreationRequest->bic, - $this->paymentCreationRequest->transferCodePrefix, - ); - return $this; - } - - public function withInterval( int $interval ): self { - $this->paymentCreationRequest = new PaymentCreationRequest( - $this->paymentCreationRequest->amountInEuroCents, - $interval, - $this->paymentCreationRequest->paymentType, - $this->paymentCreationRequest->iban, - $this->paymentCreationRequest->bic, - $this->paymentCreationRequest->transferCodePrefix, - ); - return $this; - } - - public function withPaymentType( string $paymentType ): self { - $this->paymentCreationRequest = new PaymentCreationRequest( - $this->paymentCreationRequest->amountInEuroCents, - $this->paymentCreationRequest->interval, - $paymentType, - $this->paymentCreationRequest->iban, - $this->paymentCreationRequest->bic, - $this->paymentCreationRequest->transferCodePrefix - ); - return $this; - } - - public function withBankData( string $iban, string $bic ): self { - $this->paymentCreationRequest = new PaymentCreationRequest( - $this->paymentCreationRequest->amountInEuroCents, - $this->paymentCreationRequest->interval, - $this->paymentCreationRequest->paymentType, - $iban, - $bic, - $this->paymentCreationRequest->transferCodePrefix, - ); - return $this; - } - - public function withPaymentReferenceCodePrefix( string $transferCodePrefix ): self { - $this->paymentCreationRequest = new PaymentCreationRequest( - $this->paymentCreationRequest->amountInEuroCents, - $this->paymentCreationRequest->interval, - $this->paymentCreationRequest->paymentType, - $this->paymentCreationRequest->iban, - $this->paymentCreationRequest->bic, - $transferCodePrefix - ); - return $this; - } -} diff --git a/tests/Data/ValidAddDonationRequest.php b/tests/Data/ValidAddDonationRequest.php index 81e828d3..414cb6c4 100644 --- a/tests/Data/ValidAddDonationRequest.php +++ b/tests/Data/ValidAddDonationRequest.php @@ -7,7 +7,7 @@ use WMDE\Fundraising\DonationContext\Domain\Model\DonorType; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\AddDonationRequest; use WMDE\Fundraising\PaymentContext\Domain\Model\PaymentInterval; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentParameters; /** * A Valid AddDonationRequest with a private person donor, donating @@ -17,7 +17,7 @@ class ValidAddDonationRequest { public static function getRequest(): AddDonationRequest { $request = new AddDonationRequest(); - $request->setPaymentCreationRequest( self::newPaymentCreationRequest() ); + $request->setPaymentParameters( self::newPaymentParameters() ); $request->setOptsIntoNewsletter( ValidDonation::OPTS_INTO_NEWSLETTER ); $request->setDonorType( DonorType::PERSON() ); $request->setDonorSalutation( ValidDonation::DONOR_SALUTATION ); @@ -34,8 +34,8 @@ public static function getRequest(): AddDonationRequest { return $request; } - public static function newPaymentCreationRequest(): PaymentCreationRequest { - return new PaymentCreationRequest( + public static function newPaymentParameters(): PaymentParameters { + return new PaymentParameters( 500, PaymentInterval::OneTime->value, 'BEZ', diff --git a/tests/Fixtures/CreatePaymentServiceSpy.php b/tests/Fixtures/CreatePaymentServiceSpy.php index bcac561c..f55ba844 100644 --- a/tests/Fixtures/CreatePaymentServiceSpy.php +++ b/tests/Fixtures/CreatePaymentServiceSpy.php @@ -5,16 +5,16 @@ use WMDE\Fundraising\DonationContext\UseCases\AddDonation\DonationPaymentValidator; use WMDE\Fundraising\PaymentContext\Domain\PaymentType; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\DomainSpecificPaymentCreationRequest; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\SuccessResponse as PaymentCreationSucceeded; class CreatePaymentServiceSpy extends SucceedingPaymentServiceStub { /** - * @var DomainSpecificPaymentCreationRequest[] + * @var PaymentCreationRequest[] */ private array $requests = []; - public function createPayment( DomainSpecificPaymentCreationRequest $request ): PaymentCreationSucceeded { + public function createPayment( PaymentCreationRequest $request ): PaymentCreationSucceeded { $this->requests[] = $request; return parent::createPayment( $request ); } @@ -23,7 +23,7 @@ public function createPaymentValidator(): DonationPaymentValidator { return new DonationPaymentValidator( PaymentType::cases() ); } - public function getLastRequest(): DomainSpecificPaymentCreationRequest { + public function getLastRequest(): PaymentCreationRequest { return $this->requests[0]; } diff --git a/tests/Fixtures/SucceedingPaymentServiceStub.php b/tests/Fixtures/SucceedingPaymentServiceStub.php index 86188770..fbfab645 100644 --- a/tests/Fixtures/SucceedingPaymentServiceStub.php +++ b/tests/Fixtures/SucceedingPaymentServiceStub.php @@ -7,7 +7,7 @@ use WMDE\Fundraising\DonationContext\UseCases\AddDonation\CreatePaymentService; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\DonationPaymentValidator; use WMDE\Fundraising\PaymentContext\Domain\PaymentType; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\DomainSpecificPaymentCreationRequest; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\SuccessResponse as PaymentCreationSucceeded; class SucceedingPaymentServiceStub implements CreatePaymentService { @@ -22,7 +22,7 @@ public function __construct( ?PaymentCreationSucceeded $successResponse = null ) ); } - public function createPayment( DomainSpecificPaymentCreationRequest $request ): PaymentCreationSucceeded { + public function createPayment( PaymentCreationRequest $request ): PaymentCreationSucceeded { return $this->successResponse; } diff --git a/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php b/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php index c6950368..eb9b0f30 100644 --- a/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php +++ b/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php @@ -32,7 +32,7 @@ use WMDE\Fundraising\PaymentContext\Domain\Model\PaymentInterval; use WMDE\Fundraising\PaymentContext\Services\URLAuthenticator; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\FailureResponse as PaymentCreationFailed; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentParameters; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\SuccessResponse as PaymentCreationSucceeded; use WMDE\FunValidators\ConstraintViolation; @@ -95,7 +95,7 @@ public function testWhenValidationFails_responseObjectContainsViolations(): void public function testWhenPaymentCreationFails_responseObjectContainsViolations(): void { $request = $this->newMinimumDonationRequest(); - $expectedViolation = new ConstraintViolation( $request->getPaymentCreationRequest(), 'payment_not_supported', 'payment' ); + $expectedViolation = new ConstraintViolation( $request->getPaymentParameters(), 'payment_not_supported', 'payment' ); $useCase = $this->makeUseCase( paymentService: $this->makeFailingPaymentService( 'payment_not_supported' ) ); $result = $useCase->addDonation( $request ); @@ -106,7 +106,7 @@ public function testWhenPaymentCreationFails_responseObjectContainsViolations(): private function newMinimumDonationRequest(): AddDonationRequest { $donationRequest = new AddDonationRequest(); - $donationRequest->setPaymentCreationRequest( new PaymentCreationRequest( + $donationRequest->setPaymentParameters( new PaymentParameters( 100, PaymentInterval::OneTime->value, 'UEB' @@ -117,7 +117,7 @@ private function newMinimumDonationRequest(): AddDonationRequest { private function newInvalidDonationRequest(): AddDonationRequest { $donationRequest = new AddDonationRequest(); - $donationRequest->setPaymentCreationRequest( new PaymentCreationRequest( + $donationRequest->setPaymentParameters( new PaymentParameters( 100, PaymentInterval::OneTime->value, 'BEZ' @@ -221,7 +221,7 @@ private function newValidCompanyDonationRequest(): AddDonationRequest { return $request; } - public function testUrlAuthenticatorIsPassedToPaymentCreationRequest(): void { + public function testUrlAuthenticatorIsPassedToPaymentParameters(): void { $urlAuthenticator = $this->makeUrlAuthenticatorStub(); $donationAuthorizer = $this->makeDonationAuthorizerStub( $urlAuthenticator ); $paymentService = new CreatePaymentServiceSpy(); diff --git a/tests/Integration/UseCases/AddDonation/AddDonationValidatorTest.php b/tests/Integration/UseCases/AddDonation/AddDonationValidatorTest.php index f6fb0bcc..653fb975 100644 --- a/tests/Integration/UseCases/AddDonation/AddDonationValidatorTest.php +++ b/tests/Integration/UseCases/AddDonation/AddDonationValidatorTest.php @@ -10,7 +10,7 @@ use WMDE\Fundraising\DonationContext\Tests\Data\ValidatorPatterns; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\AddDonationValidationResult; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\AddDonationValidator; -use WMDE\Fundraising\DonationContext\UseCases\AddDonation\PaymentRequestBuilder; +use WMDE\Fundraising\DonationContext\UseCases\AddDonation\PaymentParameterBuilder; use WMDE\Fundraising\PaymentContext\Domain\PaymentType; use WMDE\FunValidators\ConstraintViolation; use WMDE\FunValidators\SucceedingDomainNameValidator; @@ -48,8 +48,8 @@ public function testGivenAnonymousDonorAndEmptyAddressFields_validatorReturnsNoV $request->setDonorCity( '' ); $request->setDonorCountryCode( '' ); $request->setDonorEmailAddress( '' ); - $paymentRequest = $request->getPaymentCreationRequest(); - $request->setPaymentCreationRequest( PaymentRequestBuilder::fromExistingRequest( $paymentRequest ) + $paymentRequest = $request->getPaymentParameters(); + $request->setPaymentParameters( PaymentParameterBuilder::fromExistingParameters( $paymentRequest ) ->withPaymentType( PaymentType::BankTransfer->value ) ->build() ); diff --git a/tests/Integration/UseCases/AddDonation/ModerationServiceTest.php b/tests/Integration/UseCases/AddDonation/ModerationServiceTest.php index d2a43d77..cdb3f074 100644 --- a/tests/Integration/UseCases/AddDonation/ModerationServiceTest.php +++ b/tests/Integration/UseCases/AddDonation/ModerationServiceTest.php @@ -8,7 +8,7 @@ use WMDE\Fundraising\DonationContext\Domain\Model\DonorType; use WMDE\Fundraising\DonationContext\Tests\Data\ValidAddDonationRequest; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\Moderation\ModerationService; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentParameters; use WMDE\FunValidators\ConstraintViolation; use WMDE\FunValidators\ValidationResult; use WMDE\FunValidators\Validators\AmountPolicyValidator; @@ -55,7 +55,7 @@ public function testGivenExternalPayment_needsModerationReturnsFalse( string $pa $this->newFailingTextPolicyValidator() ); $request = ValidAddDonationRequest::getRequest(); - $request->setPaymentCreationRequest( new PaymentCreationRequest( + $request->setPaymentParameters( new PaymentParameters( 100, 0, $paymentType diff --git a/tests/Unit/UseCases/AddDonation/AddDonationRequestTest.php b/tests/Unit/UseCases/AddDonation/AddDonationRequestTest.php index f5b7b897..6f64ca3b 100644 --- a/tests/Unit/UseCases/AddDonation/AddDonationRequestTest.php +++ b/tests/Unit/UseCases/AddDonation/AddDonationRequestTest.php @@ -7,7 +7,7 @@ use WMDE\Fundraising\DonationContext\Domain\Model\DonorType; use WMDE\Fundraising\DonationContext\UseCases\AddDonation\AddDonationRequest; use WMDE\Fundraising\PaymentContext\Domain\Model\PaymentInterval; -use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentParameters; /** * @covers \WMDE\Fundraising\DonationContext\UseCases\AddDonation\AddDonationRequest @@ -61,26 +61,26 @@ public function testDonorFieldGettersAndSetters(): void { public function testPaymentRequestDefaultValues(): void { $request = new AddDonationRequest(); - $paymentCreationRequest = $request->getPaymentCreationRequest(); + $paymentParameters = $request->getPaymentParameters(); - $this->assertSame( 0, $paymentCreationRequest->amountInEuroCents ); - $this->assertSame( PaymentInterval::OneTime->value, $paymentCreationRequest->interval ); - $this->assertSame( '', $paymentCreationRequest->paymentType ); - $this->assertSame( '', $paymentCreationRequest->iban ); - $this->assertSame( '', $paymentCreationRequest->bic ); - $this->assertSame( '', $paymentCreationRequest->transferCodePrefix ); + $this->assertSame( 0, $paymentParameters->amountInEuroCents ); + $this->assertSame( PaymentInterval::OneTime->value, $paymentParameters->interval ); + $this->assertSame( '', $paymentParameters->paymentType ); + $this->assertSame( '', $paymentParameters->iban ); + $this->assertSame( '', $paymentParameters->bic ); + $this->assertSame( '', $paymentParameters->transferCodePrefix ); } public function testPaymentRequestGetterAndSetter(): void { - $paymentCreationRequest = new PaymentCreationRequest( + $paymentParameters = new PaymentParameters( 100, PaymentInterval::OneTime->value, 'BEZ' ); $request = new AddDonationRequest(); - $request->setPaymentCreationRequest( $paymentCreationRequest ); + $request->setPaymentParameters( $paymentParameters ); - $this->assertSame( $paymentCreationRequest, $request->getPaymentCreationRequest() ); + $this->assertSame( $paymentParameters, $request->getPaymentParameters() ); } public function testTrackingFields(): void { From 3c5e03fc1d2fd8be14ee1bdf61ff227affed9c14 Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Wed, 11 Oct 2023 12:09:34 +0200 Subject: [PATCH 05/10] Update branch in composer.json A payment branch was merged, we need to track the long-running branch again --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 027c1409..fa9dd98e 100644 --- a/composer.json +++ b/composer.json @@ -16,7 +16,7 @@ "wmde/euro": "~1.0", "wmde/freezable-value-object": "~2.0", "wmde/fun-validators": "~4.0", - "wmde/fundraising-payments": "dev-url-generation-refactor" + "wmde/fundraising-payments": "dev-payment-with-ppl-api" }, "repositories": [ { From 935dab20f1940139eddffd3d44e403b0a6459bda Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Wed, 11 Oct 2023 15:24:06 +0200 Subject: [PATCH 06/10] Refactor AddDonationUseCase for clarity Move call to URLAuthenticator into main method to signify its importance for the process. Split getPaymentRequestForDonor into two methods and fix the comment --- .../AddDonation/AddDonationUseCase.php | 46 +++++++++++-------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/src/UseCases/AddDonation/AddDonationUseCase.php b/src/UseCases/AddDonation/AddDonationUseCase.php index 5e6b4c3f..08d2928d 100644 --- a/src/UseCases/AddDonation/AddDonationUseCase.php +++ b/src/UseCases/AddDonation/AddDonationUseCase.php @@ -22,8 +22,10 @@ use WMDE\Fundraising\DonationContext\UseCases\AddDonation\Moderation\ModerationService; use WMDE\Fundraising\DonationContext\UseCases\DonationNotifier; use WMDE\Fundraising\PaymentContext\Domain\UrlGenerator\DomainSpecificContext; +use WMDE\Fundraising\PaymentContext\Services\URLAuthenticator; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\FailureResponse; use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentCreationRequest; +use WMDE\Fundraising\PaymentContext\UseCases\CreatePayment\PaymentParameters; use WMDE\FunValidators\ConstraintViolation; /** @@ -48,18 +50,20 @@ public function __construct( public function addDonation( AddDonationRequest $donationRequest ): AddDonationResponse { $validationResult = $this->donationValidator->validate( $donationRequest ); - if ( $validationResult->hasViolations() ) { return AddDonationResponse::newFailureResponse( $validationResult->getViolations() ); } $donationId = $this->idGenerator->getNewId(); - $paymentResult = $this->paymentService->createPayment( $this->getPaymentRequestForDonor( $donationRequest, $donationId ) ); + $urlAuthenticator = $this->donationAuthorizer->authorizeDonationAccess( $donationId ); + $paymentRequest = $this->getPaymentRequestForDonor( $donationRequest, $donationId, $urlAuthenticator ); + $paymentResult = $this->paymentService->createPayment( $paymentRequest ); if ( $paymentResult instanceof FailureResponse ) { return AddDonationResponse::newFailureResponse( [ new ConstraintViolation( $donationRequest->getPaymentParameters(), $paymentResult->errorMessage, 'payment' ) ] ); } + $donation = $this->newDonationFromRequest( $donationRequest, $donationId, $paymentResult->paymentId ); $moderationResult = $this->policyValidator->moderateDonationRequest( $donationRequest ); @@ -154,23 +158,8 @@ private function sendDonationConfirmationEmail( Donation $donation, bool $paymen } } - /** - * Modify PaymentCreationRequest from the AddDonationRequest - * - * We need to add donor-type specific properties (bank transfer code and validation) - * to the original request. - */ - private function getPaymentRequestForDonor( AddDonationRequest $request, int $donationId ): PaymentCreationRequest { - $paymentRequest = $request->getPaymentParameters(); - $paymentReferenceCodePrefix = self::PREFIX_BANK_TRANSACTION_KNOWN_DONOR; - if ( $request->donorIsAnonymous() ) { - $paymentReferenceCodePrefix = self::PREFIX_BANK_TRANSACTION_ANONYMOUS_DONOR; - } - $paymentRequest = PaymentParameterBuilder::fromExistingParameters( $paymentRequest ) - ->withPaymentReferenceCodePrefix( $paymentReferenceCodePrefix ) - ->getPaymentParameters(); - - $urlAuthenticator = $this->donationAuthorizer->authorizeDonationAccess( $donationId ); + private function getPaymentRequestForDonor( AddDonationRequest $request, int $donationId, + URLAuthenticator $urlAuthenticator ): PaymentCreationRequest { $context = new DomainSpecificContext( $donationId, null, @@ -179,13 +168,30 @@ private function getPaymentRequestForDonor( AddDonationRequest $request, int $do $request->getDonorLastName() ); return PaymentCreationRequest::newFromParameters( - $paymentRequest, + $this->getPaymentParametersForDonor( $request ), $this->paymentService->createPaymentValidator(), $context, $urlAuthenticator ); } + /** + * Modify PaymentParameters from the AddDonationRequest + * + * We need to add donor-type specific properties (bank transfer code) + * to the original request. + */ + private function getPaymentParametersForDonor( AddDonationRequest $request ): PaymentParameters { + $paymentParameters = $request->getPaymentParameters(); + $paymentReferenceCodePrefix = self::PREFIX_BANK_TRANSACTION_KNOWN_DONOR; + if ( $request->donorIsAnonymous() ) { + $paymentReferenceCodePrefix = self::PREFIX_BANK_TRANSACTION_ANONYMOUS_DONOR; + } + return PaymentParameterBuilder::fromExistingParameters( $paymentParameters ) + ->withPaymentReferenceCodePrefix( $paymentReferenceCodePrefix ) + ->getPaymentParameters(); + } + /** * We use the donation primary key as the InvoiceId because they're unique * But we prepend a letter to make sure they don't clash with memberships From d2413a9a873765b56b2f3de088d84d66c0752bda Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Wed, 11 Oct 2023 18:17:03 +0200 Subject: [PATCH 07/10] Remove deprecated authentication classes They are now implemented in the application, where they belong. https://phabricator.wikimedia.org/T341795 --- phpstan-baseline.neon | 4 - src/Authorization/DonationTokenFetcher.php | 20 -- .../DonationTokenFetchingException.php | 16 -- src/Authorization/DonationTokens.php | 36 --- src/Authorization/RandomTokenGenerator.php | 26 --- src/Authorization/TokenGenerator.php | 16 -- .../DoctrineDonationAuthorizationChecker.php | 97 -------- .../DoctrineDonationPrePersistSubscriber.php | 61 ----- .../DoctrineDonationTokenFetcher.php | 51 ---- tests/Fixtures/FixedDonationTokenFetcher.php | 30 --- tests/Fixtures/FixedTokenGenerator.php | 25 -- ...ctrineDonationAuthorizationCheckerTest.php | 217 ------------------ .../RandomTokenGeneratorTest.php | 47 ---- .../DoctrineDonationTokenFetcherTest.php | 79 ------- 14 files changed, 725 deletions(-) delete mode 100644 src/Authorization/DonationTokenFetcher.php delete mode 100644 src/Authorization/DonationTokenFetchingException.php delete mode 100644 src/Authorization/DonationTokens.php delete mode 100644 src/Authorization/RandomTokenGenerator.php delete mode 100644 src/Authorization/TokenGenerator.php delete mode 100644 src/DataAccess/DoctrineDonationAuthorizationChecker.php delete mode 100644 src/DataAccess/DoctrineDonationPrePersistSubscriber.php delete mode 100644 src/DataAccess/DoctrineDonationTokenFetcher.php delete mode 100644 tests/Fixtures/FixedDonationTokenFetcher.php delete mode 100644 tests/Fixtures/FixedTokenGenerator.php delete mode 100644 tests/Integration/DataAccess/DoctrineDonationAuthorizationCheckerTest.php delete mode 100644 tests/Unit/Authorization/RandomTokenGeneratorTest.php delete mode 100644 tests/Unit/DataAccess/DoctrineDonationTokenFetcherTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index d8e52ff5..f3bfff0d 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,9 +1,5 @@ parameters: ignoreErrors: - - - message: "#^Method WMDE\\\\Fundraising\\\\DonationContext\\\\DataAccess\\\\DoctrineDonationPrePersistSubscriber\\:\\:prePersist\\(\\) has parameter \\$args with generic class Doctrine\\\\Persistence\\\\Event\\\\LifecycleEventArgs but does not specify its types\\: TObjectManager$#" - count: 1 - path: src/DataAccess/DoctrineDonationPrePersistSubscriber.php - message: "#^Method WMDE\\\\Fundraising\\\\DonationContext\\\\Domain\\\\Model\\\\DonorType\\:\\:__callStatic\\(\\) has parameter \\$arguments with no type specified\\.$#" diff --git a/src/Authorization/DonationTokenFetcher.php b/src/Authorization/DonationTokenFetcher.php deleted file mode 100644 index 9b9b237a..00000000 --- a/src/Authorization/DonationTokenFetcher.php +++ /dev/null @@ -1,20 +0,0 @@ -accessToken = $accessToken; - $this->updateToken = $updateToken; - } - - public function getAccessToken(): string { - return $this->accessToken; - } - - public function getUpdateToken(): string { - return $this->updateToken; - } - -} diff --git a/src/Authorization/RandomTokenGenerator.php b/src/Authorization/RandomTokenGenerator.php deleted file mode 100644 index 05ff8f18..00000000 --- a/src/Authorization/RandomTokenGenerator.php +++ /dev/null @@ -1,26 +0,0 @@ -tokenLength ) ) ); - } - - public function generateTokenExpiry(): \DateTime { - return ( new \DateTime() )->add( $this->validityTimeSpan ); - } - -} diff --git a/src/Authorization/TokenGenerator.php b/src/Authorization/TokenGenerator.php deleted file mode 100644 index d540d3c5..00000000 --- a/src/Authorization/TokenGenerator.php +++ /dev/null @@ -1,16 +0,0 @@ -entityManager = $entityManager; - $this->updateToken = $updateToken; - $this->accessToken = $accessToken; - } - - /** - * Check if donation exists, has matching token and token is not expired - * - * @param int $donationId - * - * @return bool - */ - public function userCanModifyDonation( int $donationId ): bool { - $donation = $this->getDonationById( $donationId ); - - return $donation !== null - && $this->updateTokenMatches( $donation ) - && $this->tokenHasNotExpired( $donation ); - } - - private function getDonationById( int $donationId ): ?Donation { - try { - return $this->entityManager->find( Donation::class, $donationId ); - } catch ( ORMException $e ) { - throw new GetDonationException( $e, sprintf( 'Could not get donation with id %d', $donationId ) ); - } - } - - private function updateTokenMatches( Donation $donation ): bool { - if ( $this->updateToken === '' ) { - return false; - } - return hash_equals( - (string)$donation->getDataObject()->getUpdateToken(), - $this->updateToken - ); - } - - private function tokenHasNotExpired( Donation $donation ): bool { - $expiryTime = $donation->getDataObject()->getUpdateTokenExpiry(); - - return $expiryTime !== null && strtotime( $expiryTime ) >= time(); - } - - /** - * Check if donation exists and has matching token - * - * @param int $donationId - * - * @return bool - */ - public function systemCanModifyDonation( int $donationId ): bool { - $donation = $this->getDonationById( $donationId ); - - return $donation !== null - && $this->updateTokenMatches( $donation ); - } - - public function canAccessDonation( int $donationId ): bool { - $donation = $this->getDonationById( $donationId ); - - return $donation !== null - && $this->accessTokenMatches( $donation ); - } - - private function accessTokenMatches( Donation $donation ): bool { - if ( $this->accessToken === '' ) { - return false; - } - return hash_equals( - (string)$donation->getDataObject()->getAccessToken(), - $this->accessToken - ); - } -} diff --git a/src/DataAccess/DoctrineDonationPrePersistSubscriber.php b/src/DataAccess/DoctrineDonationPrePersistSubscriber.php deleted file mode 100644 index 4f5a6b5a..00000000 --- a/src/DataAccess/DoctrineDonationPrePersistSubscriber.php +++ /dev/null @@ -1,61 +0,0 @@ -getObject(); - - if ( $entity instanceof Donation ) { - $entity->modifyDataObject( - function ( DonationData $data ): void { - if ( $this->isEmpty( $data->getAccessToken() ) ) { - $data->setAccessToken( $this->accessTokenGenerator->generateToken() ); - } - - if ( $this->isEmpty( $data->getUpdateToken() ) ) { - $data->setUpdateToken( $this->updateTokenGenerator->generateToken() ); - } - - if ( $this->isEmpty( $data->getUpdateTokenExpiry() ) ) { - $expiry = $this->updateTokenGenerator->generateTokenExpiry(); - $data->setUpdateTokenExpiry( $expiry->format( self::DATE_TIME_FORMAT ) ); - } - } - ); - } - } - - private function isEmpty( ?string $stringOrNull ): bool { - return $stringOrNull === null || $stringOrNull === ''; - } - -} diff --git a/src/DataAccess/DoctrineDonationTokenFetcher.php b/src/DataAccess/DoctrineDonationTokenFetcher.php deleted file mode 100644 index 2b970f59..00000000 --- a/src/DataAccess/DoctrineDonationTokenFetcher.php +++ /dev/null @@ -1,51 +0,0 @@ -entityManager = $entityManager; - } - - /** - * @param int $donationId - * - * @return DonationTokens - * @throws DonationTokenFetchingException - */ - public function getTokens( int $donationId ): DonationTokens { - $donation = $this->getDonationById( $donationId ); - - if ( $donation === null ) { - throw new DonationTokenFetchingException( sprintf( 'Could not find donation with ID "%d"', $donationId ) ); - } - - try { - return new DonationTokens( - (string)$donation->getDataObject()->getAccessToken(), - (string)$donation->getDataObject()->getUpdateToken() - ); - } catch ( \UnexpectedValueException $e ) { - throw new DonationTokenFetchingException( $e->getMessage(), $e ); - } - } - - private function getDonationById( int $donationId ): ?Donation { - return $this->entityManager->find( Donation::class, $donationId ); - } - -} diff --git a/tests/Fixtures/FixedDonationTokenFetcher.php b/tests/Fixtures/FixedDonationTokenFetcher.php deleted file mode 100644 index 3e008617..00000000 --- a/tests/Fixtures/FixedDonationTokenFetcher.php +++ /dev/null @@ -1,30 +0,0 @@ -tokens = $tokens; - } - - /** - * @param int $donationId - * - * @return DonationTokens - */ - public function getTokens( int $donationId ): DonationTokens { - return $this->tokens; - } - -} diff --git a/tests/Fixtures/FixedTokenGenerator.php b/tests/Fixtures/FixedTokenGenerator.php deleted file mode 100644 index 1e31dbb0..00000000 --- a/tests/Fixtures/FixedTokenGenerator.php +++ /dev/null @@ -1,25 +0,0 @@ -entityManager = TestEnvironment::newInstance()->getFactory()->getEntityManager(); - } - - private function newAuthorizationService( string $updateToken = '', string $accessToken = '' ): DonationAuthorizationChecker { - return new DoctrineDonationAuthorizationChecker( $this->entityManager, $updateToken, $accessToken ); - } - - public function testGivenNoDonation_authorizationFails(): void { - $authorizer = $this->newAuthorizationService( self::CORRECT_UPDATE_TOKEN ); - $this->assertFalse( $authorizer->userCanModifyDonation( self::MEANINGLESS_DONATION_ID ) ); - $this->assertFalse( $authorizer->canAccessDonation( self::MEANINGLESS_DONATION_ID ) ); - } - - /** - * @dataProvider updateTokenProvider - */ - public function testAuthorizerChecksUpdateTokenOfDonation( string $updateToken, bool $expectedResult ): void { - $this->givenDonationWithTokens(); - $authorizer = $this->newAuthorizationService( $updateToken ); - - $this->assertSame( $expectedResult, $authorizer->userCanModifyDonation( self::DONATION_ID ) ); - } - - /** - * @return iterable - */ - public static function updateTokenProvider(): iterable { - yield 'correct update token' => [ self::CORRECT_UPDATE_TOKEN, true ]; - yield 'incorrect update token' => [ self::WRONG__UPDATE_TOKEN, false ]; - } - - /** - * @dataProvider accessTokenProvider - */ - public function testAuthorizerChecksAccessTokenOfDonation( string $accessToken, bool $expectedResult ): void { - $donation = $this->givenDonationWithTokens(); - $authorizer = $this->newAuthorizationService( '', $accessToken ); - - $this->assertSame( $expectedResult, $authorizer->canAccessDonation( self::DONATION_ID ) ); - } - - /** - * @return iterable - */ - public static function accessTokenProvider(): iterable { - yield 'correct access token' => [ self::CORRECT_ACCESS_TOKEN, true ]; - yield 'incorrect update token' => [ self::WRONG_ACCESS_TOKEN, false ]; - } - - private function getExpiryTimeInTheFuture(): string { - return date( 'Y-m-d H:i:s', time() + 60 * 60 ); - } - - public function testGivenTokenAndLegacyDonation_updateAuthorizationFails(): void { - $donation = $this->givenLegacyDonation(); - $authorizer = $this->newAuthorizationService( self::MEANINGLESS_TOKEN ); - - $this->assertFalse( $authorizer->userCanModifyDonation( self::DONATION_ID ) ); - } - - public function testGivenTokenAndLegacyDonation_accessAuthorizationFails(): void { - $donation = $this->givenLegacyDonation(); - $authorizer = $this->newAuthorizationService( self::EMPTY_TOKEN, self::MEANINGLESS_TOKEN ); - - $this->assertFalse( $authorizer->canAccessDonation( self::DONATION_ID ) ); - } - - public function testGivenEmptyTokenAndLegacyDonation_updateAuthorizationFails(): void { - $donation = $this->givenLegacyDonation(); - $authorizer = $this->newAuthorizationService( self::EMPTY_TOKEN, self::EMPTY_TOKEN ); - - $this->assertFalse( $authorizer->userCanModifyDonation( self::DONATION_ID ) ); - } - - public function testGivenEmptyTokenAndLegacyDonation_accessAuthorizationFails(): void { - $donation = $this->givenLegacyDonation(); - $authorizer = $this->newAuthorizationService( self::EMPTY_TOKEN, self::EMPTY_TOKEN ); - - $this->assertFalse( $authorizer->canAccessDonation( self::DONATION_ID ) ); - } - - public function testWhenUpdateTokenIsExpiredUpdateCheckFailsForUser(): void { - $donation = $this->givenDonationWithExpiredUpdateToken(); - - $authorizer = $this->newAuthorizationService( self::CORRECT_UPDATE_TOKEN ); - $this->assertFalse( $authorizer->userCanModifyDonation( self::DONATION_ID ) ); - } - - public function testWhenUpdateTokenIsExpiredUpdateCheckSucceedsForSystem(): void { - $donation = $this->givenDonationWithExpiredUpdateToken(); - $authorizer = $this->newAuthorizationService( self::CORRECT_UPDATE_TOKEN ); - - $this->assertTrue( $authorizer->systemCanModifyDonation( self::DONATION_ID ) ); - } - - public function testGivenExceptionFromEntityManager_authorizerWrapsExceptionForUserModification(): void { - $authorizer = new DoctrineDonationAuthorizationChecker( - $this->getThrowingEntityManager(), - self::CORRECT_UPDATE_TOKEN, - self::CORRECT_ACCESS_TOKEN - ); - - $this->expectException( GetDonationException::class ); - - $authorizer->userCanModifyDonation( self::MEANINGLESS_DONATION_ID ); - } - - public function testGivenExceptionFromEntityManager_authorizerWrapsExceptionForSystemModification(): void { - $authorizer = new DoctrineDonationAuthorizationChecker( - $this->getThrowingEntityManager(), - self::CORRECT_UPDATE_TOKEN, - self::CORRECT_ACCESS_TOKEN - ); - - $this->expectException( GetDonationException::class ); - - $authorizer->systemCanModifyDonation( self::MEANINGLESS_DONATION_ID ); - } - - public function testGivenExceptionFromEntityManager_authorizerWrapsExceptionForAccessCheck(): void { - $authorizer = new DoctrineDonationAuthorizationChecker( - $this->getThrowingEntityManager(), - self::CORRECT_UPDATE_TOKEN, - self::CORRECT_ACCESS_TOKEN - ); - - $this->expectException( GetDonationException::class ); - - $authorizer->canAccessDonation( self::MEANINGLESS_DONATION_ID ); - } - - private function getThrowingEntityManager(): EntityManager { - $entityManager = $this->getMockBuilder( EntityManager::class ) - ->disableOriginalConstructor()->getMock(); - - $entityManager->method( $this->anything() ) - ->willThrowException( new ORMException() ); - - return $entityManager; - } - - private function givenDonationWithTokens(): Donation { - $donation = new Donation(); - $donation->setId( self::DONATION_ID ); - $donation->setPaymentId( self::DUMMY_PAYMENT_ID ); - $donationData = $donation->getDataObject(); - $donationData->setUpdateToken( self::CORRECT_UPDATE_TOKEN ); - $donationData->setUpdateTokenExpiry( $this->getExpiryTimeInTheFuture() ); - $donationData->setAccessToken( self::CORRECT_ACCESS_TOKEN ); - $donation->setDataObject( $donationData ); - $this->storeDonation( $donation ); - return $donation; - } - - private function givenDonationWithExpiredUpdateToken(): Donation { - $donation = new Donation(); - $donation->setId( self::DONATION_ID ); - $donation->setPaymentId( self::DUMMY_PAYMENT_ID ); - $donationData = $donation->getDataObject(); - $donationData->setUpdateToken( self::CORRECT_UPDATE_TOKEN ); - $donationData->setUpdateTokenExpiry( $this->getExpiryTimeInThePast() ); - $donation->setDataObject( $donationData ); - $this->storeDonation( $donation ); - return $donation; - } - - private function givenLegacyDonation(): Donation { - $donation = new Donation(); - $donation->setId( self::DONATION_ID ); - $donation->setPaymentId( self::DUMMY_PAYMENT_ID ); - $this->storeDonation( $donation ); - return $donation; - } - - private function storeDonation( Donation $donation ): void { - $this->entityManager->persist( $donation ); - $this->entityManager->flush(); - } - - private function getExpiryTimeInThePast(): string { - return date( 'Y-m-d H:i:s', time() - 1 ); - } -} diff --git a/tests/Unit/Authorization/RandomTokenGeneratorTest.php b/tests/Unit/Authorization/RandomTokenGeneratorTest.php deleted file mode 100644 index c2d9dc63..00000000 --- a/tests/Unit/Authorization/RandomTokenGeneratorTest.php +++ /dev/null @@ -1,47 +0,0 @@ - - */ -class RandomTokenGeneratorTest extends \PHPUnit\Framework\TestCase { - - public function testGenerateTokenReturnsHexString(): void { - $this->assertTrue( - ctype_xdigit( - ( new RandomTokenGenerator( 10, new \DateInterval( 'PT1H' ) ) )->generateToken() - ) - ); - } - - public function testGenerateTokenReturnsDifferentStringsOnSuccessiveCalls(): void { - $generator = new RandomTokenGenerator( 10, new \DateInterval( 'PT1H' ) ); - - $this->assertNotSame( $generator->generateToken(), $generator->generateToken() ); - } - - public function testGenerateTokenReturnsDifferentStringsForInitialCalls(): void { - $this->assertNotSame( - ( new RandomTokenGenerator( 10, new \DateInterval( 'PT1H' ) ) )->generateToken(), - ( new RandomTokenGenerator( 10, new \DateInterval( 'PT1H' ) ) )->generateToken() - ); - } - - public function testGenerateTokenExpiryAddsInterval(): void { - $generator = new RandomTokenGenerator( 10, new \DateInterval( 'PT1H' ) ); - - $this->assertGreaterThan( - time(), - $generator->generateTokenExpiry()->getTimestamp() - ); - } - -} diff --git a/tests/Unit/DataAccess/DoctrineDonationTokenFetcherTest.php b/tests/Unit/DataAccess/DoctrineDonationTokenFetcherTest.php deleted file mode 100644 index 617c1f5b..00000000 --- a/tests/Unit/DataAccess/DoctrineDonationTokenFetcherTest.php +++ /dev/null @@ -1,79 +0,0 @@ -entityManager = $this->createMock( EntityManager::class ); - } - - public function testGivenExistingDonation_AuthorizerReturnsTokenSet(): void { - $donation = new Donation(); - $donationData = $donation->getDataObject(); - $donationData->setUpdateToken( self::CORRECT_UPDATE_TOKEN ); - $donationData->setAccessToken( self::CORRECT_ACCESS_TOKEN ); - $donation->setDataObject( $donationData ); - $this->entityManager->method( 'find' )->willReturn( $donation ); - - $donAuthorizer = new DoctrineDonationTokenFetcher( $this->entityManager ); - $resultTokenSet = $donAuthorizer->getTokens( self::DONATION_ID ); - - $this->assertSame( self::CORRECT_ACCESS_TOKEN, $resultTokenSet->getAccessToken() ); - $this->assertSame( self::CORRECT_UPDATE_TOKEN, $resultTokenSet->getUpdateToken() ); - } - - public function testGivenDonationWithMissingAccessToken_AuthorizerThrowsException(): void { - $donation = new Donation(); - $donationData = $donation->getDataObject(); - $donationData->setUpdateToken( self::CORRECT_UPDATE_TOKEN ); - $donation->setDataObject( $donationData ); - $this->entityManager->method( 'find' )->willReturn( $donation ); - - $donAuthorizer = new DoctrineDonationTokenFetcher( $this->entityManager ); - - $this->expectException( DonationTokenFetchingException::class ); - $donAuthorizer->getTokens( self::MEANINGLESS_DONATION_ID ); - } - - public function testGivenDonationWithMissingUpdateToken_AuthorizerThrowsException(): void { - $donation = new Donation(); - $donationData = $donation->getDataObject(); - $donationData->setAccessToken( self::CORRECT_ACCESS_TOKEN ); - $donation->setDataObject( $donationData ); - $this->entityManager->method( 'find' )->willReturn( $donation ); - - $donAuthorizer = new DoctrineDonationTokenFetcher( $this->entityManager ); - - $this->expectException( DonationTokenFetchingException::class ); - $donAuthorizer->getTokens( self::MEANINGLESS_DONATION_ID ); - } - - public function testGivenMissingDonation_AuthorizerThrowsException(): void { - $this->entityManager->method( 'find' )->willReturn( null ); - - $donAuthorizer = new DoctrineDonationTokenFetcher( $this->entityManager ); - - $this->expectException( DonationTokenFetchingException::class ); - $donAuthorizer->getTokens( self::MEANINGLESS_DONATION_ID ); - } -} From 9daa31e51bba15122fd2fda11de4b001ad8563ba Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Wed, 11 Oct 2023 18:43:46 +0200 Subject: [PATCH 08/10] Remove config from DonationContextFactory We no longer need to pass token generator information to it, so we can remove it. https://phabricator.wikimedia.org/T341795 --- src/DonationContextFactory.php | 15 --------------- tests/TestDonationContextFactory.php | 4 +--- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/src/DonationContextFactory.php b/src/DonationContextFactory.php index eb564878..e61590ab 100644 --- a/src/DonationContextFactory.php +++ b/src/DonationContextFactory.php @@ -7,25 +7,10 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\Types\Type; -/** - * @license GPL-2.0-or-later - */ class DonationContextFactory { private const DOCTRINE_CLASS_MAPPING_DIRECTORY = __DIR__ . '/../config/DoctrineClassMapping'; - /** - * @var array{token-length:int,token-validity-timestamp:string} - */ - protected array $config; - - /** - * @param array{token-length:int,token-validity-timestamp:string} $config - */ - public function __construct( array $config ) { - $this->config = $config; - } - /** * @return string[] */ diff --git a/tests/TestDonationContextFactory.php b/tests/TestDonationContextFactory.php index 987d9726..77deac74 100644 --- a/tests/TestDonationContextFactory.php +++ b/tests/TestDonationContextFactory.php @@ -30,9 +30,7 @@ class TestDonationContextFactory { */ public function __construct( array $config ) { $this->config = $config; - $this->contextFactory = new DonationContextFactory( - $config, - ); + $this->contextFactory = new DonationContextFactory(); $this->entityManager = null; $this->connection = null; } From be8ddfd8a4e4a522c31138f789117b5b68a19d55 Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Tue, 17 Oct 2023 14:53:36 +0200 Subject: [PATCH 09/10] Improve AddDonationResponse This is an adaptation of the payment change from https://github.com/wmde/fundraising-payments/pull/143 (renaming externalPaymentCompletionUrl to paymentCompletionUrl) This change is also reflected in the AddDonationResponse, which renames paymentProviderRedirectUrl to paymentCompletionUrl While updating the response object I realized that it can be constructed and used in the wrong way, so I added checks (and tests) to prevent that. --- .../AddDonation/AddDonationResponse.php | 26 ++++++--- .../AddDonation/AddDonationUseCase.php | 2 +- .../AddDonation/AddDonationUseCaseTest.php | 4 +- .../AddDonation/AddDonationResponseTest.php | 57 +++++++++++++++++++ 4 files changed, 79 insertions(+), 10 deletions(-) create mode 100644 tests/Unit/UseCases/AddDonation/AddDonationResponseTest.php diff --git a/src/UseCases/AddDonation/AddDonationResponse.php b/src/UseCases/AddDonation/AddDonationResponse.php index 6d5b451e..182fb672 100644 --- a/src/UseCases/AddDonation/AddDonationResponse.php +++ b/src/UseCases/AddDonation/AddDonationResponse.php @@ -16,12 +16,12 @@ class AddDonationResponse { private ?Donation $donation = null; - private ?string $paymentProviderRedirectUrl = null; + private ?string $paymentCompletionUrl = null; - public static function newSuccessResponse( Donation $donation, ?string $paymentProviderRedirectUrl ): self { + public static function newSuccessResponse( Donation $donation, string $paymentCompletionUrl ): self { $response = new self(); $response->donation = $donation; - $response->paymentProviderRedirectUrl = $paymentProviderRedirectUrl; + $response->paymentCompletionUrl = $paymentCompletionUrl; return $response; } @@ -32,6 +32,12 @@ public static function newSuccessResponse( Donation $donation, ?string $paymentP */ public static function newFailureResponse( array $errors ): self { $response = new self(); + + // We need this check because isSuccessful checks for the existence of errors + if ( count( $errors ) === 0 ) { + throw new \InvalidArgumentException( 'Failure response must contain at least one error' ); + } + $response->validationErrors = $errors; return $response; } @@ -54,14 +60,20 @@ public function isSuccessful(): bool { * ATTENTION: We're returning the domain object in order to avoid a verbose read-only response model. * Keep in mind that your presenters should only query the domain object * and NOT call any of its state-changing methods! - * @return Donation|null + * @return Donation */ - public function getDonation(): ?Donation { + public function getDonation(): Donation { + if ( $this->donation === null ) { + throw new \DomainException( 'Donation is not set. You probably tried to get donation from an Error response' ); + } return $this->donation; } - public function getPaymentProviderRedirectUrl(): ?string { - return $this->paymentProviderRedirectUrl; + public function getPaymentCompletionUrl(): string { + if ( $this->paymentCompletionUrl === null ) { + throw new \DomainException( 'Payment completion URL is not set. You probably tried to get it from an Error response' ); + } + return $this->paymentCompletionUrl; } } diff --git a/src/UseCases/AddDonation/AddDonationUseCase.php b/src/UseCases/AddDonation/AddDonationUseCase.php index 08d2928d..b864595d 100644 --- a/src/UseCases/AddDonation/AddDonationUseCase.php +++ b/src/UseCases/AddDonation/AddDonationUseCase.php @@ -83,7 +83,7 @@ public function addDonation( AddDonationRequest $donationRequest ): AddDonationR // The notifier checks if a notification is really needed (e.g. amount too high) $this->notifier->sendModerationNotificationToAdmin( $donation ); - return AddDonationResponse::newSuccessResponse( $donation, $paymentResult->externalPaymentCompletionUrl ); + return AddDonationResponse::newSuccessResponse( $donation, $paymentResult->paymentCompletionUrl ); } private function newDonationFromRequest( AddDonationRequest $donationRequest, int $donationId, int $paymentId ): Donation { diff --git a/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php b/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php index eb9b0f30..49407602 100644 --- a/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php +++ b/tests/Integration/UseCases/AddDonation/AddDonationUseCaseTest.php @@ -163,7 +163,7 @@ public function testGivenValidRequest_moderationEmailIsSent(): void { public function testGivenValidRequest_withIncompletePayment_confirmationEmailIsNotSent(): void { $paymentService = new SucceedingPaymentServiceStub( new PaymentCreationSucceeded( paymentId: 1, - externalPaymentCompletionUrl: '', + paymentCompletionUrl: '', paymentComplete: false ) ); $mockNotifier = $this->createMock( DonationNotifier::class ); @@ -243,7 +243,7 @@ public function testSuccessResponseContainsGeneratedUrl(): void { $response = $useCase->addDonation( $this->newMinimumDonationRequest() ); - $this->assertSame( self::PAYMENT_PROVIDER_URL, $response->getPaymentProviderRedirectUrl() ); + $this->assertSame( self::PAYMENT_PROVIDER_URL, $response->getPaymentCompletionUrl() ); } public function testUrlGeneratorGetsDonationData(): void { diff --git a/tests/Unit/UseCases/AddDonation/AddDonationResponseTest.php b/tests/Unit/UseCases/AddDonation/AddDonationResponseTest.php new file mode 100644 index 00000000..8580f8c3 --- /dev/null +++ b/tests/Unit/UseCases/AddDonation/AddDonationResponseTest.php @@ -0,0 +1,57 @@ +assertTrue( $response->isSuccessful() ); + } + + public function testGivenFailureResponse_isSuccessfulReturnsFalse(): void { + $response = AddDonationResponse::newFailureResponse( [ $this->givenConstraintViolation() ] ); + + $this->assertFalse( $response->isSuccessful() ); + } + + public function testFailureResponseMustContainAtLeastOneError(): void { + $this->expectException( \InvalidArgumentException::class ); + $this->expectExceptionMessage( 'Failure response must contain at least one error' ); + + AddDonationResponse::newFailureResponse( [] ); + } + + public function testGivenFailureResponse_getDonationWillThrowException(): void { + $response = AddDonationResponse::newFailureResponse( [ $this->givenConstraintViolation() ] ); + + $this->expectException( \DomainException::class ); + + $response->getDonation(); + } + + public function testGivenFailureResponse_getPaymentCompletionUrlWillThrowException(): void { + $response = AddDonationResponse::newFailureResponse( [ $this->givenConstraintViolation() ] ); + + $this->expectException( \DomainException::class ); + + $response->getPaymentCompletionUrl(); + } + + private function givenConstraintViolation(): ConstraintViolation { + return new ConstraintViolation( 'test', 'a sample violation', 'someField' ); + } +} From 8b5b3e5fe6398282a644bea2415f4464e2822083 Mon Sep 17 00:00:00 2001 From: Gabriel Birke Date: Mon, 23 Oct 2023 15:35:40 +0200 Subject: [PATCH 10/10] Set payment version The new payment bounded context now has a release --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index fa9dd98e..79e8ee66 100644 --- a/composer.json +++ b/composer.json @@ -16,7 +16,7 @@ "wmde/euro": "~1.0", "wmde/freezable-value-object": "~2.0", "wmde/fun-validators": "~4.0", - "wmde/fundraising-payments": "dev-payment-with-ppl-api" + "wmde/fundraising-payments": "~7.0" }, "repositories": [ {