-
Notifications
You must be signed in to change notification settings - Fork 429
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
fix: Refactor existing Chargebee webhooks for subscriptions #3047
fix: Refactor existing Chargebee webhooks for subscriptions #3047
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Uffizzi Preview |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with one minor comment question.
@@ -662,12 +668,13 @@ def test_when_subscription_is_cancelled_then_cancellation_date_set_and_alert_sen | |||
): | |||
# Given | |||
cancellation_date = datetime.now(tz=UTC) + timedelta(days=1) | |||
current_term_end = int(datetime.timestamp(cancellation_date)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious as to why this had to change as part of this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the Chargebee API sends unix time in integer seconds, not milliseconds or microseconds. So to go through the API we need to fix the integer resolution to get through the serializer.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3047 +/- ##
==========================================
- Coverage 95.89% 95.89% -0.01%
==========================================
Files 1039 1039
Lines 30688 30721 +33
==========================================
+ Hits 29429 29459 +30
- Misses 1259 1262 +3 ☔ View full report in Codecov by Sentry. |
Thanks for submitting a PR! Please check the boxes below:
pre-commit
to check lintingChanges
Taking direction from earlier work, I clustered these functionalities together with the other pieces of isolated webhook modules. The basic patterns are the same, although this instance the serializers were more involved. I took a look at the Chargebee webhooks response document and tried to match things up as best as I could.
How did you test this code?
The existing test base has good coverage of this part. I was able to add alterations to the test base where deviations had come into play based off of what is in the serializers. For example, the chargebee unixtimestamp is in integer format, not floats, so these and other things were needed to be fixed.