Add missing reaction type in sum #7
Conversation
|
@fivetran-joemarkiewicz Hopefully this PR gets things moving on the fix. Thanks for working through this on fivetran/dbt_facebook_pages#8. |
There was a problem hiding this comment.
@zhyatt thanks again so much for opening this PR! The changes you are suggesting look great and I was able to test locally and see that the null value for the total does indeed get resolved with this coalesce and addition calculation.
I had one small suggestion in the code as well as a few small requests:
- Would you be able to update the CHANGELOG for this next patch release (
v0.2.1). Don't forget to include yourself as a contributor! - Would you be able to increment the dbt_project.yml and integration_tests/dbt_project.yml versions to be
0.2.1to reflect this upcoming release.
Let me know if you have any questions regarding these requests. Once those are applied I can re-review and fold this into the next release!
Co-authored-by: Joe Markiewicz <74217849+fivetran-joemarkiewicz@users.noreply.github.com>
…cebook_pages_source into post_reactions_fix
The small code suggestion and updates for the patch release have been completed. Sending your way for review! |
|
Thanks so much @zhyatt! Apologies for the delay as I was on PTO. I will review the updates and work to integrate into the next release this week! |
There was a problem hiding this comment.
@zhyatt thanks so much for working on this PR and addressing the original issue you raised with our team! I am happy to approve this PR 😄
I will now kick off the internal release review process. You should see your changes live in 0.2.1 on Monday morning (we have a release freeze for Fridays).
Are you a current Fivetran customer?
Yes, Zach Hyatt, Invisible Agents LLC
What change(s) does this PR introduce?
Per the discussion here: fivetran/dbt_facebook_pages#8 this fixes null values for the total reactions count by summing the various types if null comes from the source.
Did you update the CHANGELOG?
Does this PR introduce a breaking change?
This only adds new data to an already existing field, which by default is null. The data type should be expected.
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
actions_post_reactions_totalfield always null dbt_facebook_pages#8How 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
💪
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.