fivetran / dbt_hubspot

Data models for Hubspot built using dbt.
https://fivetran.github.io/dbt_hubspot/
Apache License 2.0
33 stars 39 forks source link

Bugfix Engagement flags fix. #65

Closed lycanlee closed 2 years ago

lycanlee commented 2 years ago

Pull Request Are you a current Fivetran customer?

Matt Lee, Senior Data Engineer, Atmos Inc. I am currently trialing Fivetran. I am a previous customer at Prodigy Education.

What change(s) does this PR introduce? This is a bug fix which should allow for flags to be set for engagements to actually compile correctly. For example when providing hubspot_engagement_company_enabled: false you get an error:

16:46:28  83 of 91 START table model dbt_mlee_hubspot.hubspot__engagement_calls........... [RUN]
16:46:29  83 of 91 ERROR creating table model dbt_mlee_hubspot.hubspot__engagement_calls.. [ERROR in 0.63s]
Database Error in model hubspot__engagement_calls (models/sales/engagement_events/hubspot__engagement_calls.sql)
  000904 (42000): SQL compilation error: error line 26 at position 8
  invalid identifier 'ENGAGEMENTS.COMPANY_IDS'
  compiled SQL at target/run/hubspot/models/sales/engagement_events/hubspot__engagement_calls.sql

This is because macros/engagements_join.sql blindly selects columns which may not exist in hubspot__engagements.

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

:dancer: **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](https://www.surveymonkey.com/r/DQ7K7WW) on our existing dbt packages or what you'd like to see next.
fivetran-joemarkiewicz commented 2 years ago

Hi @lycanlee, thanks so much for opening this PR! I really appreciate your collaboration on dbt Slack and for helping contribute to the open source project with this PR 😃!

The first cursory glance of this PR looks good, but I will have someone from my team give a more detailed review and test these changes at the start of our next sprint (starting mid next week). Once reviewed and approved we can plan to move this into the next release of the package!

Let me know if you have any questions in the meantime.

lycanlee commented 2 years ago

Wanted to check if there has been any movement on this?

fivetran-joemarkiewicz commented 2 years ago

Wanted to check if there has been any movement on this?

Hey @lycanlee I apologize for the delay. This is being reviewed this current sprint!

fivetran-reneeli commented 2 years ago

Hi @lycanlee, thank you so much for submitting this PR! I tested it and updated relevant docs; your branch has now been merged to main.