Skip to content
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

line item enhanced updates #98

Merged
merged 11 commits into from
Dec 10, 2024
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
# dbt_stripe v0.15.1

## Bug Fixes
- Updated the logic in `stripe__line_item_enhanced` to properly bring in refund data by adjusting the joins on balance transactions, refunds and charges.
- Since charges and refunds are both types of balance transactions, included an additional join between refunds and balance transactions to bring in refunds at the same level as charges.
- Fixed `fee_amount` logic to sum together charge and refund amounts.
- Updated `transaction_type` logic to not only bring in `refund`, but return `charge+refund` if the balance transaction has a charge and a refund associated with it.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Updated `transaction_type` logic to not only bring in `refund`, but return `charge+refund` if the balance transaction has a charge and a refund associated with it.
- Updated `transaction_type` logic to not only bring in `charge`, but also return `charge + refund` if the balance transaction has a charge and a refund associated with it.

Unless we also want to return refund?

Copy link
Contributor Author

@fivetran-avinash fivetran-avinash Nov 26, 2024

Choose a reason for hiding this comment

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

Fixed.

I don't believe a refund can exist in Stripe without a charge so for now won't add refund in (documentation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding payment intent + refund as a transaction type based on Jamie's research.

- Updated conditional logic for invoice-only records to bring in only non-zero rather than not null `fee_amounts`, as summing charge and refund amounts together brings in only not null `fee_amount` values.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Updated conditional logic for invoice-only records to bring in only non-zero rather than not null `fee_amounts`, as summing charge and refund amounts together brings in only not null `fee_amount` values.
- Updated conditional logic for invoice-only records to bring in only non-zero rather than not null `fee_amount`, as summing charge and refund amounts together brings in only not null `fee_amount` values.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the update about the filter or about us coalescing fee_amount with 0 now for non-header rows?

Copy link
Contributor Author

@fivetran-avinash fivetran-avinash Nov 26, 2024

Choose a reason for hiding this comment

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

Both. Before it was possible for fee_amount to be null because of the left join. Because we are summing, summing a null with a not null would make the value null, which is not what we would want. So I coalesced to zero, meaning it could only be zero or not zero, so the filter had to change too.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, then I would make it clearer that we added a coalesce with 0 for non-header rows before getting into this filter logic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, cleaned up the CHANGELOG.


## Under the Hood
- Modified the consistency tests to better compare differences between production and development rows.

# dbt_stripe v0.15.0

## Breaking Changes
Expand Down
2 changes: 1 addition & 1 deletion dbt_project.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
config-version: 2
name: 'stripe'

version: '0.15.0'
version: '0.15.1'
require-dbt-version: [">=1.3.0", "<2.0.0"]
models:
stripe:
Expand Down
2 changes: 1 addition & 1 deletion docs/catalog.json

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions docs/index.html

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion docs/manifest.json

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions integration_tests/ci/sample.profiles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ integration_tests:
pass: "{{ env_var('CI_REDSHIFT_DBT_PASS') }}"
dbname: "{{ env_var('CI_REDSHIFT_DBT_DBNAME') }}"
port: 5439
schema: stripe_integrations_tests_14
schema: stripe_integrations_tests_15
threads: 8
bigquery:
type: bigquery
method: service-account-json
project: 'dbt-package-testing'
schema: stripe_integrations_tests_14
schema: stripe_integrations_tests_15
threads: 8
keyfile_json: "{{ env_var('GCLOUD_SERVICE_KEY') | as_native }}"
snowflake:
Expand All @@ -33,7 +33,7 @@ integration_tests:
role: "{{ env_var('CI_SNOWFLAKE_DBT_ROLE') }}"
database: "{{ env_var('CI_SNOWFLAKE_DBT_DATABASE') }}"
warehouse: "{{ env_var('CI_SNOWFLAKE_DBT_WAREHOUSE') }}"
schema: stripe_integrations_tests_14
schema: stripe_integrations_tests_15
threads: 8
postgres:
type: postgres
Expand All @@ -42,13 +42,13 @@ integration_tests:
pass: "{{ env_var('CI_POSTGRES_DBT_PASS') }}"
dbname: "{{ env_var('CI_POSTGRES_DBT_DBNAME') }}"
port: 5432
schema: stripe_integrations_tests_14
schema: stripe_integrations_tests_15
threads: 8
databricks:
catalog: "{{ env_var('CI_DATABRICKS_DBT_CATALOG') }}"
host: "{{ env_var('CI_DATABRICKS_DBT_HOST') }}"
http_path: "{{ env_var('CI_DATABRICKS_DBT_HTTP_PATH') }}"
schema: stripe_integrations_tests_14
schema: stripe_integrations_tests_15
threads: 8
token: "{{ env_var('CI_DATABRICKS_DBT_TOKEN') }}"
type: databricks
4 changes: 2 additions & 2 deletions integration_tests/dbt_project.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
config-version: 2

name: 'stripe_integration_tests'
version: '0.15.0'
version: '0.15.1'

profile: 'integration_tests'

Expand All @@ -10,7 +10,7 @@ models:
+schema: "stripe_{{ var('directed_schema','dev') }}"

vars:
stripe_schema: stripe_integrations_tests_14
stripe_schema: stripe_integrations_tests_15
stripe__standardized_billing_model_enabled: true
stripe__using_credit_notes: true
stripe_source:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,42 @@
) }}

