-
Notifications
You must be signed in to change notification settings - Fork 40
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
update hubspot_deal_changes description #132
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.
update looks good to me! would like to confirm with joe as well
README.md
Outdated
@@ -33,7 +33,7 @@ The following table provides a detailed list of all models materialized within t | |||
| [hubspot__contact_history](https://fivetran.github.io/dbt_hubspot/#!/model/model.hubspot.hubspot__contact_history) | Each record represents a change to a contact in Hubspot, with `valid_to` and `valid_from` information. | | |||
| [hubspot__contact_lists](https://fivetran.github.io/dbt_hubspot/#!/model/model.hubspot.hubspot__contact_lists) | Each record represents a contact list in Hubspot, enriched with metrics about email activities. | | |||
| [hubspot__deals](https://fivetran.github.io/dbt_hubspot/#!/model/model.hubspot.hubspot__deals) | Each record represents a deal in Hubspot, enriched with metrics about engagement activities. | | |||
| [hubspot__deal_stages](https://fivetran.github.io/dbt_hubspot/#!/model/model.hubspot.hubspot__deal_stages) | Each record represents a deal stage in Hubspot, enriched with metrics deal activities. | | |||
| [hubspot__deal_stages](https://fivetran.github.io/dbt_hubspot/#!/model/model.hubspot.hubspot__deal_stages) | Each record represents when a deal stage changes in Hubspot, enriched with metrics deal activities. | |
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.
@fivetran-joemarkiewicz this update seems correct to me based on our internal data (and the fact that the deal_stage
source table has _fivetran_start
and _fivetran_end
columns)
^ this implies that a deal can re-enter the same stage, which is reflected better with the new wording. we probably want to udpate the model description docs (ie here) as well
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.
Yeah I agree with you @fivetran-jamie! We should also update the model description as well within this same PR to be consistent with out documentation.
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.
great, i just pushed the changes to this PR if you (or i can tag someone else in) could take a quick look
note: i didn't totally regen the docs because i realized i could directly tweak the model description in the manifest.json 🌚
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.
LGTM
A user suggested this change through the docs site's feedback mechanism. Please confirm that it is correct - if not, feel free to close this PR.