Add new category field #132

fivetran-reneeli · 2024-11-01T19:33:29Z

PR Overview

This PR will address the following Issue/Feature:
#131 , and internal ticket

This PR will result in the following new package version:

v0.17.0

Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:

Breaking Change

Introduced a new category column to the below models. This association field differentiates records by either HUBSPOT_DEFINED (default label) or USER_DEFINED (custom label) and was introduced to the connector in October 2024.
- stg_hubspot__deal_company
- stg_hubspot__deal_contact
- stg_hubspot__merged_deal
- stg_hubspot__engagement_company
- stg_hubspot__engagement_contact
- stg_hubspot__engagement_deal
- stg_hubspot__ticket_company
- stg_hubspot__ticket_contact
- stg_hubspot__ticket_deal
- stg_hubspot__ticket_engagement
The category field which will be a part of the uniqueness tests for the following models:
- stg_hubspot__deal_contact
- stg_hubspot__deal_company

PR Checklist

Basic Validation

Please acknowledge that you have successfully performed the following commands locally:

dbt run –full-refresh && dbt test
[na] dbt run (if incremental models are present) && dbt test

Before marking this PR as "ready for review" the following have been applied:

[na] The appropriate issue has been linked, tagged, and properly assigned
All necessary documentation and version upgrades have been applied
docs were regenerated (unless this PR does not include any code or yml updates)
BuildKite integration tests are passing
Detailed validation steps have been provided below

Detailed Validation

Please share any and all of your validation steps:

Will have customer confirm this branch allows their tests to pass.

If you had to summarize this PR in an emoji, which would it be?

💃

fivetran-joemarkiewicz

@fivetran-reneeli thanks for working through this PR. I have a few comments and suggestions before this is ready for approval. Great to see the customer mentioned this addressed the immediate issue in the linked bug report!

integration_tests/dbt_project.yml

CHANGELOG.md

fivetran-joemarkiewicz · 2024-11-05T15:37:31Z

integration_tests/seeds/deal_company_data.csv

I realize these probably aren't a result of the changes from this PR, but I noticed there are some dbt seed warnings in the BK runs produced from these seed files.

Would you be able to inspect these and make sure the warnings/errors are resolved in this release.

Looked into this. property_hs_createdate doesn't exist in our engagement_meeting seed data so I removed it from the config.

But I saw some like the below:

And I couldn't figure out where the error was coming from because I don't see property_hs_createdate being referenced for email_event_spam_report_data anywhere...

fivetran-joemarkiewicz

LGTM just one note to address before moving to release review. No reason to block initial approval.

fivetran-joemarkiewicz · 2024-11-07T22:59:01Z

macros/get_merged_deal_columns.sql

