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

[DO NOT REVIEW] Merge Contact Information Feature Branch #1250

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jaxdesmarais
Copy link
Contributor

Summary of changes

  • These changes were reviewed and approved previously, this is just to trigger CI and ensure all passes before merging into main

Checklist

  • Added a changelog entry
  • Relevant test coverage
  • Tested and confirmed payment flows affected by this change are functioning as expected

Authors

@richherrera @jaxdesmarais

richherrera and others added 4 commits December 2, 2024 10:25
* Add contact information data class

* Add ContactInformation request model

* Add UTs

* Update CHANGELOG
* Add contact information data class

* Add ContactInformation request model

* Add UTs

* Update CHANGELOG

* Add contact information toggle
@@ -64,6 +64,7 @@ class PayPalCheckoutRequest @JvmOverloads constructor(
var currencyCode: String? = null,
var shouldRequestBillingAgreement: Boolean = false,
var shouldOfferPayLater: Boolean = false,
var contactInformation: PayPalContactInformation? = null,
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is the final PR not requiring review but realized we are missing the doc string for this property - iOS has Optional: Contact information of the recipient for the order

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, looks like we don't use the Optional part on Android based on the docstrings here, added the missing one here: 028271a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants