-
Notifications
You must be signed in to change notification settings - Fork 39
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
updating models #92
updating models #92
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some in-line comments! mostly docs-focused except the int_hubspot__email_metrics_by_contact_list
bit
models/marketing/intermediate/int_hubspot__email_metrics__by_contact_list.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks pretty good -- found a couple of fields missing docs and still getting failures on uniqueness tests on email event models tho
packages.yml
Outdated
# - package: fivetran/hubspot_source | ||
# version: [">=0.7.0", "<0.8.0"] | ||
- git: https://github.com/fivetran/dbt_hubspot_source.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
obligatory reminder to update before merging
@@ -323,11 +323,8 @@ models: | |||
- name: hubspot__contact_lists | |||
description: Each record represents a contact list in Hubspot. | |||
columns: | |||
- name: _fivetran_deleted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to add is_contact_list_deleted
right?
@@ -206,11 +206,11 @@ models: | |||
- name: hubspot__contacts | |||
description: Each record represents a contact in Hubspot. | |||
columns: | |||
- name: _fivetran_deleted | |||
description: '{{ doc("_fivetran_deleted") }}' | |||
- name: is_contact_deleted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think hubspot__email_sends
is missing this
[PR #92](https://github.com/fivetran/dbt_hubspot/pull/92) incorporates the following changes: | ||
|
||
- The below models/macros have new soft deleted record flags **explicitly** added to them: | ||
- `is_contact_deleted` added in `macros/email_events_joined` which can affect the following downstream models found in `models/marketing/email_events/`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hubspot__email_sends
as well i think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approving now that i see the dbt test failures on main as well... that's another issue we should investigate outside of this PR. so once docs are updated lgtm
Are you a current Fivetran customer?
Fivetran employee
What change(s) does this PR introduce?
Features
Soft deleted records are now included in the below models/macros:
macros/email_events_joined
models/marketing/intermediate/int_hubspot__email_metrics__by_contact_list
models/sales/intermediate/int_hubspot__deals_enhanced
models/sales/deal_stages
Soft deleted records can now be removed by using the following respective flags for the above models:
is_contact_list_member_deleted
is_deal_pipeline_deleted
is_deal_pipeline_stage_deleted
is_deal_deleted
Did you update the CHANGELOG?
Does this PR introduce a breaking change?
Did you update the dbt_project.yml files with the version upgrade (please leverage standard semantic versioning)? (In both your main project and integration_tests)
Is this PR in response to a previously created Bug or Feature Request
How did you test the PR changes?
Select which warehouse(s) were used to test the PR
Provide an emoji that best describes your current mood
💃
Feedback
We are so excited you decided to contribute to the Fivetran community dbt package! We continue to work to improve the packages and would greatly appreciate your feedback on our existing dbt packages or what you'd like to see next.