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

[V7] Add Encodable protocol for PayPal Checkout #1491

Open
wants to merge 9 commits into
base: v7
Choose a base branch
from

Conversation

richherrera
Copy link
Contributor

Summary of changes

  • Add encodable types for post bodies used in the PayPal Checkout flow.
  • PayPal Checkout has been tested in the Demo app to ensure the dictionaries are still constructed as expected.

Note: Since we have two different classes (PayPalCheckoutRequest and PayPalVaultRequest) used in PayPalClient, it's currently not possible to use Encodable until the same implementation is done for PayPalVaultRequest. We tested the flow, but we need to complete task DTMOBILES-1177 to be able to use Encodable.

Checklist

  • [ ] Added a changelog entry
  • Tested and confirmed payment flows affected by this change are functioning as expected

Authors

@richherrera richherrera requested a review from a team as a code owner January 2, 2025 18:21
if payPalRequest.requestBillingAgreement {
self.requestBillingAgreement = payPalRequest.requestBillingAgreement

if let billingAgreementDescription = payPalRequest.billingAgreementDescription {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, this property belongs to class BTPayPalRequest. After changing its properties to internal in a previous PR, merchants can no longer access these properties, meaning there's no way to configure, for example, "billingAgreementDescription" when creating an instance of BTPayPalCheckoutRequest or BTPayPalVaultRequest. Do you think we should add the necessary properties to the init of BTPayPalCheckoutRequest or BTPayPalVaultRequest, or make the properties in BTPayPalRequest public again and remove BTPayPalRequest.init?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great catch, I don't think we want to make the properties on BTPayPalRequest public since that would allow merchants to use the dot syntax, which we would like to move away from. I think it makes sense to add the properties to BTPayPalCheckoutRequest or BTPayPalVaultRequest inits then pass them in the super.init? What do you think?

@richherrera richherrera self-assigned this Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants