Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement new PayPal API #135

Merged
merged 61 commits into from
Oct 23, 2023
Merged

Implement new PayPal API #135

merged 61 commits into from
Oct 23, 2023

Conversation

gbirke
Copy link
Member

@gbirke gbirke commented Aug 1, 2023

@gbirke gbirke force-pushed the payment-with-ppl-api branch 5 times, most recently from 4c9114c to 41c499d Compare August 11, 2023 15:37
gbirke and others added 25 commits October 11, 2023 18:51
Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
Use more constants, fewer "magic strings"
PayPal supports sending credentials, which is fine for server-to-server
communication. This gets rid of a lot of code and an unnecessary HTTP
request
Add test and code, add logging

Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
Add JSON serialization to Product
Add createProduct method to GuzzlePaypalAPI

Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
This model will be used to create subscription plans (for recurring
donation and membership payments) in the PayPal API

Co-Authored-By: Corinna Hillebrand <[email protected]>
We'll need this for the "list plans" API calls

Co-Authored-By: Corinna Hillebrand <[email protected]>
Not fully tested yet

Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
Implement logic for creating products if they don't exist

Start with a test for createSubscriptionPlanForProduct in the API,
outline the TODOs for the method

Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
- changes the contents of the SuccessResult
- tests different creation scenarios
- adapt fake API etc.

Co-Authored-By: Gabriel Birke <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
Read env variables, ignore `.env` file, initialize API and list products

Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
Send auth header base64 encoded
Ignore missing total_pages in JSON response

Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
Switch around parameter order for id and name
Fix serialization for SubscriptionPlan (was missing required parameters)
Make ScalarTypeConverter generally available and explain its use

Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
Fix internal logic of CreateSubscriptionPlanForProductUseCase to return
the generated subscription plan ID when returning subscription plans.

Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
gbirke and others added 23 commits October 11, 2023 18:51
Fix broken coverage annotation

This is for https://phabricator.wikimedia.org/T340734

Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
For the CreateSubscriptionPlansCommand we decided that the configuration
should contain descriptive interval names. The
PayPalAPIURLGeneratorConfigFactory still used integers.
We extracted the interval creation from string into `PaymentInterval`

Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
Change language to locales (we discovered on integration that using
locales is more convenient in the app).

Document return_url and cancel_url

Co-Authored-By: Corinna Hillebrand <[email protected]>
Co-Authored-By: Sperling-0 <[email protected]>
Use Symfony TreeBuilder checks to check the structure of the
PayPal API configuration.

Add tests for missing and empty file.

This is for https://phabricator.wikimedia.org/T329161
Replace application_context with experience_context, because the
application_context is deprecated.

Set user_action to 'PAY_NOW' because we want to immediately capture the
payment and know the amount.
After rebasing the PayPal API branch onto `main` that has the
architecture checks in it, we needed to rethink the current architecture
a bit. The main outcome is to have `PaymentUrlGenerator` as a Domain
interface, but move the implementations out of the Domain into Services.
The implementations need services, so the whole namespace is suited
better to the "Servies" namespace.

Renamed layer "ServiceInterface" to "ServiceModel", to include value
objects (classes) used in the service interfaces.

Deleted AdditionalPaymentData class since it was not used anywhere
External identifiers for Payments. Will be used to look up a
Donation/Membership when processing IPNs that were trigged by a PayPal
API call.

https://phabricator.wikimedia.org/T343846
Add an interface and an implementation in DoctrinePaymentRepository.
We don't need a separate repository class.

Added a cascade-persist in the DDL for PayPalPaymentIdentifier to store
the Payment together with the PayPalPaymentIdentifier.
Create interface that will allow API calls to an external payment
provider during the payment creation (in the use case). We'll inject the
interface in subsequent commits.

The PayPal implementation, PayPalPaymentProviderAdapter will replace the
PayPalAPIURLGenerator. Creating Orders (with ID and invoice ID) is
supported, but currently there is no way to pass in the parameters from
the donation/membership bounded context (like we do for the
UrlGenerator). Creating Subscriptions with a start date is also not yet
supported, because it also needs a context object passed in.

Add new URL generators for PayPal that will replace PayPalAPIURLGenerator

https://phabricator.wikimedia.org/T343846
For running database commands like dumping the schema
Add TestIban test fixture for easier testing
Rename discriminator-column in PayPalPaymentIdentifier mapping to `identifier_type`

https://phabricator.wikimedia.org/T343846

