-
Notifications
You must be signed in to change notification settings - Fork 35
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
Resolve "Divide by 0" error in stripe__line_item_enhanced
#89
Conversation
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.
@fivetran-jamie thanks for working through this update. I just have a few small change requests before this will be good for approval.
Also, is the plan for this to be merged into main
and cut as it's own patch update. Or, is the plan to batch this into a release branch along with the other in flight Stripe updates this sprint?
integration_tests/tests/consistency/consistency_line_item_enhanced_count.sql
Show resolved
Hide resolved
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.
@fivetran-jamie this looks good to go! My last request would be to add this same change to this PR. Just to ensure buildkite passes with the standardized model enabled.
Also, please make sure you regen the docs once you enable that variable in the integration_tests/dbt_project.yml. Otherwise, we will overwrite the hosted docs and it will remove the model from the docs. Thanks!
Great! So that change is actually already included in this branch (and the docs are regenerated). It's not showing up in the PR since |
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.
lgtm!
PR Overview
This PR will address the following Issue/Feature:
#86
This PR will result in the following new package version:
This specific change would be 0.14.1, but batched with the other existing Stripe PRs, the release will probably be a breaking change
Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:
Bug Fixes
Divide by 0
error in calculatingunit_amount
in thestripe__line_item_enhanced
model. Now, if the denominatorinvoice_line_item.quantity
is 0,unit_amount
will also be 0.PR Checklist
Basic Validation
Please acknowledge that you have successfully performed the following commands locally:
Before marking this PR as "ready for review" the following have been applied:
Detailed Validation
Please share any and all of your validation steps:
Adjusted
consistency_line_item_enhanced_count
test to also look at differences inunit_amount
(though this should be covered byconsistency_line_item_enhanced
as well) and all validation tests are passingIf you had to summarize this PR in an emoji, which would it be?
0️⃣