Bugfix/email send idi

[fivetran-reneeli]

Pull Request
Are you a current Fivetran customer?

Internal
What change(s) does this PR introduce?

Updates surrogate key email_send_id to include primary_attribute_value_id.

Did you update the CHANGELOG?

• Yes

Does this PR introduce a breaking change?

• [] Yes (please provide breaking change details below.)
• No (please provide an explanation as to how the change is non-breaking below.)
  Updating a field

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)

• Yes

Is this PR in response to a previously created Bug or Feature Request

• Yes, Issue [Bug] Under-reporting of metrics for Nurture Campaigns (Or campaigns with multiple emails) #25
• No

How did you test the PR changes?

• CircleCi
• Local (please provide additional testing details below)

Select which warehouse(s) were used to test the PR

• BigQuery
• Redshift
• Snowflake
• Postgres
• Databricks
• Other (provide details below)

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.

[fivetran-reneeli]

Hey @fivetran-joemarkiewicz this is ready for review. As for testing it looks like our data only has one primary_attribute_value_id per campaign id so we're a little limited in testing to ensure, but I think it's straightforward enough where we'd be comfortable moving forward, what do you think?

with ranker as 
(
SELECT campaign_id, primary_attribute_value_id,campaign_run_id, lead_id, rank() over(partition by campaign_id, primary_attribute_value_id order by primary_attribute_value_id) as ranking FROM `dbt-package-testing.dbt_renee_stg_marketo.stg_marketo__activity_send_email` 
order by 1,2
)
select * from ranker where ranking != 1

[fivetran-joemarkiewicz]

Hey @fivetran-reneeli thanks for opening this PR. I am comfortable with the limited data we have. However, there are a few changes I think we need to make. It looks like the email_send_id is used in a downstream join but we only updated the field in the stg_marketo__activity_send_email model. Can we update this field in all other models where it is created (for example it also occurs in stg_marketo__activity_email_bounced? Additionally, we will need to update the yml definitions similarly where we update it.

Once those updates are applied, can you test that this branch works with the downstream dbt_marketo package. Since this is a patch release, we won't want to accidentally cut a release that can have downstream failures. Let me know if you have any questions.

[fivetran-joemarkiewicz]

Can you link the issue @sfc-gh-sugupta originally created. This was we can see their contribution since it is not a direct commit.

[fivetran-joemarkiewicz]

It looks like this wasn't added. Would you be able to link the issue next to the account name

[fivetran-reneeli]

Hey @fivetran-reneeli thanks for opening this PR. I am comfortable with the limited data we have. However, there are a few changes I think we need to make. It looks like the email_send_id is used in a downstream join but we only updated the field in the stg_marketo__activity_send_email model. Can we update this field in all other models where it is created (for example it also occurs in stg_marketo__activity_email_bounced? Additionally, we will need to update the yml definitions similarly where we update it.

Once those updates are applied, can you test that this branch works with the downstream dbt_marketo package. Since this is a patch release, we won't want to accidentally cut a release that can have downstream failures. Let me know if you have any questions.

Thanks for catching that @fivetran-joemarkiewicz! I've updated email_send_id in the remaining stg models/ checked for the ones being used in the downstream joins. Ymls updated. Following that, downstream package works

[fivetran-joemarkiewicz]

Thanks for applying the changes @fivetran-reneeli! It looks like the CHANGELOG request from the last review was missed. Would you be able to make that update. Once that is applied, this is good to go!

[fivetran-joemarkiewicz]

It looks like this wasn't added. Would you be able to link the issue next to the account name