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

add not statement for multi touch resolution metric #74

Merged

Conversation

tonytusharjr
Copy link
Contributor

@tonytusharjr tonytusharjr commented Jun 10, 2022

Pull Request
Are you a current Fivetran customer?
Tony Tushar, Sr Analytics Engineer, Apto Payments

What change(s) does this PR introduce?
PR is a quick fix on a missing logic in a case statement for determining multi-touch resolution metric. I noticed in our data that 32k records were being defined as both two and multi-touch = 1, so this fix corrects for that.

Additional fix on is_agent_submitted case statement where admin, agent should be all lowercase.

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 is a minor additional clause in a case statement, no breaking changes introduced by this.

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/Feature [link bug/feature number here]
  • No

How did you test the PR changes?

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

I initially ran this statement in my project and received 32k records in response where is_two_touch_resolution and is_multi_touch_resolution both equal True. Once I made the change suggested in this PR, I did not receive any rows back.

select
  ticket_id,
  sum(case when is_one_touch_resolution = true then 1 else 0 end) as one_touch_count,
  sum(case when is_two_touch_resolution = true then 1 else 0 end) as two_touch_count,
  sum(case when is_multi_touch_resolution = true then 1 else 0 end) as multi_touch_count,
from [data]
group by 1
having (sum(case when is_two_touch_resolution = true then 1 else 0 end) = 1
and sum(case when is_multi_touch_resolution = true then 1 else 0 end) = 1)
or ((sum(case when is_one_touch_resolution = true then 1 else 0 end) = 1
and sum(case when is_multi_touch_resolution = true then 1 else 0 end) = 1))
or (sum(case when is_one_touch_resolution = true then 1 else 0 end) = 1
and sum(case when is_two_touch_resolution = true then 1 else 0 end) = 1);

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-sheringuyen fivetran-sheringuyen merged commit 635caca into fivetran:main Jun 16, 2022
@fivetran-sheringuyen
Copy link
Contributor

Awesome work on this contribution @tonytusharjr ! I have just merged and cut the release for your changes, watch out for the release on the dbt package hub and looking forward to seeing more future contributions! 🎉

@tonytusharjr tonytusharjr deleted the fix/touch-resolution-metrics branch June 21, 2022 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants