Sync: initialize Dedicated Sync endpoint earlier

[sergeymitr]

Proposed changes:

Initialize dedicated sync endpoint earlier in the init hook to avoid cookie conflicts.

Other information:

• Have you written new tests for your changes, if applicable?
• Have you checked the E2E test CI results, and verified that your changes do not break them?
• Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

p1686285707645099-slack-CBG1CP4EN

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

Make sure the code makes sense.

Do not checkout the code just yet, the issue may be a bit hard to reproduce.

1. Create a JN site with Woo, WCPay, Jetpack, and Smooth Generator.
2. Connect Jetpack, generated a few products.
3. Go to "WooCommerce -> Settings -> Accounts & Privacy" and enable "Allow customers to log into an existing account during checkout"
4. Create a new user.
5. In a private browser window, add a few products to the cart, go to checkout and login right there.
6. The products will probably still be in the cart as expected. Go to Cart and remove them. Then add a few products to the cart manually and remove them again. Make sure your cart is empty.
7. Close the window to clean the session, and repeat 6-7 a few times, and eventually you should lose the products in your card after logging in.
8. Checkout the PR and try a few more times, confirm the issue got fixed.

[github-actions]

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

• To test on WoA, go to the Plugins menu on a WordPress.com Simple site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack plugin. Once the plugin is active, go to Jetpack > Jetpack Beta > Jetpack and enable the fix/dedicated-sync-cookies branch.

• To test on Simple, run the following command on your sandbox:

  bin/jetpack-downloader test jetpack fix/dedicated-sync-cookies
  

Interested in more tips and information?

• In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
• Read more about our development workflow here: PCYsg-eg0-p2
• Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ All commits were linted before commit.
  
• ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

---

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team Review" label and ask someone from your team review the code. Once reviewed, it can then be merged.
If you need an extra review from someone familiar with the codebase, you can update the labels from "[Status] Needs Team Review" to "[Status] Needs Review", and in that case Jetpack Approvers will do a final review of your PR.

[fgiannar]

When we spawn Dedicated Sync, we send $_COOKIES with the request to help out with any firewall and/or caching functionality that might prevent us to ping the site directly.

I believe the proper fix would actually involve hooking the add_dedicated_sync_sender_init on init action (and exiting) as soon as possible, aka replace:

add_action( 'init', array( __CLASS__, 'add_dedicated_sync_sender_init' ), 90 ); with add_action( 'init', array( __CLASS__, 'add_dedicated_sync_sender_init' ), 0 );

[sergeymitr]

Hi @fgiannar.
Thanks for the suggestion. I applied your approach, works well.

[fgiannar]

Thanks for working on this, Sergey! Tests well 👍 Can we pls update the PR description and changelog as per the latest changes?

Also I'd appreciate it if @bisko could also take a look and confirm we are not missing anything :)

[csmcneill]

@bisko @sergeymitr @fgiannar As I'm handling comms for affected WCPay merchants, could one of y'all please provide me with some details about when we expect this to be released? Are we expecting to see it in a patched release (e.g., 12.2.2) or a major release (12.3.0)? And what would be an expected release date for said release?

[sergeymitr]

Hi @csmcneill.
We are not planning a Jetpack point release at the moment, and the next release (12.3.0) is scheduled for July 5th.

Another option is to upgrade the Sync package in WCPay and release the upgrade. I see that's it's due to upgrade anyway (1.47.7 vs 1.49.0), so it might be a good way to speed up the process.
If they decide to go down that road, we'll release the new version of Sync package (1.49.1) with the fix. If WCPay will include the latest package version, all Jetpack plugins will also use it, so the problem will be solved.

[naman03malhotra]

If they decide to go down that road, we'll release the new version of Sync package (1.49.1) with the fix. If WCPay will include the latest package version, all Jetpack plugins will also use it, so the problem will be solved.

@RadoslavGeorgiev this should be doable for 6.1.0 release, right? Would there be any specific testing other than critical flows?

[naman03malhotra]

@sergeymitr are there any particular changes we should be careful of while upgrading to 1.49.1?

@RadoslavGeorgiev @dat I was going through jetpack sync changelog, can you please verify If this change won't have any side effect on WCpayments.

[naman03malhotra]

Another option is to upgrade the Sync package in WCPay and release the upgrade. I see that's it's due to upgrade anyway (1.47.7 vs 1.49.0)

@sergeymitr I like this option. We have a release due next week. So we'll try to include the upgrade with WCPayments 6.1.0. Let us know when you release 1.49.1.

[sergeymitr]

Discussion continues here: p1686285707645099-slack-CBG1CP4EN
We reached the decision to wait for the next Jetpack release on July 4th.
Release of WCPay 6.1.0 is too close and will not allow enough time for testing of Jetpack packages. The plan is to get the packages upgraded in 6.2.0.

[samiff]

Noting that this PR had some unintended side effects (Woo mobile app new order notifications not being pushed).

Internal references:

• Slack discussion: p1688494436784579-slack-C01U2KGS2PQ
• Reported via P2: peaMlT-82-p2

We'll either fix this or revert this change before Jetpack 12.3 ships out tomorrow.

[samiff]

Also Woo sales stats issue that may be related: p9F6qB-cEF-p2