@@ -3,7 +3,8 @@
 {% set columns = [
    {"name": "_fivetran_synced", "datatype": dbt.type_timestamp()},
    {"name": "deal_id", "datatype": dbt.type_int()},
-    {"name": "merged_deal_id", "datatype": dbt.type_int()}
+    {"name": "merged_deal_id", "datatype": dbt.type_int()},
+    {"name": "category", "datatype": dbt.type_string()}


Category is not included in merged_deal so we should remove here as well

thanks for catching!

fivetran-avinash

@fivetran-reneeli Had a few questions!

fivetran-avinash · 2024-11-08T18:34:17Z

CHANGELOG.md

+- Introduced a new `category` column to the below models. This association field differentiates records by either HUBSPOT_DEFINED (default label) or USER_DEFINED (custom label) and was introduced to the Hubspot connector in October 2024. See the [connector release notes](https://fivetran.com/docs/connectors/applications/hubspot/changelog#october2024) for more.
+  - `stg_hubspot__deal_company`
+  - `stg_hubspot__deal_contact`
+  - `stg_hubspot__engagement_company`


@fivetran-reneeli I do not see the category column added to any of the models from lines 7-13. Was the intention to add category to these models, or should these lines be removed?

Great question! So these models leverage a macro to bring through all the columns and therefore the category field was not explicitly called out in them, thus the models themselves didn't require any changes. But you can see that the new field gets brought through after running the package.

Ah makes sense! Thanks for that.

fivetran-avinash · 2024-11-08T18:35:51Z

CHANGELOG.md

+
+## Under the Hood
+- Updated the respective seed files, `get_[source_table]_column` macros, and documentation of the mentioned models to include the `category` field.
+- Added explicit casting of `pipeline_id` to string in `stg_hubspot__deal_pipeline_stage` to ensure successful downstream joins.


@fivetran-reneeli If this could potentially change the data type, wouldn't this be a breaking change?

Good point, moved to the Breaking Change section

fivetran-avinash · 2024-11-08T18:45:27Z

CHANGELOG.md

@@ -1,3 +1,26 @@
+# dbt_hubspot_source v0.17.0


Could you bump the README version up to [">=0.17.0", "<0.18.0"]?

fivetran-avinash · 2024-11-08T18:46:45Z

integration_tests/dbt_project.yml

@@ -146,8 +147,6 @@ seeds:
      +column_types:
        engagement_id: "{{ 'int64' if target.type == 'bigquery' else 'bigint' }}"
        _fivetran_synced: timestamp
-        property_hs_createdate: timestamp


was there a reason this casting was removed for these fields? it wasn't called out in the CHANGELOG.

Yeah I had realized that these fields were not existent in the seed data and therefore not necessary. I'll call this out in the Under the Hood section too

CHANGELOG.md

fivetran-avinash

@fivetran-reneeli One small suggestion but after Buildkite passes you're good to merge!

Co-authored-by: Avinash Kunnath <108772760+fivetran-avinash@users.noreply.github.com>

fivetran-reneeli added 6 commits November 1, 2024 15:28

add category to macros, src, stg, ymls

932edc0

versioning

99afc0d

changelog, docs

5e315e6

docs and seed update

2957030

docs

7822cec

changelog updates

d57c075

fivetran-joemarkiewicz assigned fivetran-reneeli Nov 4, 2024

fivetran-joemarkiewicz self-requested a review November 4, 2024 21:33

fivetran-joemarkiewicz assigned fivetran-joemarkiewicz and unassigned fivetran-joemarkiewicz Nov 4, 2024

new schema for databricks sql

4bda432

fivetran-joemarkiewicz requested changes Nov 5, 2024

View reviewed changes

fivetran-reneeli added 10 commits November 5, 2024 16:01

updates

5a97f6e

cast all pipeline_id as string

ad968f8

rm category from merged_deal

cebe8a6

update some configs in integration tests

1ab6ea3

update changelog

e4505bd

try new schema

e0b3439

databricks sql new schema

f184aad

docs regen

64f3544

new schema

2974eb2

new schema

db77823

fivetran-joemarkiewicz approved these changes Nov 7, 2024

View reviewed changes

fivetran-avinash mentioned this pull request Nov 8, 2024

[Bug] Account for new category field in Hubspot models, bring in engagement table and update upstream joins fivetran/dbt_unified_rag#10

Closed

4 tasks

rm category from merged_deal

65feffa

fivetran-avinash reviewed Nov 8, 2024

View reviewed changes

fivetran-reneeli added 4 commits November 11, 2024 10:36

updates

35f0438

new schema

3a1ca0f

update dbt project yml to make consistent with transforms

34dc093

new schema

6ff3090

fivetran-avinash reviewed Nov 11, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

fivetran-avinash approved these changes Nov 11, 2024

View reviewed changes

Update CHANGELOG.md

567ab91

Co-authored-by: Avinash Kunnath <108772760+fivetran-avinash@users.noreply.github.com>

fivetran-reneeli merged commit c80c6e7 into main Nov 11, 2024
8 checks passed

fivetran-reneeli deleted the bugfix/category_add branch November 11, 2024 17:30

fivetran-reneeli linked an issue Nov 12, 2024 that may be closed by this pull request

[Feature] Support for new association type composite keys #131

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new category field #132

Add new category field #132

fivetran-reneeli commented Nov 1, 2024 •

edited

Loading

fivetran-joemarkiewicz left a comment

fivetran-joemarkiewicz Nov 5, 2024

fivetran-reneeli Nov 5, 2024

fivetran-joemarkiewicz left a comment

fivetran-joemarkiewicz Nov 7, 2024

fivetran-reneeli Nov 8, 2024

fivetran-avinash left a comment

fivetran-avinash Nov 8, 2024

fivetran-reneeli Nov 11, 2024 •

edited

Loading

fivetran-avinash Nov 11, 2024

fivetran-avinash Nov 8, 2024

fivetran-reneeli Nov 11, 2024

fivetran-avinash Nov 8, 2024

fivetran-reneeli Nov 11, 2024

fivetran-avinash Nov 8, 2024

fivetran-reneeli Nov 11, 2024

fivetran-avinash left a comment

Add new category field #132

Add new category field #132

Conversation

fivetran-reneeli commented Nov 1, 2024 • edited Loading

PR Overview

Breaking Change

PR Checklist

Basic Validation

Detailed Validation

If you had to summarize this PR in an emoji, which would it be?

fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fivetran-avinash left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fivetran-reneeli Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fivetran-avinash left a comment

Choose a reason for hiding this comment

fivetran-reneeli commented Nov 1, 2024 •

edited

Loading

fivetran-reneeli Nov 11, 2024 •

edited

Loading