Guided Tour: Redirect to Login page if logged out #25498
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When you hit a valid route as a logged-out user, we should send you to the login page with a redirect back to the page you were trying to access. This is how all wp-admin links in core WP works. There have been fixes for individual pages (eg #24687 ) but this should be handled at the framework level. We have an email campaign using the Simple Payments Guided Tour #24627 and it freezes at a blank screen for a logged-out user. What this change does is to do a redirect for a valid section with the `enableLoggedOut` property set to true to the login screen with a redirect. A future PR could probably remove all bespoke uses of `import { redirectLoggedOut } from 'controller'` as well since this should supersede it.
mattwiebe
added
[Status] Needs Review
mattwiebe
added
[Status] In Progress
and removed
[Status] Needs Review
This is only trying to fix the `stats` route. The previous change broke signup and other critical views that begin logged-out. This will simply fix the Guided Tour.
mattwiebe
added
[Status] Needs e2e Testing
Stats
mattwiebe
changed the title
blowery
approved these changes
Jun 14, 2018
blowery
removed
[Status] Needs Review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
EDIT: This PR has been amended to more surgically focused. A framework-wide redirect for all logged-in pages to the login screen should be pursued, but this is tied to a time-sensitive project and I don't have the resources to pursue something on the framework level.
All that will happen now is that if you hit a URL beginning with
/stats, you will be redirected to the login page and then back to your original URL on success. This will enable the users who click in their email to the Guided Tour and are logged out to actually enter it.Original discussion left below for posterity. I will try to open a Framework issue later.
When you hit a valid route as a logged-out user, we should send you to the login page with a redirect back to the page you were trying to access. This is how all
wp-adminlinks in core WP works.There have been fixes for individual pages (eg #24687) but this should be handled at the framework level. We have an email campaign using the Simple Payments Guided Tour (#24627) and it freezes at a blank screen for a logged-out user, using the path
/stats/day/{siteName}/?tour=simplePaymentsEmailTour.What this change does is to do a redirect for a valid section with the
enableLoggedOutproperty set to true to the login screen with a redirect. A future PR could probably remove all bespoke uses ofimport { redirectLoggedOut } from 'controller'as well since this should supersede it.