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 where if a user isn't logged into wpcom, go to blank screen #8951

Merged
merged 3 commits into from
Mar 3, 2018

Conversation

withinboredom
Copy link
Contributor

When viewing the plans page, it is possible to be logged out of wpcom, this makes sure they go to the right flow depending on whether they're logged in or not by using the jetpack redirector.

Due to Automattic/wp-calypso#22857, we need to control the final destination of these links without requiring a jetpack release.

Changes proposed in this Pull Request:

  • Adds the wp-admin user id to the redux state tree
  • Adds a reducer to retrieve the wp-admin user id
  • Sends the wp-admin user id to the destination (so tracks aliasing works correctly)
  • Links go to the jetpack redirector where it makes sense so they go to the right place

Testing instructions:

Proposed changelog entry for your changes:

@withinboredom withinboredom added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Pri] High [Type] Quick Fix [Focus] FixTheFlows Plans labels Feb 28, 2018
@withinboredom withinboredom added this to the 5.9 milestone Feb 28, 2018
@withinboredom withinboredom self-assigned this Feb 28, 2018
@withinboredom withinboredom requested a review from a team as a code owner February 28, 2018 19:54
@withinboredom
Copy link
Contributor Author

Failing tests look unrelated to this PR

@withinboredom
Copy link
Contributor Author

fixes #8953

@oskosk
Copy link
Contributor

oskosk commented Mar 1, 2018

@withinboredom could you get someone from Growth to approve this one so we can get it cherry-picked to the release branch?

@mattwiebe
Copy link
Contributor

This works and tests as expected, and avoids the blank screen in Calypso in a logged-out state.

Both logged-in and -out flows need improving on the Calypso side, but that's fine. This keeps us in control and we can iterate there without needing to ship a Jetpack release.

@withinboredom
Copy link
Contributor Author

@oskosk is there some reason this hasn't been merged into the release yet?

Copy link
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

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

LGTM

@oskosk
Copy link
Contributor

oskosk commented Mar 3, 2018

@withinboredom . not other than I've been intermitently offline during the meetup I'm attending during this week. I'm doing it now

@oskosk oskosk merged commit 4e272a1 into master Mar 3, 2018
@oskosk oskosk deleted the fix/plans-tracking branch March 3, 2018 23:19
@oskosk
Copy link
Contributor

oskosk commented Mar 3, 2018

Cherry-picked to branch-5.9 in 35310fd

@kraftbj kraftbj removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] FixTheFlows Plans [Pri] High [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants