Skip to content
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

bugfix/sla_duplicates #67

Merged
merged 4 commits into from
Mar 11, 2022
Merged

bugfix/sla_duplicates #67

merged 4 commits into from
Mar 11, 2022

Conversation

fivetran-joemarkiewicz
Copy link
Contributor

Are you a current Fivetran customer?

Fivetran created PR

What change(s) does this PR introduce?

The 0.7.1 version of the package changed adjusted the join condition within the int_zendesk__sla_policy_applied model to account for sla_policy_name fields that were set shortly before or after the sla_policy was actually set for the ticket. However, as identified within Issue #66, this join condition has the potential to match with many sla policies and cause duplicates in the final zendesk__sla_policy model.

Therefore, the PR addresses the above issue by leveraging the valid_starting_at field for the sla_policy instead of the sla_applied_at field. This is needed as the sla_applied_at field changes to be the ticket.created_at field if the sla is first_reply_time as the first_reply_time sla needs to reference the the ticket created_at time and not the actual time the sla was set. However, using this time opposed to the valid_starting_at is not an accurate join. Using this logic instead of the previous logic ensure the join condition brings in the correct sla_policy_name and not produce duplicates.

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.)

This change will not impact any of the incremental models and only adjusts a join condition. This will not cause 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)

  • Yes

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

How did you test the PR changes?

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

I also tested this locally and received confirmation from the customer that this change resolved the issue on their end.

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-joemarkiewicz fivetran-joemarkiewicz marked this pull request as ready for review March 1, 2022 21:50
@fivetran-sheringuyen
Copy link
Contributor

Hey @fivetran-joemarkiewicz, good PR! It passed both dbt run and dbt test, and I also went to double check that using ticket_id and valid_starting_at would have no duplicates.

I believe this should change the dbt docs, so if you could rerun the docs, that'd be great!

@fivetran-joemarkiewicz fivetran-joemarkiewicz added the bug Something isn't working label Mar 11, 2022
@fivetran-joemarkiewicz
Copy link
Contributor Author

Thanks so much @fivetran-sheringuyen and good call with the docs! I just added the updated docs to the branch.

Would you be able to take another pass and approve or request any other changes. Thanks!

Copy link
Contributor

@fivetran-sheringuyen fivetran-sheringuyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@fivetran-joemarkiewicz fivetran-joemarkiewicz merged commit cdde7f5 into main Mar 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants