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

People: Fixes RoleSelect issue with localStorage cleared #3542

Merged
merged 1 commit into from
Feb 24, 2016

Conversation

ebinnion
Copy link
Contributor

Fixes #3442

Previously, the RoleSelect did not work properly when going to /people/new/$site with a cleared localStorage. After some digging, this seems to have been because we were not re-fetching once site was set properly. 😱

This PR fixes that by ensuring that we fetch in componentWillReceiveProps. Note that while we call this.fetchRoles() each time we receive new props, we do not in fact fetch each time. If a site already has roles, fetchRoles() bails early.

To test:

  • Checkout update/people-invites-localstorage branch
  • Go to /people/new/$site where $site is a WP.com site
  • In console, localStorage.clear()
  • Refresh
  • Ensure that all site roles load
  • Go to /people/team/$site where $site is a WP.com or Jetpack site
  • Ensure that roles load properly

cc @lezama for review

@ebinnion ebinnion added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. People Management labels Feb 24, 2016
@ebinnion ebinnion self-assigned this Feb 24, 2016
@ebinnion ebinnion added this to the People Management: m7 milestone Feb 24, 2016
@@ -20,7 +20,7 @@ import sitesList from 'lib/sites-list';
/**
* Module variables
*/
const debug = debugFactory( 'calypso:role-select' );
const debug = debugFactory( 'calypso:my-sites:people:role-select' );
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@lezama lezama added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 24, 2016
ebinnion added a commit that referenced this pull request Feb 24, 2016
…torage

People: Fixes RoleSelect issue with localStorage cleared
@ebinnion ebinnion merged commit f2ac915 into master Feb 24, 2016
@ebinnion ebinnion deleted the update/people-invites-localstorage branch February 24, 2016 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

People: Handle cleared localStorage for invite people
3 participants