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

Merged deals #135

Merged
merged 28 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
029f7eb
add aggregate merged deals field and rm merged deals from final
fivetran-reneeli Feb 13, 2024
c90839d
update deps
fivetran-reneeli Feb 13, 2024
3afe3fb
update deal stage model
fivetran-reneeli Feb 13, 2024
406d4a3
explicit data type cast in int test, rm utc in seed
fivetran-reneeli Feb 13, 2024
24bff3e
add datatype casting for merged deal id too
fivetran-reneeli Feb 13, 2024
5be9c9c
reduce byte size?
fivetran-reneeli Feb 13, 2024
d4b39a0
mirror merged deal seed data as in source
fivetran-reneeli Feb 14, 2024
3450a24
try new schema and add dbt unique test config
fivetran-reneeli Feb 14, 2024
46cfed1
add remaining unique test configs
fivetran-reneeli Feb 14, 2024
0bc1eb8
changelog
fivetran-reneeli Feb 14, 2024
cdfef06
correct unique test to not null test
fivetran-reneeli Feb 16, 2024
2ba80e6
include merged_deal_enabled config and following documentation and te…
fivetran-reneeli Feb 16, 2024
568bcae
changelog update
fivetran-reneeli Feb 16, 2024
6f70dd1
try new schema
fivetran-reneeli Feb 16, 2024
6cff115
feature/quickstart-yml-add
fivetran-joemarkiewicz Feb 19, 2024
5a9f70a
removal of not needed dupes
fivetran-joemarkiewicz Feb 19, 2024
d5b611c
variable updates
fivetran-joemarkiewicz Feb 19, 2024
c0bd2e6
review notes
fivetran-reneeli Feb 19, 2024
6b229d7
Merge branch 'merged_deals' into feature/quickstart-yml-add
fivetran-reneeli Feb 20, 2024
8e6721b
Merge pull request #136 from fivetran/feature/quickstart-yml-add
fivetran-reneeli Feb 20, 2024
1a1fbeb
quickstart add
fivetran-reneeli Feb 20, 2024
3c930e8
new schema
fivetran-reneeli Feb 20, 2024
d13442f
oops didn't push
fivetran-reneeli Feb 20, 2024
80edb49
Update README.md
fivetran-reneeli Feb 21, 2024
5a39f63
Update models/sales/intermediate/int_hubspot__deals_enhanced.sql
fivetran-reneeli Feb 21, 2024
603204d
update deps
fivetran-reneeli Feb 21, 2024
b157c08
docs
fivetran-reneeli Feb 21, 2024
5be67aa
update changelog and docs
fivetran-reneeli Feb 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
# dbt_hubspot v0.15.2
fivetran-joemarkiewicz marked this conversation as resolved.
Show resolved Hide resolved
[PR #135](https://github.com/fivetran/dbt_hubspot/pull/135) includes the following updates:

## Features
- Updates the `hubspot__deal_stages` and `hubspot__deals` models to remove stale deals that have been merged since into other deals. In addition we've introduced a new field `merged_deal_ids` that lists all historic merged deals for each deal.

## Under the Hood
- Added tests for uniqueness for relevant keys on the condition they have not been deleted.

# dbt_hubspot v0.15.1
[PR #129](https://github.com/fivetran/dbt_hubspot/pull/129) includes the following updates:

Expand Down
3 changes: 2 additions & 1 deletion dbt_project.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: 'hubspot'
version: '0.15.1'
version: '0.15.2'

config-version: 2
require-dbt-version: [">=1.3.0", "<2.0.0"]
Expand All @@ -13,6 +13,7 @@ vars:
company: "{{ ref('stg_hubspot__company') }}"
company_property_history: "{{ ref('stg_hubspot__company_property_history') }}"
deal: "{{ ref('stg_hubspot__deal') }}"
merged_deal: "{{ ref('stg_hubspot__merged_deal') }}"
deal_stage: "{{ ref('stg_hubspot__deal_stage') }}"
deal_company: "{{ ref('stg_hubspot__deal_company') }}"
deal_pipeline: "{{ ref('stg_hubspot__deal_pipeline') }}"
Expand Down
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: hubspot_integration_tests_8
schema: hubspot_integration_tests_9
threads: 8
bigquery:
type: bigquery
method: service-account-json
project: 'dbt-package-testing'
schema: hubspot_integration_tests_8
schema: hubspot_integration_tests_9
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: hubspot_integration_tests_8
schema: hubspot_integration_tests_9
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: hubspot_integration_tests_8
schema: hubspot_integration_tests_9
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: hubspot_integration_tests_8
schema: hubspot_integration_tests_9
threads: 8
token: "{{ env_var('CI_DATABRICKS_DBT_TOKEN') }}"
type: databricks
9 changes: 7 additions & 2 deletions integration_tests/dbt_project.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
name: 'hubspot_integration_tests'
version: '0.15.1'
version: '0.15.2'

profile: 'integration_tests'
config-version: 2
vars:
hubspot_schema: hubspot_integration_tests_8
hubspot_schema: hubspot_integration_tests_9
hubspot_source:
hubspot_service_enabled: true
# hubspot_sales_enabled: true # enable when generating docs
Expand Down Expand Up @@ -58,6 +58,7 @@ vars:
hubspot_contact_list_identifier: "contact_list_data"
hubspot_email_event_sent_identifier: "email_event_sent_data"
hubspot_email_event_dropped_identifier: "email_event_dropped_data"
hubspot_merged_deal_identifier: "merged_deal_data"

seeds:
hubspot_integration_tests:
Expand Down Expand Up @@ -173,6 +174,10 @@ seeds:
ticket_property_history_data:
+column_types:
timestamp_instant: timestamp
merged_deal_data:
+column_types:
deal_id: "{{ 'int64' if target.type == 'bigquery' else 'bigint' }}"
merged_deal_id: "{{ 'int64' if target.type == 'bigquery' else 'bigint' }}"

dispatch:
- macro_namespace: dbt_utils
Expand Down
15 changes: 15 additions & 0 deletions integration_tests/seeds/merged_deal_data.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
deal_id,merged_deal_id,_fivetran_synced
9089908390,9014070905,2023-06-22 11:37:07.210000
9089908390,9077310858,2023-06-22 11:37:07.210000
9089908390,9077311778,2023-06-22 11:37:07.210000
9089908390,9014071335,2023-06-22 11:37:07.210000
1364146343,10829010820,2023-06-22 11:37:07.220000
1364146343,10828874842,2023-06-22 11:37:07.220000
1368273549,10829051678,2023-06-22 11:37:07.227000
1368273549,10829028256,2023-06-22 11:37:07.227000
1373595330,10879517198,2023-06-22 11:37:07.227000
1373595330,10828874855,2023-06-22 11:37:07.227000
1006463008,10064581040,2023-06-22 11:37:07.213000
1006463008,9351825260,2023-06-22 11:37:07.213000
1389291069,12351788138,2023-06-28 11:18:32.940000
1389291069,13817889862,2023-06-28 11:18:32.939000
33 changes: 33 additions & 0 deletions models/marketing/email_events/email_events.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ models:
description: The ID of the event.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_deleted, false)"

- name: returned_response
description: The full response from the recipient's email server.
Expand Down Expand Up @@ -65,6 +68,9 @@ models:
description: The ID of the event.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_deleted, false)"