Co-Authored-By: Sperling-0 <[email protected]>
`CreatePaymentUseCase` now uses an *interface* called
`PaymentProviderAdapterFactory`. We renamed the implementation to
`PaymentProviderAdapterFactoryImplementation` (we could not think of a
better name :( ) The use case will call both methods, giving the payment
provider the option to modify the payment (or store additional data
related to the payment) and the option to replace or modify the
UrlGenerator implementation. At the moment, we need both methods for
PayPal. In the future, we might move the Sofort API into an adapter and
use a URL generator pattern similar to PayPal.

We split the Fake PayPal API classes into two different implementations,
one for tests that test the interaction with the API that creates
subscription plans and their products. The other one is for tests that
interact with the API calls that create payments. This split is an
indication that we might want to split the `PaypalAPI` interface into
two, one for the "recurring payments setup" methods and the other for
the "create payment" methods. This is too much effort at the moment, so
we'll leave it for now.

https://phabricator.wikimedia.org/T343846

Co-Authored-By: Sperling-0 <[email protected]>
The factory now has a proper feature flag for using the
`LegacyPayPalURLGenerator` vs the `PayPalURLGenerator`.

One-time payments with the PayPal API are currently not supported (see
https://phabricator.wikimedia.org/T344263), we fall back on the
`LegacyPayPalURLGenerator` for them.

Changed PayPalAPIURLGeneratorConfigFactory as a factory for
PayPalPaymentProviderAdapterConfig. This will be used by the Fundraising
App when it creates a configuration from its YAML file

Add comments across the code to explain how the classes work together.
Also add open tickets for the removal of the LegacyPayPalURLGenerator
and implementing the PayPalURLGenerator for one-time-payments

https://phabricator.wikimedia.org/T343846

Co-Authored-By: Sperling-0 <[email protected]>
It carries domain-specific information from the donation/membership
domain. It'll replace the `RequestContext` class for the URL generators.
We'll also use it in `PayPalPaymentProviderAdapter`.

This PR only introduces the `DomainSpecificContext` class, we're not
actively using it yet.

This is a backwards-compatibility breaking change because it changes the
behavior of `PaymentCreationRequest`, forcing the calling code to call
`setDomainSpecificContext`!

I specifically chose setter injection over constructor injection for the
`DomainSpecificContext` dependency, because in the Fundraising App, the
Controller builds `PaymentCreationRequest`, while only the
donation/membership creation use case has all the information to create
the `DomainSpecificContext`, and needs to *add* that information to an
existing `PaymentCreationRequest`.

https://phabricator.wikimedia.org/T344271
Modify placeholders in returnUrl and cancelURL of the
PayPalPaymentProviderAdapterConfig using the values in DomainSpecificContext

The DomainUrlTemplate class that does the replacing is prepared for a
future where we pass a user and a system token instead of passing an
update and access token
(see [T344346](https://phabricator.wikimedia.org/T344346))

Add Spy functionality to FakePayPalAPIForPayments.

https://phabricator.wikimedia.org/T344271
Add method to convert `DomainSpecificContext` into a `RequestContext` value
object. We do the conversion to keep the two layers separate.

This is backwards-compatibility breaking change because it changes the
`SuccessResponse` of `CreatePaymentUseCase`! The `SuccessResponse` now
contains the generated URL instead of the `PaymentProviderURLGenerator`
instance.

https://phabricator.wikimedia.org/T344271
Remove userAccessToken and systemAccessToken from RequestContext.
Instead, allow the calling code to add authentication tokens. This
will allow us to change the URL parameters of our own application more
easily in the future.

https://phabricator.wikimedia.org/T344271
Use DomainSpecificContext instead. Now that the tokens are gone from it,
they're almost idential.

https://phabricator.wikimedia.org/T344271
Split into base PaymentCreationRequest (used in the Fundraising
Application to create a request from user data) and
DomainSpecificPaymentCreationRequest (used in the donation and
membership bounded contexts to add context-specific validators,
authenticators and context information).

This gets rid of the brittle setter injection, at the cost of breaking
backwards compatibility.

https://phabricator.wikimedia.org/T343846
Make URLAuthenticator a call dependency, not a constructor dependency.
This makes it possible to construct the PaymentURLFactory in the
FunFunFactory in the Fundraising App.

https://phabricator.wikimedia.org/T341795
We moved the url generator stuff to the adapter, this was a last remnant
of this. Renaming for consistency in the FunFunFactory
The adapter was receiving errors when modifyPaymentUrlGenerator was
called with LegacyPayPalURLGenerator. We now allow LegacyPayPalURLGenerator
and return it unchanged.
Rename PaymentCreationRequest to PaymentParameters, because it's not a
request object passed to a use case any more, but parameters to
construct a request.

Rename DomainSpecificPaymentCreationRequest to PaymentCreationRequest
because that name is now available.

This will cause further confusion and BC breaks up the chain of pull
requests, but makes the payment domain and responsibilities clearer.
This prepares the database layer for the two-step process of creating a
one-time-payment (Order) with the PayPal API:
- The first API call creates the Order, returning an Order ID
- When the user comes back from the payment provider to the return URL,
  the URL will contain the Order ID parameter and we'll have to make
  another API request to *capture* the order, which will return a
  transaction ID. We won't *book* the payment (that's still done in the
  IPN handling), but we will store the returned transaction ID in the
  PayPalOrder class, so the notification code can lookup the payment via
  transaction ID.

This change also introduces indexes for the `PayPalPaymentIdentifier`
subclasses `PayPalOrder` and `PayPalSubscription`

Ticket: https://phabricator.wikimedia.org/T344839
Add migration to create the table payment_paypal_identifier and indexes;
The `SuccessResponse` of the `CreatePaymentUseCase` now returns a
`paymentCompletionUrl` for all payment types. This will save a branch
condition and extra code in the controllers of the fundraising
application.

Conceptually, the URL generators are no longer for payment providers but
for the "completion" of the payment, so I renamed `PaymentProviderURLGenerator` to
`PaymentCompletionURLGenerator`.

The `NullGenerator` is no longer used in production code, so I moved it to a fixture.
@gbirke gbirke merged commit f05fbe3 into main Oct 23, 2023
2 checks passed
@gbirke gbirke deleted the payment-with-ppl-api branch October 23, 2023 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants