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

Tdl 20909 Add premade reports with dimensions filters #31

Merged
merged 6 commits into from
Dec 12, 2022

Conversation

leslievandemark
Copy link
Collaborator

@leslievandemark leslievandemark commented Dec 1, 2022

Description of change

Adds two premade reports, conversions_report and in_app_purchases, and hard codes their dimension filters in the client class.

Manual QA steps

  • It's not worth adding a tap tester test for these premade reports with filters. Mainly because the dimension in the filter is not selected in the premade reports, so we would not be able to make any assertions on the dimension being filtered. Also, we have no data whatsoever for in_app_purchases
  • Tested conversions_report manually, by running the report with isConversionEvent selected and without, and confirming the records received are the same.
  • Tested conversions_report without the filter syncs more records
  • Tested that in_app_purchases can complete a sync, but not records were synced.

Risks

  • Our test account doesn't have any purchaser data, so I was unable to test thatin_app_purchases synced the correct rows.

Rollback steps

  • revert this branch

tap_ga4/reports.py Show resolved Hide resolved
tap_ga4/reports.py Show resolved Hide resolved
tests/test_automatic_fields.py Show resolved Hide resolved
tests/test_sync_canary.py Show resolved Hide resolved
@leslievandemark leslievandemark merged commit da63fa1 into main Dec 12, 2022
@leslievandemark leslievandemark deleted the TDL-20909/premade-report-filters branch December 12, 2022 14:48
leslievandemark added a commit that referenced this pull request Dec 12, 2022
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.

2 participants