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.

Sign up for GitHub

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

Jump to bottom

Add support for the payment_intent resource #543

Merged
merged 1 commit into from
Jun 29, 2018
Merged

Add support for the payment_intent resource #543

merged 1 commit into from
Jun 29, 2018

Conversation

remi-stripe
Copy link
Contributor

This resource is gated so for now tests will be stubbed.

cc @michelle-stripe @jenan-stripe @stripe/api-libraries

@remi-stripe remi-stripe force-pushed the remi-add-payment-intent branch 3 times, most recently from 3b7df4b to b345fe2 Compare June 28, 2018 17:56
rattrayalex-stripe
rattrayalex-stripe reviewed Jun 28, 2018
View reviewed changes
src/main/java/com/stripe/model/PaymentIntentCollection.java Outdated
@@ -0,0 +1,4 @@
package com.stripe.model;

public class PaymentIntentCollection extends StripeCollection<Payout> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm unfamiliar with our client libs / java in general, but is Payout a typo?

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 it's just a WIP branch to start discussion on naming

@remi-stripe remi-stripe force-pushed the remi-add-payment-intent branch 3 times, most recently from af7bb00 to a91df5d Compare June 28, 2018 19:42
remi-stripe
remi-stripe commented Jun 28, 2018
View reviewed changes
src/main/java/com/stripe/model/PaymentIntent.java
PaymentIntentSourceAction nextSourceAction;
@Getter(AccessLevel.NONE) @Setter(AccessLevel.NONE) ExpandableField<Account> onBehalfOf;
String receiptEmail;
String returnUrl;

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

src/main/java/com/stripe/model/PaymentIntentSourceActionDeserializer.java
String type = jsonObject.get("type").getAsString();
sourceAction.setType(type);
Class<? extends PaymentIntentSourceActionValue> cl = objectMap.get(type);
if (cl != null) {

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

src/main/java/com/stripe/model/ShippingDetails.java
protected String name;
protected String phone;
protected String trackingNumber;

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

@remi-stripe remi-stripe force-pushed the remi-add-payment-intent branch from a91df5d to fc22242 Compare June 28, 2018 19:46
@remi-stripe remi-stripe changed the title [WIP] Add support for the payment resource Add support for the payment resource Jun 28, 2018
@remi-stripe
Copy link
Contributor Author

r? @ob-stripe

ob-stripe
ob-stripe suggested changes Jun 29, 2018
View reviewed changes
src/main/java/com/stripe/model/PaymentIntentSourceActionValue.java Outdated
@Getter
@Setter
@EqualsAndHashCode(callSuper = false)
public class PaymentIntentSourceActionValue extends StripeObject {
Copy link
Contributor

Choose a reason for hiding this comment

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

This class does not have any attributes, so you can remove all the Lombok annotations. You should also declare the class as abstract as it is an abstract superclass that should not be used directly.

src/main/java/com/stripe/model/ShippingDetails.java
protected String name;
protected String phone;
protected String trackingNumber;

This comment was marked as resolved.

Sign in to view

src/main/java/com/stripe/model/PaymentIntent.java
PaymentIntentSourceAction nextSourceAction;
@Getter(AccessLevel.NONE) @Setter(AccessLevel.NONE) ExpandableField<Account> onBehalfOf;
String receiptEmail;
String returnUrl;

This comment was marked as resolved.

Sign in to view

src/main/java/com/stripe/model/PaymentIntent.java Outdated
return (this.application != null) ? this.application.getId() : null;
}

public void setApplication(String applicationID) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you replace all fooID with fooId? This will make it easier to rebase the v6 branch.

src/main/java/com/stripe/model/PaymentIntent.java Outdated
}

public void setSourceObject(Account c) {
this.onBehalfOf = new ExpandableField<Account>(c.getId(), c);
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy/paste error here, Account should be ExternalAccount.

src/main/java/com/stripe/model/PaymentIntent.java Outdated

// <editor-fold desc="capture">
/**
* capture a payment intent.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, but can you check that all Javadoc comments start with a capital letter?

src/main/java/com/stripe/model/PaymentIntent.java Outdated
return request(RequestMethod.POST, instanceURL(PaymentIntent.class, this.id) + "/confirm", null,
PaymentIntent.class, options);
}
// </editor-fold>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you confirm that none of the cancel, confirm and create requests accept any parameters server-side (and that there are no plans to add parameters)? If not, we should add signatures that accept params.

src/main/java/com/stripe/model/PaymentIntentSourceActionDeserializer.java
String type = jsonObject.get("type").getAsString();
sourceAction.setType(type);
Class<? extends PaymentIntentSourceActionValue> cl = objectMap.get(type);
if (cl != null) {

This comment was marked as resolved.

Sign in to view

src/test/java/com/stripe/model/PaymentIntentTest.java Outdated
assertNotNull(actionValue);
assertEquals("https://stripe.com", actionValue.getUrl());
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hate to be a bother, but do you mind adding a second test (and fixture) for the expandable attributes?

@remi-stripe
Copy link
Contributor Author

@ob-stripe Thank you for the careful review! Damn I missed a few things! Can you have a look?

@remi-stripe
Add support for the payment resource
cc5748f 
This resource is gated so for now tests will be stubbed
Fix linting errors
@remi-stripe remi-stripe force-pushed the remi-add-payment-intent branch from ea0d9e7 to cc5748f Compare June 29, 2018 12:56
@remi-stripe remi-stripe assigned ob-stripe and unassigned remi-stripe Jun 29, 2018
@ob-stripe ob-stripe changed the title Add support for the payment resource Add support for the payment_intent resource Jun 29, 2018
@stripe-ci stripe-ci assigned remi-stripe and unassigned ob-stripe Jun 29, 2018
@ob-stripe ob-stripe merged commit 534c1e6 into master Jun 29, 2018
@ob-stripe ob-stripe deleted the remi-add-payment-intent branch June 29, 2018 13:30
@ob-stripe
Copy link
Contributor

Released in 5.46.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rattrayalex-stripe rattrayalex-stripe rattrayalex-stripe left review comments

@ob-stripe ob-stripe ob-stripe approved these changes

Assignees

@remi-stripe remi-stripe

Labels
approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@remi-stripe @ob-stripe @rattrayalex-stripe @stripe-ci