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

Subscription Items are not retrievable with items-attribute on Subscription object #297

Closed
anelder-stripe opened this issue Apr 1, 2017 · 10 comments

Comments

@anelder-stripe
Copy link

anelder-stripe commented Apr 1, 2017

The Subscriptions object now has a new attribute on it items that represent the Subscription Items (AKA Multiple Plans) associated with a Subscription. Unfortunately, there is a method on Python Dictionaries called items() and it is causing conflicts.

For anyone who runs into this-- as a work around, you can retrieve the items using sub['items'].

I recommend we rename the parsed items-contents when we're deserializing the JSON and associate it instead with a subscription_items-attribute then update our documentation accordingly for Python.

@anelder-stripe
Copy link
Author

anelder-stripe commented Apr 1, 2017

Not my brainchild, so thank @ob-stripe -- but an easy fix might be:

@property
def subcription_items(self):
    return self['items']

@subscription_items.setter
def subscription_items(self, value):
    self['items'] = value

@brandur-stripe
Copy link
Contributor

Thanks for opening this @anelder-stripe!

So I guess that the two critiques I'd have of the accessor approach are:

  1. This seems like it might grow to be an unbounded problem. It's not a great workaround to just add more accessors every time that we collide with a language keyword, and not all accessors will have such an easy alternate name as subscription_items.
  2. As far as I know, there isn't really a great mechanic right now for exposing the existence of these alternate accessors. We don't really a list of them on the API reference documentation, which means that users will have to either go to code or ask someone on support.

I'm not sure if I have a good alternative, but maybe even recommending the use of sub['items'] in this sort of case isn't the worst thing in the world.

@ob-stripe
Copy link
Contributor

Even though I'm the one who suggested the accessor approach, I'm not too thrilled about adding aliases either -- esp. because it'd be very hard to properly document those.

The annoying part with the existing state is that you can use sub.items as a setter but not as a getter:

sub = stripe.Subscription.retrieve('sub_...')
sub.items = [{'plan': '10monthly'}]
sub.save()
# At this point, the subscription item is refreshed with the values returned by the API
# and the items attribute is reassigned to dict's items() function.
sub.items
# <built-in method items of Subscription object at 0x10f83fe30>
sub['items']
# <ListObject list at 0x11041aab8> JSON: {...} 

It's largely a matter of opinion, but I'd argue that preserving getter/setter consistency is more important than preserving the items() function (which IMO has limited utility outside of tests / debugging).

We could define an items property to properly return the expected value, and it'd still be possible to access the items() method via dict(sub).items().

@brandur-stripe
Copy link
Contributor

@ob-stripe Cool, thanks!

I dunno. I think you're right, but I can't get behind the idea of overwriting built-in methods, even though we're almost certainly doing it elsewhere in Ruby and such.

The best thing that I can think of right now might be to start implementing server-side naming checks for common programming language names and maybe start phasing these ones out with API versioning.

@ob-stripe
Copy link
Contributor

I'll go ahead and close this issue since it doesn't sound like we have a better client-side fix other than the existing workaround (i.e. using ['items'] instead of .items when getting the value).

Hopefully in the next major version of stripe-python, StripeObjects will no longer subclass dict directly and this will not be an issue anymore.

@ColdHeat
Copy link

Why is the issue closed if the workaround isn't clear in the documentation? I'm just trying to update Subscription plan quantities...

@rishig
Copy link

rishig commented Aug 30, 2018

Just ran into this as well, in case that's a helpful data point. Took me a while to find this issue.

@Benoss
Copy link

Benoss commented Dec 17, 2022

4 years later I this thread saved me ...
sub["items"].data[0].price.product works but
sub.items.data[0].price.product does because it is a setter so
sub.items().data[0].price.product does not work either because you just set the items to nothing

@dzhlobo
Copy link

dzhlobo commented Jun 27, 2023

Having subscription items accessible only via __getitem__()(subscription["items"]) makes it harder to mock Subscription objects in tests.

I ended up creating a unittest.mock.MagicMock object and then assign items in a separate step:

def create_stripe_subscription(self):
    subscription = unittest.mock.MagicMock(
        spec=stripe.Subscription,
        # ...
    )
    subscription["items"].data = [
        unittest.mock.Mock(
            spec=stripe.SubscriptionItem,
            # ...
        )
    )
    return subscription

Not nice.

@matt-dalton
Copy link

There is an additional problem here because the python types allow you to write subscription.items. So I'm trying to make our code typesafe, but this crashes:
subscription.items.data[0].plan

If I do this subscription['items'].data[0].plan all the types get set to Any, which leaves us a bit stuck

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

No branches or pull requests

8 participants