feature/country-region-reports#44
Conversation
fivetran-jamie
left a comment
fivetran-jamie
left a comment
There was a problem hiding this comment.
looks great! see comments
CHANGELOG.md
Outdated
| | linkedin_ads__monthly_campaign_country_report | New transformation model | | | Table that represents the monthly performance of a campaign at the country level. | | ||
| | linkedin_ads__monthly_campaign_region_report | New transformation model | | | Table that represents the monthly performance of a campaign at the region level. | |
There was a problem hiding this comment.
Could be good to add the DAG/docs links to the end models
There was a problem hiding this comment.
Agreed - added.
| geo.value as region_name, | ||
| report.campaign_id, | ||
| campaign.campaign_name, | ||
| campaign.version_tag, |
There was a problem hiding this comment.
same question re: prepending campaign/account fields with campaign_ or account_
There was a problem hiding this comment.
See above response.
Co-authored-by: Jamie Rodriguez <65564846+fivetran-jamie@users.noreply.github.com>
CHANGELOG.md
Outdated
| |---------------------------------------------------|-------------|----------|-------------------------------------------|-------------------------------------------------------------------| | ||
| | [linkedin_ads__monthly_campaign_country_report](https://fivetran.github.io/dbt_linkedin/#!/model/model.linkedin.linkedin_ads__monthly_campaign_country_report) | New transformation model | | | Table that represents the monthly performance of a campaign at the country level. | | ||
| | [linkedin_ads__monthly_campaign_region_report]((https://fivetran.github.io/dbt_linkedin/#!/model/model.linkedin.linkedin_ads__monthly_campaign_region_report)) | New transformation model | | | Table that represents the monthly performance of a campaign at the region level. | | ||
| | stg_tiktok_ads__geo_tmp | New temp model | | | Temp model added for `geo`. | |
There was a problem hiding this comment.
Same recommendation to update these models with links to the DAG.
There was a problem hiding this comment.
Updated
integration_tests/tests/integrity/vertical_sum_monthly_country_report.sql
Outdated
Show resolved
Hide resolved
integration_tests/tests/integrity/vertical_sum_monthly_region_report.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
We will want to apply line 154 with the new support for the new tables added I think, with a link to this upcoming release.
There was a problem hiding this comment.
Same as source response. Made the appropriate update
fivetran-avinash
left a comment
fivetran-avinash
left a comment
There was a problem hiding this comment.
@fivetran-joemarkiewicz Nearly there! A few comments, questions and suggestions before approval.
Co-authored-by: Avinash Kunnath <108772760+fivetran-avinash@users.noreply.github.com>
There was a problem hiding this comment.
@fivetran-joemarkiewicz LGTM (pending Snowflake fix)
3 participants
PR Overview
Package version introduced in this PR:
v0.11.0This PR addresses the following Issue/Feature(s): Internal Ticket
Summary of changes:
Includes new
linkedin_ads__monthly_campaign_country_reportandlinkedin_ads__monthly_campaign_region_reportend models.Submission Checklist
For all above validations and testing please see internal ticket. Additionally, docs have not be generated and will be once PR is near approval.
Changelog