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

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

Closed
2 of 4 tasks
codingcyclist opened this issue Jan 24, 2024 · 6 comments · Fixed by #135
Closed
2 of 4 tasks

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

codingcyclist opened this issue Jan 24, 2024 · 6 comments · Fixed by #135
Labels
error:unforced status:scoping Currently being scoped type:enhancement New functionality or enhancement

Comments

@codingcyclist
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the issue

image
  • [expected] Merged contacts automatically get removed and collapsed into calculated_merged_vids in the final models (using the CONTACT.property_hs_calculated_merged_vids column)
  • [expected] Merged companies can be manually filtered from final models, using the is_deleted column
  • [unexpected] Merged deals don't get removed in final models and can't be filtered, which causes deals to still stick around even though they are "stale"

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?

  • Yes.
  • Yes, but I will need assistance and will schedule time during our office hours for guidance
  • No.
@fivetran-reneeli
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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 fivetran-reneeli added type:enhancement New functionality or enhancement status:scoping Currently being scoped labels Jan 29, 2024
This was referenced Feb 14, 2024
@fivetran-reneeli fivetran-reneeli linked a pull request Feb 21, 2024 that will close this issue
18 tasks
@fivetran-reneeli
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error:unforced status:scoping Currently being scoped type:enhancement New functionality or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants