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

Fetching upcoming invoice's line items loses the request's original parameters #451

Closed
remi-stripe opened this issue Feb 19, 2018 · 8 comments
Assignees
Labels

Comments

@remi-stripe
Copy link
Contributor

remi-stripe commented Feb 19, 2018

When you retrieve an upcoming invoice in the API, your invoice might have more than 10 items. In that case you end up having to paginate the invoice. When doing so, you want to make sure that your parameters are passed automatically so that things like proration or plan changes are still taken into account on the next page(s).

Today, this is not working properly. This was originally reported in #264 and fixed in #275 so that you could explicitly pass the parameters to autoPagingIterable(). This was broken again in #294 when we removed StripeCollectionAPIResource though.

To reproduce the issue you need to do something like this:

  • Create a customer with a subscription on a $20 plan
  • Create 11 invoice items on that customer
  • Retrieve the upcoming invoice simulating a change to a plan for $50
  • Confirm that the last line item, for next month's subscription, is for the $50 plan

Example code:

    String customerId = "cus_1234";
    String newPlan = "plan_1234";
    String subscriptionId = "sub_1234";

    Subscription sub = Subscription.retrieve(subscriptionId);
    Map<String, Object> itemNew = new HashMap<String, Object>();
    itemNew.put("id", sub.getSubscriptionItems().getData().get(0).getId());
    itemNew.put("plan", newPlan);

    List<Object> itemsChange = new ArrayList<>();
    itemsChange.add(itemNew);

    Map<String, Object> invoiceParams = new HashMap<String, Object>();
    invoiceParams.put("customer", customerId);
    invoiceParams.put("subscription", sub.getId());
    invoiceParams.put("subscription_items", itemsChange);
    Invoice upcoming = Invoice.upcoming(invoiceParams);

    Iterable<InvoiceLineItem> itItems = upcoming.getLines().autoPagingIterable();
    for (InvoiceLineItem item : itItems) {
        System.out.println(item);
    }
@ob-stripe
Copy link
Contributor

I believe this is essentially the same issue as #264 -- the collection returned by getLines() does not have the parameters and options that were passed when fetching the upcoming invoice (cf. #264 (comment)).

A workaround is to re-fetch the line items and explicitly pass params/options. It's what we do in this test:

@Test
public void testUpcomingInvoiceLines() throws Exception {
Customer customer = Customer.create(defaultCustomerParams);
InvoiceItem item = createDefaultInvoiceItem(customer);
Map<String, Object> upcomingParams = new HashMap<String, Object>();
upcomingParams.put("customer", customer.getId());
Invoice upcomingInvoice = Invoice.upcoming(upcomingParams);
assertFalse(upcomingInvoice.getAttempted());
InvoiceLineItemCollection lines = upcomingInvoice.getLines().all(null);
assertFalse(lines.getData().isEmpty());
assertEquals(item.getId(), lines.getData().get(0).getId());
Map<String, Object> fetchParams = new HashMap<String, Object>();
fetchParams.put("starting_after", item.getId());
InvoiceLineItemCollection linesAfterFirst = upcomingInvoice.getLines().all(fetchParams);
assertTrue(linesAfterFirst.getData().isEmpty());
}
.

This is sub-optimal as it forces you to re-fetch the line items that were already returned with the invoice object.

We could reintroduce the fix from #275, or try the second approach described by @brandur-stripe here: #264 (comment).

@brandur-stripe wdyt?

@brandur-stripe
Copy link
Contributor

Thanks for the great summary @remi-stripe / @oliver-stripe! I must admit that I'd totally forgotten about pretty much all of this stuff ...

We could reintroduce the fix from #275, or try the second approach described by @brandur-stripe here: #264 (comment).

Maybe both are in order? It seems like having the option to send parameters and options as seen in #275 can't hurt, but even if it's possible, it still strikes me as more of a workaround than anything else — as a library user, I think I'd still be surprised that I'd have to send parameters/options twice when fetching something like additional invoice items.

I'm not a fan of reflection-based solutions by any means (as described by the aforementioned "second approach), but it does strike me as the most foolproof option — parameters/options would plumb through automatically, which is probably what most people want most of the time. Do you guys have an opinion?

@ob-stripe
Copy link
Contributor

@brandur-stripe I agree with everything you said :)

I opened a PR for the first approach in #459.

@rabzu
Copy link

rabzu commented Jan 17, 2020

Hi, is this issue going to be fixed?

@remi-stripe
Copy link
Contributor Author

@rabzu Unfortunately this is a gnarly one to fix reliably but it's definitely on our radar to focus on. I've bumped it internally to see if it's something we can prioritize this quarter to fix.

@itgeorge
Copy link

Hey everyone, any updates on this? We recently updated our development (test) version from stripe-java 11.0.0 to stripe-java 19.32.0 (and I just tried with 19.34.0 too), and this issue popped up. Note that the Stripe API version we're using in production (live) is still 2019-08-14 - not sure if the newer stripe-java communicates with the new API version, or with our old production version and if that may be affecting the results. But I'd like to know if this issue will persist before we start migrating our production codebase.

It seems invoice.getLines().autoPagingIterable() appends the ?starting_after=... parameter to the customer id when getting the next page, hence causing a (warning: test data and ids ahead): com.stripe.exception.InvalidRequestException: No such customer: cus_HhTsOHuYEb72d8?starting_after=ii_1H84v7ADfYIaWhWJ2Av69oq8; code: resource_missing;.

This code used to work fine on stripe-java 11.0.0 (we used to do InvoiceLineItemCollection lines = invoice.getLines(); lines.setRequestOptions(...);, and then iterate the autoPagingIterable())

I've tried passing in the customer parameter in the autoPagingIterable(Map<String, Object> params, RequestOptions options) overload, but that causes the paging to just hang indefinitely (i.e. after the iteration hits the 10th item it gets stuck in socketRead).
I also tried passing in an invoice parameter instead, but that came back with an error that it isn't supported.

Is this something I'm doing wrong, and is there a workaround to this? I suppose this would work if I manually do the pagination with starting_after?

@remi-stripe
Copy link
Contributor Author

@itgeorge Thanks for the ping and sorry for the lack of update on that one. This is not an issue we have fixed yet because it's unfortunately a tricky edge-case that we are trying to fix another way by adding a client/service infrastructure that would let you explicitly call the endpoint you expect instead.

We'll investigate again though to see if there's something we can temporarily introduce to fix this.

@pakrym-stripe
Copy link
Contributor

The original issue with the upcoming invoice line item iteration should be fixed on the service side now.

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

No branches or pull requests

7 participants