with prod as (
select balance_transaction_id, dispute_id as dispute_ids -- we don't have multi-dispute records in our data
select balance_transaction_id, dispute_ids
from {{ target.schema }}_stripe_prod.stripe__activity_itemized_2
),

dev as (
select balance_transaction_id, dispute_ids
from {{ target.schema }}_stripe_dev.stripe__activity_itemized_2
),

prod_not_in_dev as (
-- rows from prod not found in dev
select * from prod
except distinct
select * from dev
),

dev_not_in_prod as (
-- rows from dev not found in prod
select * from dev
except distinct
select * from prod
),

final as (
select
*,
'from prod' as source
from prod_not_in_dev

union all -- union since we only care if rows are produced

select
*,
'from dev' as source
from dev_not_in_prod
)

-- test will return values and fail if the values are different (which they shouldn't be in our test data)
select *
from prod
join dev
on prod.balance_transaction_id = dev.balance_transaction_id
where prod.dispute_ids != dev.dispute_ids
from final
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,42 @@
) }}

with prod as (
select balance_transaction_id, dispute_reason as dispute_reasons -- we don't have multi-dispute records in our data
select balance_transaction_id, dispute_reasons
from {{ target.schema }}_stripe_prod.stripe__balance_change_from_activity_itemized_3
),

dev as (
select balance_transaction_id, dispute_reasons
from {{ target.schema }}_stripe_dev.stripe__balance_change_from_activity_itemized_3
),

prod_not_in_dev as (
-- rows from prod not found in dev
select * from prod
except distinct
select * from dev
),

dev_not_in_prod as (
-- rows from dev not found in prod
select * from dev
except distinct
select * from prod
),

final as (
select
*,
'from prod' as source
from prod_not_in_dev

union all -- union since we only care if rows are produced

select
*,
'from dev' as source
from dev_not_in_prod
)

-- test will return values and fail if the values are different (which they shouldn't be in our test data)
select *
from prod
join dev
on prod.balance_transaction_id = dev.balance_transaction_id
where prod.dispute_reasons != dev.dispute_reasons
from final
34 changes: 24 additions & 10 deletions integration_tests/tests/consistency/consistency_daily_overview.sql
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,35 @@ with
from {{ target.schema }}_stripe_{{ prod_or_dev }}.stripe__daily_overview
group by 1,2 -- need to group to remove randomization stemming from rolling totals
),
{% endfor %}
{% endfor %}

final as (
-- test will fail if any rows from prod are not found in dev
(select * from prod
prod_not_in_dev as (
-- rows from prod not found in dev
select * from prod
except distinct
select * from dev
),

dev_not_in_prod as (
-- rows from dev not found in prod
select * from dev
except distinct
select * from dev)
select * from prod
),

final as (
select
*,
'from prod' as source
from prod_not_in_dev

union all -- union since we only care if rows are produced

-- test will fail if any rows from dev are not found in prod
(select * from dev
except distinct
select * from prod)
)
select
*,
'from dev' as source
from dev_not_in_prod
)

select *
from final
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,42 @@
) }}

with prod as (
select balance_transaction_id, dispute_reason as dispute_reasons -- we don't have multi-dispute records in our data
select balance_transaction_id, dispute_reasons
from {{ target.schema }}_stripe_prod.stripe__ending_balance_reconciliation_itemized_4
),

dev as (
select balance_transaction_id, dispute_reasons
from {{ target.schema }}_stripe_dev.stripe__ending_balance_reconciliation_itemized_4
),

prod_not_in_dev as (
-- rows from prod not found in dev
select * from prod
except distinct
select * from dev
),

dev_not_in_prod as (
-- rows from dev not found in prod
select * from dev
except distinct
select * from prod
),

final as (
select
*,
'from prod' as source
from prod_not_in_dev

union all -- union since we only care if rows are produced

select
*,
'from dev' as source
from dev_not_in_prod
)

-- test will return values and fail if the values are different (which they shouldn't be in our test data)
select *
from prod
join dev
on prod.balance_transaction_id = dev.balance_transaction_id
where prod.dispute_reasons != dev.dispute_reasons
from final
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,43 @@

with prod as (
select *
except(fee_amount, refund_amount, transaction_type) --remove before merge
from {{ target.schema }}_stripe_prod.stripe__line_item_enhanced
),

dev as (
select *
except(fee_amount, refund_amount, transaction_type) --remove before merge
from {{ target.schema }}_stripe_dev.stripe__line_item_enhanced
),
),

final as (
-- test will fail if any rows from prod are not found in dev
(select * from prod
prod_not_in_dev as (
-- rows from prod not found in dev
select * from prod
except distinct
select * from dev)
select * from dev
),

dev_not_in_prod as (
-- rows from dev not found in prod
select * from dev
except distinct
select * from prod
),

final as (
select
*,
'from prod' as source
from prod_not_in_dev

union all -- union since we only care if rows are produced

-- test will fail if any rows from dev are not found in prod
(select * from dev
except distinct
select * from prod)
)
select
*,
'from dev' as source
from dev_not_in_prod
)

select *
from final
Loading