- name: geo_location
description: '{{ doc("email_event_location") }}'
Expand Down Expand Up @@ -112,6 +118,9 @@ models:
description: The ID of the event.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_deleted, false)"

- name: returned_response
description: The full response from the recipient's email server.
Expand Down Expand Up @@ -147,6 +156,9 @@ models:
description: The ID of the event.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_deleted, false)"

- name: returned_response
description: The full response from the recipient's email server.
Expand Down Expand Up @@ -200,6 +212,9 @@ models:
description: The ID of the event.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_deleted, false)"

- name: from_email
description: The 'from' field of the email message.
Expand Down Expand Up @@ -241,6 +256,9 @@ models:
description: The ID of the event.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_deleted, false)"

- name: geo_location
description: '{{ doc("email_event_location") }}'
Expand Down Expand Up @@ -288,6 +306,9 @@ models:
description: The ID of the event.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_deleted, false)"

- name: geo_location
description: '{{ doc("email_event_location") }}'
Expand Down Expand Up @@ -332,6 +353,9 @@ models:
description: The ID of the event.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_deleted, false)"

- name: geo_location
description: '{{ doc("email_event_location") }}'
Expand Down Expand Up @@ -382,6 +406,9 @@ models:
description: The ID of the event.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_deleted, false)"

- name: from_email
description: The 'from' field of the email message.
Expand Down Expand Up @@ -420,6 +447,9 @@ models:
description: The ID of the event.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_deleted, false)"

- name: ip_address
description: '{{ doc("email_event_ip_address") }}'
Expand Down Expand Up @@ -461,6 +491,9 @@ models:
description: The ID of the event.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_deleted, false)"

