-
Notifications
You must be signed in to change notification settings - Fork 1
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
Adapts billing history to invoice grain #8
Merged
fivetran-avinash
merged 9 commits into
main
from
bugfix/modifying-zuora-billing-history-to-invoice-grain
Aug 21, 2023
Merged
Adapts billing history to invoice grain #8
fivetran-avinash
merged 9 commits into
main
from
bugfix/modifying-zuora-billing-history-to-invoice-grain
Aug 21, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 tasks
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-reneeli Should be good for re-review!
fivetran-reneeli
approved these changes
Aug 18, 2023
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.
Looks good, just remember to change the deps!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Overview
This PR will address the following Issue/Feature: [#7]
This PR will result in the following new package version:
v0.2.0
It's a breaking change as a number of fields were removed or created in
zuora_billing_history
.Please detail what change(s) this PR introduces and any additional information that should be known during the review of this PR:
zuora__billing_history
andint_zuora__billing_enriched
soinvoice_id
was the actual grain, accounting for the cases when invoices can be tied to multiple credit balance adjustments and payments. This required the following steps:credit_balance_adjustment_id
were aggregated on the invoice level (number of credit balance adjustments,total credit balance amount in home currency, first and last dates of credit balance adjustments on the invoice). Fields associated with an individualcredit_balance_adjustment_id
were removed.payment_id
were aggregated on the invoice level (number of payments, total payment amount in home currency, first and last dates of payments on the invoice). Fields associated with an individualpayment_id
were removed.payment_method_id
to gather a count of payment methods on each invoice.zuora__billing_history
with aggregated fields, and removed non-aggregated fields associated with credit balance adjustments, payments, and payment methods.int_zuora__account_enriched
to account for new aggregated metric fields being pulled fromzuora__billing_history
.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 acknowledge that the following validation checks have been performed prior to marking this PR as "ready for review":
Made sure that the models ran on
dbt run --vars
, particularly for adjusting the credit_balance_adjustment and taxation_item variables which are part of the CTEs that are modified.For further validation, I recommend using the updated seed files and testing against the
main
branch to reproduce the bug, then making sure the new branch passes all tests.Standard Updates
Please acknowledge that your PR contains the following standard updates:
dbt Docs
Please acknowledge that after the above were all completed the below were applied to your branch:
If you had to summarize this PR in an emoji, which would it be?
🪙