People: Invite: Add site to sites-list after accepting invite. #1261

lezama · 2015-12-04T02:46:44Z

Use flux to add the site

How to test

checkout update/invites-add-site-right-after-accept

to check

both flows logged in and logged out flows should still work
after accepting an invite you should be redirected to the site/post list

(I still need to make it work for logged out users)

cc @ebinnion @roccotripaldi

ebinnion · 2015-12-04T21:54:24Z

client/lib/sites-list/index.js

@@ -16,7 +22,15 @@ module.exports = function() {
 				case 'DELETE_SITE':
 					_sites.removeSite( action.site );
 					break;
-
+				case InvitesActionTypes.INVITE_ACCEPTED:


I like using flux to handle this. My first thought was to just clear the sites list and re-fetch, but this should be a bit more elegant 👍

ebinnion · 2015-12-04T22:12:08Z

It looks like we're no longer auto-filling the email address:

Also, when I tried to accept an invite while logged out, I got this error:

I saw above where you said, I still need to make it work for logged out users. But, just above that, it also says, both flows logged in and logged out flows should still work. So, I wasn't sure if this error is expected.

Lastly, we should probably fix it so the error doesn't occur before merging this PR.

ebinnion · 2015-12-04T22:14:50Z

Also, I got this error when trying to accept an invite while logged in:

lezama · 2015-12-04T22:42:01Z

I addressed both last comments, still working on make it work for logged out users. Thanks for the feedback @ebinnion, I will ping you again once everything in place.

lezama · 2015-12-07T15:55:59Z

@ebinnion this should be ready for another review :)

ebinnion · 2015-12-07T19:49:19Z

client/my-sites/invites/invite-message/index.jsx

@@ -30,7 +30,7 @@ export default React.createClass( {
 		}
 		const { accepted, declined, siteId } = this.state;
 		if ( accepted ) {
-			const site = this.props.sites.getSite( siteId );
+			const site = this.props.sites.getSite( parseInt( siteId ) );


parseInt() needs the second parameter of 10 in this case.

const site = this.props.sites.getSite( parseInt( siteId, 10 ) );

ebinnion · 2015-12-07T20:14:20Z

This tests fine for the logged out invite accept flow, but I'm getting an error when trying to accept an invite while logged in.

…e user

lezama · 2015-12-09T15:23:01Z

@ebinnion:

I'm getting an error when trying to accept an invite while logged in.

Should be fixed now \o/

ebinnion · 2015-12-09T18:02:32Z

I just tested again, and I'm now actually getting two errors when trying to accept an invite.

ebinnion · 2015-12-09T18:07:06Z

client/lib/sites-list/index.js

+					if ( sites.length === 0 ) {
+						action.invite.site.primary = true;
+					}
+					sites.push( action.invite.site );


It seems like we keep having errors that are likely due to us trying to add our idea of a site object into the sites list. We should try to find some way to normalize our data into a site object. Perhaps we should try creating a proper site object using lib/site?

We are already doing that, _sites.update calls createSiteObject on each new site that we pass on sites

ebinnion · 2015-12-09T19:03:00Z

@lezama and I did a screenhero session and I was not able to repro the errors that I mentioned in the last comment. I pointed out an issue where I could no longer dismiss notices. Other than that, functionally, this seems to work well.

lezama · 2015-12-10T11:53:33Z

I pointed out an issue where I could no longer dismiss notices

Fixed in a8d3d2f

…t-after-accept People: Invite: Add site to sites-list after accepting invite.

lezama added [Status] In Progress People Management labels Dec 4, 2015

lezama added this to the People Management: m6 milestone Dec 4, 2015

lezama self-assigned this Dec 4, 2015

This was referenced Dec 4, 2015

People: Invites: Redirect to /posts/%site% #1185

Closed

People: Invites: Normalize get invite api response #1267

Merged

lezama force-pushed the update/invites-add-site-right-after-accept branch from 7301968 to c6ca266 Compare December 4, 2015 19:25

ebinnion reviewed Dec 4, 2015
View reviewed changes

lezama 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 Dec 7, 2015

ebinnion reviewed Dec 7, 2015
View reviewed changes

ebinnion added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 7, 2015

lezama added 6 commits December 9, 2015 12:21

Invites: Dispatch actions instead of using callback

58aa0d3

Invites: Add the new site to sites-list before redirecting

f0a7519

Invites: increment sites count after accepting an invite

fecb660

Invites: callback removed

3d81f72

Invites: split long line

1c36278

Sidebar: Don't display items if we don't have the capabilities for th…

e5b0c0c

…e user

lezama added 4 commits December 9, 2015 12:21

Invites: Fix auto-filling the email address

150a767

Invites: fix logged out users sign up

fd7f5c9

Invites: remove unused query param

e9b6ffb

Invites: Add radix to parseInt calls

5e85624

lezama force-pushed the update/invites-add-site-right-after-accept branch from aa1677c to 5e85624 Compare December 9, 2015 15:21

lezama added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Author Reply labels Dec 9, 2015

ebinnion reviewed Dec 9, 2015
View reviewed changes

Invites: use onDismissClick instead of onClick

a8d3d2f

lezama added a commit that referenced this pull request Dec 10, 2015

Merge pull request #1261 from Automattic/update/invites-add-site-righ…

eea4616

…t-after-accept People: Invite: Add site to sites-list after accepting invite.

lezama merged commit eea4616 into master Dec 10, 2015

scruffian removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 10, 2015

lezama deleted the update/invites-add-site-right-after-accept branch December 10, 2015 12:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

People: Invite: Add site to sites-list after accepting invite. #1261

People: Invite: Add site to sites-list after accepting invite. #1261

lezama commented Dec 4, 2015

ebinnion Dec 4, 2015

ebinnion commented Dec 4, 2015

ebinnion commented Dec 4, 2015

lezama commented Dec 4, 2015

lezama commented Dec 7, 2015

ebinnion Dec 7, 2015

ebinnion commented Dec 7, 2015

lezama commented Dec 9, 2015

ebinnion commented Dec 9, 2015

ebinnion Dec 9, 2015

lezama Dec 9, 2015

ebinnion commented Dec 9, 2015

lezama commented Dec 10, 2015

People: Invite: Add site to sites-list after accepting invite. #1261

People: Invite: Add site to sites-list after accepting invite. #1261

Conversation

lezama commented Dec 4, 2015

ebinnion Dec 4, 2015

Choose a reason for hiding this comment

ebinnion commented Dec 4, 2015

ebinnion commented Dec 4, 2015

lezama commented Dec 4, 2015

lezama commented Dec 7, 2015

ebinnion Dec 7, 2015

Choose a reason for hiding this comment

ebinnion commented Dec 7, 2015

lezama commented Dec 9, 2015

ebinnion commented Dec 9, 2015

ebinnion Dec 9, 2015

Choose a reason for hiding this comment

lezama Dec 9, 2015

Choose a reason for hiding this comment

ebinnion commented Dec 9, 2015

lezama commented Dec 10, 2015