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

Support User: Perform a complete page refresh when activating #3492

Merged
merged 5 commits into from
Feb 26, 2016

Conversation

jordwest
Copy link
Contributor

This performs a full page reload when a support user token is activated/deactivated to ensure all user data is fully refreshed. Between refreshes, the support token is stored in localstorage using the store module. On the following boot of Calypso, the User initialization sequence is intercepted before the original user is loaded, to ensure none of the original user's data is loaded.

Additionally, an error message is now displayed when an error occurs while fetching a token.

No other functionality has been changed; the testing procedure is the same (ref: p4TIVU-2OW-p2)

A few notes for review:

Fixes #3029, fixes #3067, fixes #3068 as well as various other non-reported issues (ref p4TIVU-2OW-p2)

cc @dllh @gwwar @mtias

@jordwest jordwest added [Status] In Progress [Feature Group] Support All things related to WordPress.com customer support. labels Feb 23, 2016
@jordwest jordwest force-pushed the update/support-user/full-refresh branch 3 times, most recently from ca0f1d7 to 1f74723 Compare February 25, 2016 08:01
@jordwest jordwest added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Feb 25, 2016
@dllh
Copy link
Member

dllh commented Feb 25, 2016

I gave this a test drive, and it's working well for me functionally.

@@ -179,7 +178,7 @@ export default connect(
const { isLoading, section, hasSidebar, isFullScreen, chunkName } = state.ui;
return {
isLoading,
isSupportUser: isSupportUser( state ),
isSupportUser: state.support.isSupportUser,
Copy link
Contributor

Choose a reason for hiding this comment

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

Were the selector imports becoming too lengthy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously this selector was using the support user and token in the Redux store to determine whether support user was active. Now that we're storing those outside the Redux store, I've opted for just a boolean flag in the Redux state to keep track of whether support user is active or not. We could keep the selector, but it would just be:

const isSupportUser = ( state ) => state.support.isSupportUser

@gwwar
Copy link
Contributor

gwwar commented Feb 26, 2016

👍 Nice work! ✨ It's really coming along.

One thing we may want to clear up in this PR or in an immediate follow up is that the currentUser id doesn't change back to the original user id after exiting the mode.

As is, I think it's good to 🚢 as an outstanding wip.

@jordwest jordwest force-pushed the update/support-user/full-refresh branch from 1f74723 to 65b8518 Compare February 26, 2016 01:25
@jordwest
Copy link
Contributor Author

Thanks for the review @gwwar. I've fixed up the typos and will create issues to track the other remaining bits.

For the time being I've fixed the issue with currentUser id not changing back to the original user by just nuking the store completely (as is done in User when it detects a change):

    rebootNormally() {
        debug( 'Rebooting Calypso normally' );

-       store.remove( STORAGE_KEY );
+       store.clear();
        window.location.reload();
    }

If there are no other objections I'll merge shortly

@gwwar
Copy link
Contributor

gwwar commented Feb 26, 2016

As long as you're awake for the deploy, go for it. 😄

@jordwest jordwest force-pushed the update/support-user/full-refresh branch from b340c9d to adf4a5c Compare February 26, 2016 02:29
jordwest added a commit that referenced this pull request Feb 26, 2016
…resh

Support User: Perform a complete page refresh when activating
@jordwest jordwest merged commit b6cfb58 into master Feb 26, 2016
@scruffian scruffian removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 26, 2016
@jordwest jordwest deleted the update/support-user/full-refresh branch February 26, 2016 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature Group] Support All things related to WordPress.com customer support.
Projects
None yet
5 participants