fivetran / dbt_recurly

Fivetran data transformations for Recurly built using dbt.
https://fivetran.github.io/dbt_recurly/
Apache License 2.0
4 stars 2 forks source link

Update recurly__churn_analysis.sql #10

Closed suelai closed 1 year ago

suelai commented 1 year ago

Update the definition of involuntary churn, according to Recurly https://docs.recurly.com/docs/churn-analysis

Are you a current Fivetran customer? Suela Isaj, Data Engineer, Issuu

What change(s) does this PR introduce? This is a small bug in the definition of voluntary and involuntary churn

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

:first_quarter_moon_with_face: **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 1 year ago

Hi @suelai thanks so much for opening this PR! After reviewing your update and seeing the Recurly docs do indeed include the tax location invalid churn reason for being involuntary, I think this is a great update to include in our package.

That being said, I wanted to confirm the reason within the data does map to this exact string. Our internal data is only test data and doesn't have the tax location invalid reason. Would you be able to verify that this string is exactly what you see within your warehouse?

suelai commented 1 year ago

@fivetran-joemarkiewicz That is a great observation, I did the change with the correct spelling (the lat commit). I initially thought it had been renamed in CTEs, but after I traced it to the source, the value should be tax_location_invalid

suelai commented 1 year ago

@fivetran-joemarkiewicz You're welcome! Happy holidays 🎄