fivetran / dbt_hubspot

Data models for Hubspot built using dbt.
https://fivetran.github.io/dbt_hubspot/
Apache License 2.0
33 stars 38 forks source link

[Bug] Merged deals don't get remove in final models #134

Closed codingcyclist closed 6 months ago

codingcyclist commented 7 months ago

Is there an existing issue for this?

Describe the issue

image

Relevant error log or model output

No response

Expected behavior

Adopt the same behavior as for contacts: merged deals should get removed in final models (using the merged_deal table) and the IDs of merged deals should get collapsed into a calculated_merged_vids column

dbt Project configurations

Running with dbt=1.7.4
dbt version: 1.7.4
python version: 3.10.12
python path: /home/vscode/venv/bin/python
os info: Linux-5.15.49-linuxkit-aarch64-with-glibc2.31
Using profiles dir at /workspace/dbt
Using profiles.yml file at /workspace/dbt/profiles.yml
Using dbt_project.yml file at /workspace/dbt/dbt_project.yml
adapter type: snowflake
adapter version: 1.7.1
Configuration:
  profiles.yml file [OK found and valid]
  dbt_project.yml file [OK found and valid]
Required dependencies:
 - git [OK found]

Package versions

packages:
  - package: dbt-labs/dbt_external_tables
    version: "0.8.5"
  - package: fivetran/hubspot
    version: [">=0.15.0", "<0.16.0"]
  - package: dbt-labs/dbt_utils
    version: "1.1.1"
  - package: calogica/dbt_expectations
    version: [">=0.10.0", "<0.11.0"]

What database are you using dbt with?

snowflake

dbt Version

1.7.4

Additional Context

No response

Are you willing to open a PR to help address this issue?

fivetran-reneeli commented 7 months ago

Thanks @codingcyclist for noticing this! I've dug around and just want to make sure we're on the same page:

There is a merged_deal table that should be added in our package so that we can create a field such as 'is_merged' in the end deal model so you can filter out the merged (stale ) deals.

image

So for the example above, the 1st column deal_id is the deal that the 2nd column merged_deal_id has been merged into.

Vice versa, it would also be helpful to introduce a new column such as 'calculated_merged_deal_ids' that will aggregate all deals that have merged, for each record that it's been merged into.

Let me know if I'm missing anything!

codingcyclist commented 7 months ago

Yes, I think we're generally on the same page here. With that said, though, I have one minor comment

so that we can create a field such as 'is_merged' in the end deal model so you can filter out the merged (stale ) deals.

I think the merged deals should be filtered out automatically in the end model and the IDs of merged deals should be collapsed into a calculated_merged_vids column. This is how it's done already today for contacts (see screenshot) image

fivetran-reneeli commented 7 months ago

Ah I see your point. Filtering out individual deals if they're already merged and aggregating them in 1 record; that way we'll have a continuous history for these rather them going stale. Great, I believe I got the scope of this ticket, will look into adding this to an future sprint for our team!

fivetran-reneeli commented 6 months ago

Hi @codingcyclist, we've been able to work on the issue of merged deals. If you're interested in trying it out before we release it, see the branch below to paste into your packages.yml. Keen to see what you think!


    - git: https://github.com/fivetran/dbt_hubspot.git
      revision: merged_deals
      warn-unpinned: false
fivetran-reneeli commented 6 months ago

Hi @codingcyclist , we have merged this into the main branch! Let us know if this suits your needs and if you have any further questions! Will close this out for now

codingcyclist commented 6 months ago

Thanks a lot for the quick turnaround! I really appreciate it