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

Continuation of #88 - handle multiple disputes in stripe__balance_transactions #92

Merged
merged 11 commits into from
Oct 3, 2024

Conversation

fivetran-jamie
Copy link
Contributor

@fivetran-jamie fivetran-jamie commented Sep 30, 2024

PR Overview

This PR will address the following Issue/Feature:
#87

Includes merging of PR #88

This PR will result in the following new package version:

v0.15.0

Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:

🚨 Breaking Changes 🚨

  • Updated stripe__balance_transactions to correctly handle multiple disputes on the same transaction:
    • Adjusted customer_facing_amount to reflect the dispute_amount of the latest dispute (if the transaction is not a charge or refund and is associated with any disputes).
    • Added the following the dispute-related columns:
      • latest_dispute_amount_won: Latest disputed amount that was won in favor of the merchant.
      • latest_dispute_amount_lost: Latest disputed amount that was lost and returned to the customer.
      • latest_dispute_amount_under_review: Latest disputed amount that is currently under review by the bank.
      • latest_dispute_amount_needs_response: Latest disputed amount that currently needs a response (the dispute has been filed but the merchant has not yet responded with evidence).
      • latest_dispute_amount_warning_closed: Latest disputed amount that is currently of status warning_under_closed (early fraud warning being closed due to no formal dispute).
      • latest_dispute_amount_warning_under_review: Latest disputed amount that is currently of status warning_under_review (card issuer suspects possible fraud but hasn't yet escalated the situation to a full dispute).
      • latest_dispute_amount_warning_needs_response: Latest disputed amount that is currently of status warning_needs_response (early fraud warning has been escalated into formal dispute and/or card issuer has requested more information).
      • dispute_count: Count of disputes raised against this transaction. If > 1, join in dispute data for additional information on each dispute.
    • Adjusted the dispute_id and dispute_reason fields to aggregate together data from multiple disputes if present. They have been renamed to dispute_ids and dispute_reasons in the following models (PR #88):
      • stripe__balance_transactions
      • stripe__activity_itemized_2
      • stripe__balance_change_from_activity_itemized_3
      • stripe__ending_balance_reconciliation_itemized_4

Under the Hood

  • Updated dispute seed data to test the above changes.

PR Checklist

Basic Validation

Please acknowledge that you have successfully performed the following commands locally:

  • dbt run –full-refresh && dbt test
  • dbt run (if incremental models are present) && dbt test

Before marking this PR as "ready for review" the following have been applied:

  • The appropriate issue has been linked, tagged, and properly assigned
  • All necessary documentation and version upgrades have been applied
  • docs were regenerated (unless this PR does not include any code or yml updates)
  • BuildKite integration tests are passing
  • Detailed validation steps have been provided below

Detailed Validation

Please share any and all of your validation steps:

New Validation tests passing on our internal data.
image

If you had to summarize this PR in an emoji, which would it be?

🥊

bramrodenburg and others added 3 commits September 27, 2024 17:09
* Documentation Standard Updates (#85)

* MagicBot/documentation-updates

* Apply suggestions from code review

* Apply suggestions from code review

---------

Co-authored-by: Jamie Rodriguez <[email protected]>

* fix!: handle multiple disputes

* replace array_agg with string_agg

* update changelog

* bump version

---------

Co-authored-by: Joe Markiewicz <[email protected]>
Co-authored-by: Jamie Rodriguez <[email protected]>
@fivetran-jamie fivetran-jamie self-assigned this Sep 30, 2024
@fivetran-jamie
Copy link
Contributor Author

@bramrodenburg @jsnorthrup Here's my working branch if you've got a chance to test it out! Thanks

packages:
- git: https://github.com/fivetran/dbt_stripe.git
  revision: releases/v0.15.latest

Copy link
Contributor

@fivetran-reneeli fivetran-reneeli left a comment

Choose a reason for hiding this comment

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

Thanks Jamie! Had some comments!

CHANGELOG.md Outdated Show resolved Hide resolved
models/stripe.yml Outdated Show resolved Hide resolved
models/stripe.yml Outdated Show resolved Hide resolved
models/stripe__balance_transactions.sql Outdated Show resolved Hide resolved
models/stripe__balance_transactions.sql Outdated Show resolved Hide resolved
models/stripe__balance_transactions.sql Outdated Show resolved Hide resolved
Copy link
Contributor

@fivetran-reneeli fivetran-reneeli left a comment

Choose a reason for hiding this comment

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

looks good!

Copy link

@fivetran-avinash fivetran-avinash left a comment

Choose a reason for hiding this comment

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

@fivetran-jamie A few comments before approval

CHANGELOG.md Outdated
@@ -1,3 +1,31 @@
# dbt_stripe v0.15.0

## 🚨 Breaking Changes 🚨

Choose a reason for hiding this comment

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

I don't think we can do emojis in Changelogs/releases anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😢 you're right

CHANGELOG.md Outdated Show resolved Hide resolved
integration_tests/seeds/dispute_data.csv Outdated Show resolved Hide resolved
models/stripe.yml Show resolved Hide resolved
models/stripe.yml Show resolved Hide resolved
Copy link

@fivetran-avinash fivetran-avinash left a comment

Choose a reason for hiding this comment

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

@fivetran-jamie fivetran-jamie merged commit 7cfdfc5 into main Oct 3, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants