Skip to content

Conversation

@itsmeichigo
Copy link
Contributor

@itsmeichigo itsmeichigo commented Jun 8, 2023

Following up with an internal discussion: p1686154005250689/1686153929.996589-slack-C03L1NF1EA3

Description

@joshheald found that I made a mistake in the check for plan site sync as the logic check for skipping the site plan sync should check for WPCom login instead of the other way around. This error was from #8638 when I switch the logic from using siteID to using isAuthenticatedWithoutWPCom.

Testing instructions

It's not straightforward to test DefaultStoresManager for this fix, and the site sync doesn't work properly after creating a free trial site so it's tricky to test this issue. I'd suggest putting a breakpoint inside the if block to make sure that it's triggered only for when you log in with WPCom.

Screenshots

N/A


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@itsmeichigo itsmeichigo added the type: bug A confirmed bug. label Jun 8, 2023
@itsmeichigo itsmeichigo added this to the 14.0 milestone Jun 8, 2023
@wpmobilebot
Copy link
Collaborator

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr9900-fcf223f on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@itsmeichigo itsmeichigo marked this pull request as ready for review June 8, 2023 04:21
@selanthiraiyan selanthiraiyan self-assigned this Jun 8, 2023
Copy link
Contributor

@selanthiraiyan selanthiraiyan left a comment

Choose a reason for hiding this comment

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

I validated that the if block is called only when logging in using WPCOM creds. It doesn't fire when logging in using wp-admin creds.

🚀

@itsmeichigo itsmeichigo merged commit 1a23d62 into trunk Jun 8, 2023
@itsmeichigo itsmeichigo deleted the bug/plan-sync-for-wpcom branch June 8, 2023 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug A confirmed bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants