-
Notifications
You must be signed in to change notification settings - Fork 2
[Bug fix] MRR type updates, deterministic line item indices, validation tests and streamlit link fixes #43
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
Changes from all commits
fc6ad14
0d54562
655340c
33cb06f
abe6c41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ logs/ | |
| profiles.yml | ||
| target/ | ||
| *.log | ||
| dbt_internal_packages/ | ||
|
|
||
| # IDE files | ||
| .idea/ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| name: 'recurly' | ||
| version: '1.3.0' | ||
| version: '1.3.1' | ||
| config-version: 2 | ||
| require-dbt-version: [">=1.3.0", "<3.0.0"] | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| {{ config( | ||
| tags="fivetran_validations", | ||
| enabled=var('fivetran_validation_tests_enabled', false) | ||
| ) }} | ||
|
|
||
| -- this test ensures the recurly__monthly_recurring_revenue end model matches the prior version | ||
|
|
||
| with prod as ( | ||
|
|
||
| select | ||
| account_daily_id, | ||
| source_relation, | ||
| daily_charges, | ||
| daily_credits, | ||
| daily_discounts, | ||
| daily_net_change, | ||
| daily_taxes, | ||
| rolling_account_balance, | ||
| rolling_charge_balance, | ||
| rolling_credit_balance, | ||
| rolling_discount_balance, | ||
| rolling_tax_balance | ||
| from {{ target.schema }}_recurly_prod.recurly__account_daily_overview | ||
| ), | ||
|
|
||
| dev as ( | ||
|
|
||
| select | ||
| account_daily_id, | ||
| source_relation, | ||
| daily_charges, | ||
| daily_credits, | ||
| daily_discounts, | ||
| daily_net_change, | ||
| daily_taxes, | ||
| rolling_account_balance, | ||
| rolling_charge_balance, | ||
| rolling_credit_balance, | ||
| rolling_discount_balance, | ||
| rolling_tax_balance | ||
| from {{ target.schema }}_recurly_dev.recurly__account_daily_overview | ||
| ), | ||
|
|
||
|
|
||
| final as ( | ||
|
|
||
| select | ||
| prod.account_daily_id, | ||
| prod.source_relation, | ||
| prod.daily_charges as prod_daily_charges, | ||
| dev.daily_charges as dev_daily_charges, | ||
| prod.daily_credits as prod_daily_credits, | ||
| dev.daily_credits as dev_daily_credits, | ||
| prod.daily_discounts as prod_daily_discounts, | ||
| dev.daily_discounts as dev_daily_discounts, | ||
| prod.daily_net_change as prod_daily_change, | ||
| dev.daily_net_change as dev_daily_change, | ||
| prod.daily_taxes as prod_daily_taxes, | ||
| dev.daily_taxes as dev_daily_taxes, | ||
| prod.rolling_account_balance as prod_account_balance, | ||
| dev.rolling_account_balance as dev_account_balance, | ||
| prod.rolling_charge_balance as prod_charge_balance, | ||
| dev.rolling_charge_balance as dev_charge_balance, | ||
| prod.rolling_credit_balance as prod_credit_balance, | ||
| dev.rolling_credit_balance as dev_credit_balance, | ||
| prod.rolling_discount_balance as prod_discount_balance, | ||
| dev.rolling_discount_balance as dev_discount_balance, | ||
| prod.rolling_tax_balance as prod_tax_balance, | ||
| dev.rolling_tax_balance as dev_tax_balance | ||
|
|
||
| from prod | ||
| full outer join dev | ||
| on dev.account_daily_id = prod.account_daily_id | ||
| and dev.source_relation = prod.source_relation | ||
| ) | ||
|
|
||
| select * | ||
| from final | ||
| where abs(prod_account_balance - dev_account_balance) >= 0.01 | ||
| or abs(prod_charge_balance - dev_charge_balance) >= 0.01 | ||
| or abs(prod_credit_balance - dev_credit_balance) >= 0.01 | ||
| or abs(prod_discount_balance - dev_discount_balance) >= 0.01 | ||
| or abs(prod_tax_balance - dev_tax_balance) >= 0.01 | ||
| or abs(prod_daily_charges - dev_daily_charges) >= 0.01 | ||
| or abs(prod_daily_credits - dev_daily_credits) >= 0.01 | ||
| or abs(prod_daily_discounts - dev_daily_discounts) >= 0.01 | ||
| or abs(prod_daily_taxes - dev_daily_taxes) >= 0.01 | ||
| or abs(prod_daily_change - dev_daily_change) >= 0.01 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
| enabled=var('fivetran_validation_tests_enabled', false) | ||
| ) }} | ||
|
|
||
| {% set exclude_cols = var('consistency_test_exclude_metrics', []) %} | ||
| {% set exclude_cols = ['current_month_mrr', 'previous_month_mrr'] + var('consistency_test_exclude_metrics', []) %} | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment about if we really want to exclude these columns in future releases?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I created a separate consistency test to compare the amounts. Called this out more explicitly in the changelog. |
||
|
|
||
| -- this test ensures the recurly__monthly_recurring_revenue end model matches the prior version | ||
| with prod as ( | ||
|
|
@@ -45,4 +45,4 @@ final as ( | |
| ) | ||
|
|
||
| select * | ||
| from final | ||
| from final | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| {{ config( | ||
| tags="fivetran_validations", | ||
| enabled=var('fivetran_validation_tests_enabled', false) | ||
| ) }} | ||
|
|
||
| -- this test ensures the recurly__monthly_recurring_revenue end model matches the prior version | ||
|
|
||
| with prod as ( | ||
|
|
||
| select | ||
| account_monthly_id, | ||
| source_relation, | ||
| current_month_mrr, | ||
| previous_month_mrr | ||
| from {{ target.schema }}_recurly_prod.recurly__monthly_recurring_revenue | ||
| ), | ||
|
|
||
| dev as ( | ||
|
|
||
| select | ||
| account_monthly_id, | ||
| source_relation, | ||
| current_month_mrr, | ||
| previous_month_mrr | ||
| from {{ target.schema }}_recurly_dev.recurly__monthly_recurring_revenue | ||
| ), | ||
|
|
||
|
|
||
| final as ( | ||
|
|
||
| select | ||
| prod.account_monthly_id, | ||
| prod.source_relation, | ||
| prod.current_month_mrr as prod_current_month_mrr, | ||
| dev.current_month_mrr as dev_current_month_mrr, | ||
| prod.previous_month_mrr as prod_previous_month_mrr, | ||
| dev.previous_month_mrr as dev_previous_month_mrr | ||
| from prod | ||
| full outer join dev | ||
| on dev.account_monthly_id = prod.account_monthly_id | ||
| and dev.source_relation = prod.source_relation | ||
| ) | ||
|
|
||
| select * | ||
| from final | ||
| where abs(prod_current_month_mrr - dev_current_month_mrr) >= 0.01 | ||
| or abs(prod_previous_month_mrr - dev_previous_month_mrr) >= 0.01 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,17 +53,17 @@ current_vs_previous_mrr as ( | |
|
|
||
| mrr_type_enhanced as ( | ||
|
|
||
| select | ||
| select | ||
| *, | ||
| case when current_month_mrr > previous_month_mrr then 'expansion' | ||
| when current_month_mrr < previous_month_mrr then 'contraction' | ||
| when current_month_mrr = previous_month_mrr then 'unchanged' | ||
| case when round(cast(current_month_mrr as {{ dbt.type_numeric() }}), 2) > round(cast(previous_month_mrr as {{ dbt.type_numeric() }}), 2) then 'expansion' | ||
| when round(cast(current_month_mrr as {{ dbt.type_numeric() }}), 2) < round(cast(previous_month_mrr as {{ dbt.type_numeric() }}), 2) then 'contraction' | ||
| when round(cast(current_month_mrr as {{ dbt.type_numeric() }}), 2) = round(cast(previous_month_mrr as {{ dbt.type_numeric() }}), 2) then 'unchanged' | ||
| when previous_month_mrr is null then 'new' | ||
| when (current_month_mrr = 0.0 or current_month_mrr is null) | ||
| and (previous_month_mrr != 0.0) | ||
| then 'churned' | ||
| when (previous_month_mrr = 0.0 and current_month_mrr > 0.0 | ||
| and account_month_number >= 3) | ||
| when (previous_month_mrr = 0.0 and current_month_mrr > 0.0 | ||
| and account_month_number >= 3) | ||
|
Comment on lines
+56
to
+66
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it instead be more accurate to change the casts to float we apply in the staging model to be numeric in order to ensure consistent results as opposed to adjusting these case statements downstream. In this case, numeric would be a more accurate representation of the field(s) and we should apply that at the staging layer.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree changing numeric to float is probably the right approach at the staging layer. But that would increase the scope of this issue as all Recurly amounts are designed as floats and need to be updated across the model, so I opted for the band-aid. Should I add these in now or file a separate issue for future scoping? |
||
| then 'reactivation' | ||
| end as mrr_type | ||
| from current_vs_previous_mrr | ||
|
|
||
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.
Do we actually want to always exclude these? These seem like important columns to check in the future.
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.
Moved these tests over to a new consistency test designed to compare amounts. I missed some the first time around but applied them again now.