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

fix: execute tracking in an earlier hook #1314

Merged
merged 4 commits into from
Oct 5, 2023

Conversation

miguelpeixe
Copy link
Member

All Submissions:

Changes proposed in this Pull Request:

The existing approach for processing the pixel and click tracking processes the requests in the template_redirect hook, which can potentially be quite heavy being a hook called farther down the hook tree.

Removing the custom rewrite rule allows the hook to be executed early in init, which is executed much earlier, reducing the load of such requests.

The rewrite rule and the hook to template_redirect will continue to exist for backward compatibility, but every new newsletter sent will use a query parameter, which will be executed in the new hook.

How to test the changes in this Pull Request:

  1. While on the release branch, send a newsletter with links
  2. Check out this branch
  3. Open the newsletter in your inbox and make sure to allow images to render
  4. Click on any link in the newsletter
  5. Go to the newsletters dashboard table and confirm the Open and Click were tracked (backward compatibility)
  6. Send a new newsletter with links
  7. Open the newsletter in your inbox and make sure to allow images to render
  8. Click on any link in the newsletter
  9. Go to the newsletters dashboard table and confirm the Open and Click were tracked (query var approach)

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?

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.

Works as described!

@github-actions github-actions bot added [Status] Approved Ready to merge and removed [Status] Needs Review labels Oct 5, 2023
@miguelpeixe miguelpeixe merged commit e48aeab into release Oct 5, 2023
1 check passed
@miguelpeixe miguelpeixe deleted the hotfix/tracking-performance branch October 5, 2023 17:49
matticbot pushed a commit that referenced this pull request Oct 5, 2023
## [2.1.4](v2.1.3...v2.1.4) (2023-10-05)

### Bug Fixes

* execute tracking in an earlier hook ([#1314](#1314)) ([e48aeab](e48aeab))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.1.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Oct 5, 2023
# [2.2.0-alpha.2](v2.2.0-alpha.1...v2.2.0-alpha.2) (2023-10-05)

### Bug Fixes

* execute tracking in an earlier hook ([#1314](#1314)) ([e48aeab](e48aeab))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.2.0-alpha.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants