diff --git a/.github/PULL_REQUEST_TEMPLATE/maintainer_pull_request_template.md b/.github/PULL_REQUEST_TEMPLATE/maintainer_pull_request_template.md index 768ac3f..1e22b09 100644 --- a/.github/PULL_REQUEST_TEMPLATE/maintainer_pull_request_template.md +++ b/.github/PULL_REQUEST_TEMPLATE/maintainer_pull_request_template.md @@ -4,48 +4,27 @@ **This PR will result in the following new package version:** -**Please detail what change(s) this PR introduces and any additional information that should be known during the review of this PR:** +**Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:** + ## PR Checklist ### Basic Validation Please acknowledge that you have successfully performed the following commands locally: -- [ ] dbt compile -- [ ] dbt run –full-refresh -- [ ] dbt run -- [ ] dbt test -- [ ] dbt run –vars (if applicable) +- [ ] dbt run –full-refresh && dbt test +- [ ] dbt run (if incremental models are present) && dbt test Before marking this PR as "ready for review" the following have been applied: -- [ ] The appropriate issue has been linked and tagged -- [ ] You are assigned to the corresponding issue and this PR +- [ ] The appropriate issue has been linked, tagged, and properly assigned +- [ ] All necessary documentation and version upgrades have been applied + +- [ ] docs were regenerated (unless this PR does not include any code or yml updates) - [ ] BuildKite integration tests are passing +- [ ] Detailed validation steps have been provided below ### Detailed Validation -Please acknowledge that the following validation checks have been performed prior to marking this PR as "ready for review": -- [ ] You have validated these changes and assure this PR will address the respective Issue/Feature. -- [ ] You are reasonably confident these changes will not impact any other components of this package or any dependent packages. -- [ ] You have provided details below around the validation steps performed to gain confidence in these changes. +Please share any and all of your validation steps: -### Standard Updates -Please acknowledge that your PR contains the following standard updates: -- Package versioning has been appropriately indexed in the following locations: - - [ ] indexed within dbt_project.yml - - [ ] indexed within integration_tests/dbt_project.yml -- [ ] CHANGELOG has individual entries for each respective change in this PR - -- [ ] README updates have been applied (if applicable) - -- [ ] DECISIONLOG updates have been updated (if applicable) -- [ ] Appropriate yml documentation has been added (if applicable) - -### dbt Docs -Please acknowledge that after the above were all completed the below were applied to your branch: -- [ ] docs were regenerated (unless this PR does not include any code or yml updates) - ### If you had to summarize this PR in an emoji, which would it be? -:dancer: +:dancer: \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 69bf8f7..e31b882 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ -# dbt_snapchat_ads v0.6.1 +# dbt_snapchat_ads v0.6.2 +## Bug Fixes +- Adjust the severity of the `ad_account_id` test in `snapchat_ads__account_report` to `warn`. This is required since Snapchat can hard-delete records from the history tables, but not from the reporting tables. This ensures that accurate statistics are being reported and production pipelines aren't failing. ([PR #20](https://github.com/fivetran/dbt_snapchat_ads/pull/20)) + - Documents the above decision in the [DECISIONLOG.md](https://github.com/fivetran/dbt_snapchat_ads/blob/main/DECISIONLOG.md). + +## Under the Hood +- Updated the repo-maintainer PR template to our most up-to-date format ([PR #24](https://github.com/fivetran/dbt_snapchat_ads/pull/24)). +## Contributors +- [@bthomson22](https://github.com/bthomson22) ([PR #20](https://github.com/fivetran/dbt_snapchat_ads/pull/20)) + +# dbt_snapchat_ads v0.6.1 [PR #22](https://github.com/fivetran/dbt_snapchat_ads/pull/22) includes the following updates: ## Bug Fixes - This package now leverages the new `snapchat_ads_extract_url_parameter()` macro for use in parsing out url parameters. This was added to create special logic for Databricks instances not supported by `dbt_utils.get_url_parameter()`. diff --git a/DECISIONLOG.md b/DECISIONLOG.md index b280af7..f0bf3e3 100644 --- a/DECISIONLOG.md +++ b/DECISIONLOG.md @@ -5,4 +5,25 @@ An ad can be associated with multiple ad squads and therefore multiple campaigns ## UTM Filtering This package contains a `snapchat_ads__url_report` which provides daily metrics for your utm compatible ads. It is important to note that not all Ads leverage utm parameters. Therefore, this package takes an opinionated approach to filter out any records that do not contain utm parameters or leverage a url within the ad. -If you would like to leverage a report that contains all ads and their daily metrics, I would suggest you leverage the snpapchat_ads__ad_report which does not apply any filtering. \ No newline at end of file +If you would like to leverage a report that contains all ads and their daily metrics, I would suggest you leverage the snpapchat_ads__ad_report which does not apply any filtering. + +## Ad Account Report Metrics Associated with Deleted Entities +Similar to some other Ad Platforms, Snapchat Ads will hard-delete entities (i.e. ads, ad squads, campaigns, accounts) from their `*_history` tables but retain associated records in their respective `*_hourly_report` tables. This typically does not pose an issue for our `not_null` tests on our end models, as most entities have their own `_hourly_report` source tables that come with the appropriate entity-level ID. However, `snapchat_ads__account_report` draws from the `ad_hourly_report` table, rolls it up to the account level, and joins in the `ad_account_id` using history tables. Thus, if any ad report record is associated with a deleted ad, campaign, ad squad, or account, the `ad_account_id` will be `null`. + +We have opted to keep these records in `snapchat_ads__account_report`, as it may be valuable to know that non-zero ad metrics are associated with deleted entities (though null-account records will be grouped together). However, we have changed the severity of the `not_null` test on `ad_account_id` to be `warn` instead of `error`. + +If you would like to disable this `not_null` test completely to avoid warnings, add the following to your root project `dbt_project.yml`: +```yml +tests: + snapchat_ads: + not_null_snapchat_ads__account_report_ad_account_id: + +enabled: false +``` + +And if you are using the downstream [Ad Reporting](https://github.com/fivetran/dbt_ad_reporting/) data models, you'd also want to apply the same change to `ad_reporting__account_report`: +```yml +tests: + ad_reporting: + not_null_ad_reporting__account_report_account_id: + +enabled: false +``` \ No newline at end of file diff --git a/dbt_project.yml b/dbt_project.yml index 5110ba4..92345c7 100644 --- a/dbt_project.yml +++ b/dbt_project.yml @@ -1,5 +1,5 @@ name: 'snapchat_ads' -version: '0.6.1' +version: '0.6.2' config-version: 2 require-dbt-version: [">=1.3.0", "<2.0.0"] vars: diff --git a/integration_tests/dbt_project.yml b/integration_tests/dbt_project.yml index 9ba3860..42509cb 100644 --- a/integration_tests/dbt_project.yml +++ b/integration_tests/dbt_project.yml @@ -1,5 +1,5 @@ name: 'snapchat_ads_integration_tests' -version: '0.6.1' +version: '0.6.2' profile: 'integration_tests' config-version: 2 diff --git a/models/snapchat.yml b/models/snapchat.yml index 17ae4e0..0813614 100644 --- a/models/snapchat.yml +++ b/models/snapchat.yml @@ -19,7 +19,9 @@ models: - name: ad_account_id description: The ID of the account in Snapchat. tests: - - not_null + - not_null: + config: + severity: warn - name: ad_account_name description: The name of the account in Snapchat. - name: currency