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.

Sign up for GitHub

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

Jump to bottom

feat: Remove the campaign rendered event #2301

Merged
merged 1 commit into from
Feb 27, 2023
Merged

feat: Remove the campaign rendered event #2301

merged 1 commit into from
Feb 27, 2023

Conversation

leogermani
Copy link
Contributor

All Submissions:

Changes proposed in this Pull Request:

We should not trigger an event when a campaign is rendered.

  1. This happens all the time, and since we are using Action Scheduler to trigger events, this could overflow our database with events
  2. This event does not tell us anything useful. The campaigns are rendered all the time, even when the user doesn't match the segment and won't see the popup. So we can't use this to build funnels and calculating conversion rates.
  3. The page output will be cached. So the counts of renders will not match the times the campaign was rendered for users. Again, this count won't be useful in our analytics to build funnels.

How to test the changes in this Pull Request:

  1. Nothing really. Just make sure you don't see [NEWSPACK-DATA-EVENTS][Newspack\Data_Events::dispatch]: Dispatching action "campaign_interaction" via Action Scheduler. in the logs when just loading a page.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@leogermani leogermani added the [Status] Needs Review The issue or pull request needs to be reviewed label Feb 24, 2023
@leogermani leogermani requested a review from a team as a code owner February 24, 2023 14:14
@leogermani leogermani self-assigned this Feb 24, 2023
dkoo
dkoo approved these changes Feb 24, 2023
View reviewed changes
Copy link
Contributor

@dkoo dkoo left a comment

Choose a reason for hiding this comment

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

Agreed, I don't think this event is really useful in any real way.

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Feb 24, 2023
@leogermani leogermani merged commit 23caa1d into master Feb 27, 2023
matticbot pushed a commit that referenced this pull request Mar 3, 2023
@semantic-release-bot
chore(release): 1.106.0-alpha.1 [skip ci]
a554a5a 
# [1.106.0-alpha.1](v1.105.0...v1.106.0-alpha.1) (2023-03-03)

### Bug Fixes

* **ads:** gam api availability according to error type ([#2289](#2289)) ([024fe08](024fe08))

### Features

* add a Add new button to subscription lists ([#2314](#2314)) ([9543ad2](9543ad2))
* add ga4 user registered handler ([#2281](#2281)) ([5eb2336](5eb2336))
* add pid to Logger ([#2290](#2290)) ([fd3011c](fd3011c))
* Add popup info to donations ([#2300](#2300)) ([7ea800b](7ea800b))
* allow external links in dashboard via a filter ([#2279](#2279)) ([3943b1a](3943b1a))
* campaigns listeners for the data events api ([#2291](#2291)) ([ab407d4](ab407d4))
* disable save button for unchanged settings ([#2259](#2259)) ([e06d72f](e06d72f)), closes [#1531](#1531)
* **donate-block:** support modal checkout ([#2256](#2256)) ([34226dd](34226dd))
* Normalize donation events ([#2299](#2299)) ([2624d53](2624d53))
* **perfmatters:** improve config ([267306e](267306e))
* prevent homepage from being unpublished ([#2307](#2307)) ([a151d53](a151d53))
* Remove the campaign rendered event ([#2301](#2301)) ([23caa1d](23caa1d))
* Stripe Subscriptions to WC subscriptions migrator ([#2298](#2298)) ([6904356](6904356)), closes [#2251](#2251)
* **wc:** force allowing subscription switching ([#2305](#2305)) ([c13e741](c13e741))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.106.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Mar 14, 2023
@semantic-release-bot
chore(release): 1.106.0 [skip ci]
00f9fc5 
# [1.106.0](v1.105.1...v1.106.0) (2023-03-14)

### Bug Fixes

* **ads:** gam api availability according to error type ([#2289](#2289)) ([024fe08](024fe08))
* show handoff to finish Newspack setup only if setup is incomplete ([#2343](#2343)) ([1173b5b](1173b5b))

### Features

* add a Add new button to subscription lists ([#2314](#2314)) ([9543ad2](9543ad2))
* add ga4 user registered handler ([#2281](#2281)) ([5eb2336](5eb2336))
* add pid to Logger ([#2290](#2290)) ([fd3011c](fd3011c))
* Add popup info to donations ([#2300](#2300)) ([7ea800b](7ea800b))
* allow external links in dashboard via a filter ([#2279](#2279)) ([3943b1a](3943b1a))
* campaigns listeners for the data events api ([#2291](#2291)) ([ab407d4](ab407d4))
* disable save button for unchanged settings ([#2259](#2259)) ([e06d72f](e06d72f)), closes [#1531](#1531)
* **donate-block:** support modal checkout ([#2256](#2256)) ([34226dd](34226dd))
* Normalize donation events ([#2299](#2299)) ([2624d53](2624d53))
* **perfmatters:** improve config ([267306e](267306e))
* prevent homepage from being unpublished ([#2307](#2307)) ([a151d53](a151d53))
* Remove the campaign rendered event ([#2301](#2301)) ([23caa1d](23caa1d))
* Stripe Subscriptions to WC subscriptions migrator ([#2298](#2298)) ([6904356](6904356)), closes [#2251](#2251)
* **wc:** force allowing subscription switching ([#2305](#2305)) ([c13e741](c13e741))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.106.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@leogermani leogermani mentioned this pull request Jun 6, 2023
Merged
6 tasks
@miguelpeixe miguelpeixe deleted the remove/campaign-rendered-event branch July 12, 2023 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkoo dkoo dkoo approved these changes

Assignees

@leogermani leogermani

Labels
released on @alpha released [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@leogermani @matticbot @dkoo