Skip to content

Conversation

@wscourge
Copy link
Contributor

@wscourge wscourge commented Dec 26, 2025

  • Added deprecation warning to Subscription.list_imported() method
  • Added CustomerSubscription import to customer module
  • Updated tests to use correct imports and assertions

@wscourge wscourge requested a review from Copilot December 26, 2025 10:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR deprecates CustomerSubscription.list_imported() in favor of a new Customer.subscriptions() method and updates documentation examples to use correct field names.

  • Adds new Customer.subscriptions() method for retrieving customer subscriptions
  • Marks CustomerSubscription.list_imported() as deprecated with a warning
  • Corrects field names from external_id to uuid in subscription connection examples

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
chartmogul/api/customer.py Adds new Customer.subscriptions() method
chartmogul/api/customers/subscription.py Implements deprecation warning for list_imported() method
test/api/test_customer.py Adds test coverage for new subscriptions() method and updates field names in existing tests
README.md Documents new subscriptions() method and corrects field names in examples

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@loomchild loomchild self-assigned this Dec 31, 2025
@loomchild loomchild self-requested a review December 31, 2025 18:30
Copy link

@loomchild loomchild left a comment

Choose a reason for hiding this comment

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

Overall, LGTM, but I have some trouble testing it. Please provide testing instructions and which environment to use.

Customer.unmerge = Customer._method("unmerge", "post", "/customers/unmerges")
Customer.subscriptions = CustomerSubscription._method(
"all", "get", "/customers/{uuid}/subscriptions", useCallerClass=True
)
Copy link

@loomchild loomchild Dec 31, 2025

Choose a reason for hiding this comment

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

I get the following error in production, am I doing something wrong?

customer.subscriptions(config).get()

requests.exceptions.HTTPError: 400 Client Error: Bad Request for url: https://api.chartmogul.com/v1/customers//subscriptions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is documented in the README. Same as other methods that require uuid in the URL, it requires passing it:

>>> customer = Customer.retrieve(config, uuid='cus_c9352490-8d66-403c-90c5-01663aae2959').get()
>>> customer.subscriptions(config, uuid='cus_c9352490-8d66-403c-90c5-01663aae2959').get()
Subscriptions(entries=[<CustomerSubscription{arr=217200, billing_cycle='year', billing_cycle_count=1, currency='PLN', currency_sign='zł', end_date=datetime.datetime(2026, 11, 23, 17, 51, 44, tzinfo=UTC), external_id='cbdemo_ZpbKpmKUbu83EsNv', id=5322874574, mrr=18100, plan='Professional Suite Annual(cbdemo_omnisupport-solutions)', quantity=1, start_date=datetime.datetime(2024, 11, 22, 17, 51, 46, tzinfo=UTC), status='active', subscription_set_external_id='cbdemo_ZpbKpmKUbu83EsNv', uuid='8d80f275-a494-4957-8968-6cb68acdcfab'}>, <CustomerSubscription{arr=108576, billing_cycle='year', billing_cycle_count=1, currency='PLN', currency_sign='zł', end_date=datetime.datetime(2026, 11, 23, 17, 51, 44, tzinfo=UTC), external_id='cbdemo_ZpbKpmKUbu83EsNv_cbdemo_workforce-optimizer-addon-annual', id=5322874575, mrr=9048, plan='Workforce Optimizer Add-on Annual(cbdemo_omnisupport-solutions)', quantity=1, start_date=datetime.datetime(2024, 11, 22, 17, 51, 46, tzinfo=UTC), status='active', subscription_set_external_id='cbdemo_ZpbKpmKUbu83EsNv', uuid='77867070-2435-4da1-8bde-014f6817bd49'}>, <CustomerSubscription{arr=233184, billing_cycle='month', billing_cycle_count=1, currency='PLN', currency_sign='zł', end_date=datetime.datetime(2026, 1, 11, 14, 9, 32, tzinfo=UTC), external_id='169vEGV551MI2J0', id=5322874576, mrr=19432, plan='Professional Suite Monthly(cbdemo_omnisupport-solutions)', quantity=1, start_date=datetime.datetime(2025, 12, 11, 14, 9, 32, tzinfo=UTC), status='active', subscription_set_external_id='169vEGV551MI2J0', uuid='a8640a5a-0d43-41c7-803e-76fc042267b0'}>], has_more=False, cursor='c3Vic2NyaXB0aW9uc19uZXh0X3BhZ2U9Mg==')

return _original_list_imported.__func__(cls, config, **kwargs)


CustomerSubscription.list_imported = _deprecated_list_imported

Choose a reason for hiding this comment

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

This works. Maybe also worth updating the README.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a README.md update, I added this comment:

# DEPRECATED: use chartmogul.Customer.subscriptions() instead

Do you have anything else in mind?

@wscourge wscourge force-pushed the wiktor/ome-417-add-support-for-subscription-connections-via branch from 264a4ad to 899c842 Compare January 2, 2026 14:40
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