- name: is_bounced
description: |
Expand Down
9 changes: 9 additions & 0 deletions models/marketing/marketing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ models:
description: The ID of the event.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_deleted, false)"
- name: from_email
description: The 'from' field of the email message.
- name: reply_to_email
Expand Down Expand Up @@ -154,6 +157,9 @@ models:
description: The ID of the contact.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_deleted, false)"
- name: contact_company
description: The name of the contact's company
- name: first_name
Expand Down Expand Up @@ -237,6 +243,9 @@ models:
description: The ID of the contact list.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_contact_list_deleted, false)"
- name: contact_list_name
description: The name of the contact list.
- name: is_contact_list_deleted
Expand Down
1 change: 1 addition & 0 deletions models/sales/hubspot__deal_stages.sql
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ with deals_enhanced as (
deal_stage.deal_id || '-' || row_number() over(partition by deal_stage.deal_id order by deal_stage.date_entered asc) as deal_stage_id,
deals_enhanced.deal_id,
deals_enhanced.deal_name,
deals_enhanced.merged_deal_ids,
fivetran-joemarkiewicz marked this conversation as resolved.
Show resolved Hide resolved
deal_stage._fivetran_start as date_stage_entered,
deal_stage._fivetran_end as date_stage_exited,
deal_stage._fivetran_active as is_stage_active,
Expand Down
21 changes: 21 additions & 0 deletions models/sales/intermediate/int_hubspot__deals_enhanced.sql
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@ with deals as (
select *
from {{ var('deal') }}

), merged_deals as (

select *
from {{ var('merged_deal')}}

), aggregate_merged_deals as (

select
deal_id,
{{ fivetran_utils.array_agg("merged_deal_id") }} as merged_deal_ids

fivetran-reneeli marked this conversation as resolved.
Show resolved Hide resolved
from {{ var('merged_deal')}}
group by 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, following the conversation from the source PR we may want to add a conditional to the merged deal components if we do decide to leverage a variable to enable/disable these.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Included hubspot_merged_deal_enabled config


fivetran-joemarkiewicz marked this conversation as resolved.
Show resolved Hide resolved
), pipelines as (

select *
Expand All @@ -26,6 +40,7 @@ with deals as (

select
deals.*,
aggregate_merged_deals.merged_deal_ids,
coalesce(pipelines.is_deal_pipeline_deleted, false) as is_deal_pipeline_deleted,
pipelines.pipeline_label,
pipelines.is_active as is_pipeline_active,
Expand All @@ -44,11 +59,17 @@ with deals as (
on deals.deal_pipeline_id = pipelines.deal_pipeline_id
left join pipeline_stages
on deals.deal_pipeline_stage_id = pipeline_stages.deal_pipeline_stage_id
left join aggregate_merged_deals
on deals.deal_id = aggregate_merged_deals.deal_id
left join merged_deals
on deals.deal_id = merged_deals.merged_deal_id
fivetran-joemarkiewicz marked this conversation as resolved.
Show resolved Hide resolved

{% if var('hubspot_owner_enabled', true) %}
left join owners
on deals.owner_id = owners.owner_id
{% endif %}

where merged_deals.merged_deal_id is null -- remove deals that have been merged
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this true? I don't imagine the merged_deals table will contain all deal_ids and then if it has no merged deals then it will be null for the merged_deal_id column. I would assume this table is only populated with deals that have been merged. Therefore, if a deal has not been merged I would assume it would not be present in this table.

If the above is the case then this logic would not be accurate for filtering out merged deals.

Copy link
Contributor

Choose a reason for hiding this comment

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

When looking at the underlying data I see that there do in fact seem to be more deals in the deal table than exist in the merged_deal table. Given this, I believe this logic needs to be adjusted to accurately filter out merged deals.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Also is filtering out the null records the correct way to filter out merged accounts? Wouldn't we want to cross reference which deals are present in the merged_deal_id column and then when there are any matches that is what we filter out?

Let me know if I am missing a component, but I am not sure if filtering where the merged_deal_id is null is the right way to achieve this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh goodness yes I confused how I was looking at the table-- right not all deals exist in the merged_deals. Therefore to correctly filter out deals in the final models that have been merged, we would need to do a check for if deal_id is a merged_deal_id.

I updated it to the following:

where deals.deal_id not in (select merged_deal_id from merged_deals)

)

select *
Expand Down
9 changes: 9 additions & 0 deletions models/sales/sales.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ models:
description: The ID of the deal
tests:
- unique
fivetran-joemarkiewicz marked this conversation as resolved.
Show resolved Hide resolved
- unique:
config:
where: "not coalesce(is_deal_deleted, false)"
- name: deal_name
description: The name you have given this deal.
- name: is_deal_deleted
Expand Down Expand Up @@ -71,6 +74,9 @@ models:
description: The unique deal stage identifier.
tests:
- unique
fivetran-joemarkiewicz marked this conversation as resolved.
Show resolved Hide resolved
- unique:
config:
where: "not coalesce(is_deal_pipeline_stage_deleted, false)"
- name: is_deal_pipeline_deleted
description: '{{ doc("is_deleted") }}'
- name: is_deal_pipeline_stage_deleted
Expand Down Expand Up @@ -125,6 +131,9 @@ models:
description: The ID of the company.
tests:
- not_null
- unique:
config:
where: "not coalesce(is_company_deleted, false)"
- name: portal_id
description: '{{ doc("portal_id") }}'
- name: is_company_deleted
Expand Down
8 changes: 6 additions & 2 deletions packages.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
packages:
- package: fivetran/hubspot_source
version: [">=0.14.0", "<0.15.0"]
# - package: fivetran/hubspot_source
# version: [">=0.14.0", "<0.15.0"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Reminder to update before merge


- git: https://github.com/fivetran/dbt_hubspot_source.git
revision: merged_deals
warn-unpinned: false
Loading