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

Move Shipping Callback URL to PayPalCheckoutRequest #1200

Merged
merged 6 commits into from
Nov 1, 2024

Conversation

richherrera
Copy link
Contributor

Summary of changes

  • Move the shippingCallbackUrl parameter to PayPalCheckoutRequest to limit it to checkout flows. Shipping callbacks should only be available for PayPal checkout flows.

Checklist

  • Added a changelog entry
  • Relevant test coverage

Authors

@richherrera richherrera self-assigned this Oct 31, 2024
@richherrera richherrera requested a review from a team as a code owner October 31, 2024 19:32
Comment on lines -85 to -87
shippingCallbackUrl?.let {
if (it.toString().isNotEmpty()) parameters.put(SHIPPING_CALLBACK_URL_KEY, it)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this logic move into PayPalCheckoutRequest?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Logic is already there

thanks for asking

My mistake was approaching this change differently in iOS. In iOS, I created the feature branch from scratch, while here, I only removed shippingCallbackUrl from the Vault flow, keeping the changes from the PR previously merged into this feature branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah gotcha, no worries! Thanks for pointing out where it was since the approach was slightly different.

@@ -301,57 +299,4 @@ public void createRequestBody_sets_userPhoneNumber_when_not_null() throws JSONEx

assertTrue(requestBody.contains("\"phone_number\":{\"country_code\":\"1\",\"national_number\":\"1231231234\"}"));
}

@Test
public void createRequestBody_sets_shippingCallbackUri_when_not_null() throws JSONException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we able to add these tests to PayPalCheckoutRequestUnitTest?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, there are already 3 unit tests that cover this in PayPalCheckoutRequestUnitTest

public void createRequestBody_sets_shippingCallbackUri_when_not_null() throws JSONException {

@richherrera richherrera merged commit 182f599 into shipping-callback-feature Nov 1, 2024
3 checks passed
@richherrera richherrera deleted the add-shipping-calback-uri branch November 1, 2024 19:13
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.

3 participants