Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce new stream - campaign_insights_by_province #25

Merged

Conversation

sgandhi1311
Copy link
Member

Description of change

On customer demand, they would like to track campaign data based on province.
Customer feedback: "We need to be able to run reports based on province, as a lot of our campaign ad spend is earmarked for certain regions or states. Breakdown by campaign, province and just province would be extremely useful to us."
This PR contains the necessary changes required to introduce any new stream.

Manual QA steps

  • Locally tested on the cloned integration.
  • Generate the correct catalog in the discovery mode.
  • Sync, Pagination, bookmarking and transformation are working fine.

Risks

  • Low

Rollback steps

  • revert this branch

@RushiT0122 RushiT0122 self-requested a review July 24, 2023 07:20
Copy link

@RushiT0122 RushiT0122 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some queries on the schema file. Also we need to add new stream in the README.md

Comment on lines 153 to 164
"conversion_rate_v2": {
"type": [
"null",
"string"
]
},
"real_time_conversion_rate_v2": {
"type": [
"null",
"string"
]
},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason these fields have type string instead of number?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the documentation mentions the data type as a string. link - https://ads.tiktok.com/marketing_api/docs?id=1751454162042882

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the field type to the number after reading the documentation, as it will only display rates which will be the numeric value.

@sgandhi1311 sgandhi1311 force-pushed the TDL-23578-new-stream-campaign-province branch from 2711e16 to d016cb4 Compare July 25, 2023 07:43
Copy link

@rdeshmukh15 rdeshmukh15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting of campaign_insights_by_province.json is not proper. And, add the blank line at the EOF

@sgandhi1311
Copy link
Member Author

Formatting of campaign_insights_by_province.json is not proper. And, add the blank line at the EOF

done

@sgandhi1311 sgandhi1311 force-pushed the TDL-23578-new-stream-campaign-province branch from aaa69aa to 7cefd82 Compare July 25, 2023 19:30
@sgandhi1311 sgandhi1311 force-pushed the TDL-23578-new-stream-campaign-province branch from 7cefd82 to 10712f6 Compare July 26, 2023 06:05
@rdeshmukh15 rdeshmukh15 self-requested a review July 26, 2023 13:30
@sgandhi1311 sgandhi1311 merged commit 497182d into TDL-22949-schema-changes Jul 27, 2023
sgandhi1311 added a commit that referenced this pull request Aug 3, 2023
* modify API endpoint

* add fields in the campaigns schema

* schema changes for adgroup

* fix unit test

* Schema changes for ads stream

* advertisers schema change

* update Advertisers key properties

* update field types for insight/s streams

* set the advertiser_id if it is not present in record

* update placement field to placement_type

* setup and changelog updates

* add few more fields in the ads and adgroups stream

* pass the advertiser_id in the param as json formatted string

* fix failing unit test cases

* update changelog

* add new fields for the insight related streams

* address review comments

* add  new field - rf_campaign_type in the schemas

* remove aeo_type field

* update primary key for advertisers stream in the base.py

* modify all fields test

* update docs links in readme

* update field type for conversion rates

* retry for code - 40100

* fix the integration tests

* remove few fields from test all field test

* Introduce new stream - campaign_insights_by_province (#25)

* Add new stream - campaign_insights_by_province

* update base.py with the new stream - campaign_insights_by_province

* remove user_action field from the campaign insights stream

* update the README.md file to include the latest stream details.

* format the campaign insights json file

* update field type for conversion rates

* remove campaigns by province stream from start date and pagination
sgandhi1311 added a commit that referenced this pull request Aug 17, 2023
* modify API endpoint

* add fields in the campaigns schema

* schema changes for adgroup

* fix unit test

* Schema changes for ads stream

* advertisers schema change

* update Advertisers key properties

* update field types for insight/s streams

* set the advertiser_id if it is not present in record

* update placement field to placement_type

* setup and changelog updates

* add few more fields in the ads and adgroups stream

* pass the advertiser_id in the param as json formatted string

* fix failing unit test cases

* update changelog

* add new fields for the insight related streams

* address review comments

* add  new field - rf_campaign_type in the schemas

* remove aeo_type field

* update primary key for advertisers stream in the base.py

* modify all fields test

* update docs links in readme

* update field type for conversion rates

* retry for code - 40100

* fix the integration tests

* remove few fields from test all field test

* Introduce new stream - campaign_insights_by_province (#25)

* Add new stream - campaign_insights_by_province

* update base.py with the new stream - campaign_insights_by_province

* remove user_action field from the campaign insights stream

* update the README.md file to include the latest stream details.

* format the campaign insights json file

* update field type for conversion rates

* remove campaigns by province stream from start date and pagination
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants