Update/ad reporting v2

[fivetran-sheringuyen]

Pull Request
Are you a current Fivetran customer?

What change(s) does this PR introduce?

• Addition of new facebook_ads__ad_report that reports spend, clicks and impressions at the ad level.
• Renames facebook_ads__ad_adapter model to facebook_ads__utm_report to more accurately reflect what is included in the report; this report now also filters for only records that have valid URLs.
• README updates for easier navigation and use of the package.
• Moves previously used dbt_facebook_ads_creative_history.stg_facebook_ads__url_tag directly into this package via the use of the get_url_tag_query macro as an intermediate model called int_facebook_ads__url_tags.
• Renamed facebook_ads__creative_history_prep model to int_facebook_ads__creative_history to conform with new styling standards.

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

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)

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]

This PR will actually also close out Issue #12

[fivetran-joemarkiewicz]

@fivetran-sheringuyen thanks so much for putting this PR together and taking on the mighty task of combining the creative history and this package! These updates are looking great, I do have a few comments I would like for you to address before approving the PR:

• Address the inline comments within this review
• I noticed in the end models the records can have a null record for clicks, but there are also records for 0. This doesn't seem to be the same for impressions and spend. Should we convert these null records to be 0? Or what is the difference between null and 0 clicks in this case?
• Open a FR for us to integrate the remaining creative history models into this package. In the meantime within the FR we can point users to the creative history package to replicate the models in the meantime.
• I notice we don't have any tests or yml documentation for the creative history models. Can we port them over to ensure we are still applying the same tests we were previously.
• What is your stance on renaming the models that were moved from dbt_facebook_ads_creative_history to stg_ instead of int_?
  • Since I could see people using them as staging models, I wonder if it would make more sense to name them as such, but obviously keep the logic in the transformation package.
  • Otherwise, what are your thoughts on making them final models that users can reference. Simply because I see these models being leveraged on their own, and it may be confusing to name them as intermediate. Thoughts?

[fivetran-joemarkiewicz]

Just commenting to make sure we update on release 😄

[fivetran-sheringuyen]

Hey @fivetran-joemarkiewicz, I've just committed the PR fixes you've requested, let me know if you want to discuss further!

Regarding your question:

What is your stance on renaming the models that were moved from dbt_facebook_ads_creative_history to stg_ instead of int_?

• Since I could see people using them as staging models, I wonder if it would make more sense to name them as such, but obviously keep the logic in the transformation package.
• Otherwise, what are your thoughts on making them final models that users can reference. Simply because I see these models being leveraged on their own, and it may be confusing to name them as intermediate. Thoughts?

I agree that people could definitely use them so I think it would make sense to create them as their own final models. With the one model (*_url_tags) that we've migrated over in this update, I've made it into a final model and added appropriate testing for it in facebook_ads.yml.

int_facebook_ads__creative_history, however, should be kept as an intermediate model -- it actually takes *_url_tags and prepares it for the utm_report. While we've transformed the data to fit for our specific modeling needs, these transformations may not scale across the user base.

Additionally, as per our conversation last week, I believe we spoke about keeping

  - package: fivetran/facebook_ads_creative_history
    version: [">=0.4.0", "<0.5.0"]

in the packages.yml, so I updated this as well. Let me know if your thoughts have changed on this front.

[fivetran-joemarkiewicz]

We will also want to cut a release for this package then to have the new source package as a dependency on the updated source package.

[fivetran-joemarkiewicz]

Looks good to go!!! Thanks so much @fivetran-sheringuyen

[fivetran-sheringuyen]

Running comments before merge:

• Add not_null tests to final model tests