Google Apps: Show Current User #3584

umurkontaci · 2016-02-26T03:42:20Z

Fixes #603.

Requires D1179-code

This changes enables the users to see the current list of Google Apps users per domain and per site.

There quite some changes in this PR, namely I moved the Google Apps store to Redux. Since this component also needs non-Redux data as well, they are glued in the data component.

Screenshots

Sitewide - Has Google Apps

Per domain - Has Google Apps

Sitewide - No domains

Sitewide - Only mapped domains

Per domain - Not eligible

Testing

Go to domain management -> Email
Ensure they look like the screenshots above and the links work.
Ensure Add button is hidden if you're not an admin
Go to domain management -> Select domain -> Click Email
Ensure they look like the screenshots above and the links work

/cc: @klimeryk @stephanethomas @mtias for code review (first attempt at Redux – extra eyes welcome)
/cc: @breezyskies for design
/cc: @ranh for copy

klimeryk · 2016-02-27T02:18:21Z

client/lib/wpcom-undocumented/lib/undocumented.js

+};
+
+Undocumented.prototype.googleAppsFilterBySiteId = function( siteId, fn ) {
+	debug( '/sites/:siteId/google-apps/site' );


It should be /sites/:siteId/google-apps (without /site at the end).
Stupid nitpick, but someone might get confused by this print and search for a non-existing endpoint.

klimeryk · 2016-03-01T02:18:13Z

I've encountered an error in weird place - when navigating away from the Email view.
Steps to reproduce:

Visit the Email view: /domains/manage/email/example.com
Try to navigate to your profile (/me) by clicking your profile picture in the upper right corner
You'll get this error in the console:

Failed propType: Required prop domains was not specified in Email. Check the render method of StoreConnection.

And then periodically:
TypeError: Cannot read property 'hasLoadedFromServer' of undefined in index.jsx:110

umurkontaci · 2016-03-01T23:17:48Z

I've encountered an error in weird place - when navigating away from the Email view.

Weird bug indeed. During a navigation we unset the selected site before unmounting the component and this led to passing undefined for domains prop. Fixed it by sending an empty object as default.

umurkontaci · 2016-03-01T23:19:00Z

@klimeryk @gwwar Thanks for the review! I've addressed all the concerned mentioned above. I made them into separate commit to let you keep track of what happened since; but before merging I'll rebase the commits – no worries on that part.

klimeryk · 2016-03-02T01:13:09Z

client/my-sites/upgrades/domain-management/style.scss

-		background: url('/calypso/images/upgrades/google-apps-logo.png');
-		background-position: left center;
-		background-repeat: no-repeat;
+		//noinspection CssUnknownTarget


It looks like a weird comment creeped in ;)

It stops Webstorm from complaining about unknown target. But I guess we're not using anything to supress Webstorm errors and it might be best to remove it actually

klimeryk · 2016-03-02T01:33:54Z

Thanks, @umurkontaci - the issue I've reported has indeed been fixed 👍
I'll let @gwwar comment on the redux improvements :)

I've tested the "no domains" situation:

Per domain - Not eligible
There shouldn't be direct links to this page

I actually do see an Email link which leads to this view for my mapped domain :

Is this correct? I mean, I think it's a good idea to show the user some information on why he can't add email to his domain right now. But you mentioned "There shouldn't be direct links to this page" so I'm double checking :)

Sitewide - No domains

This scenario shows a "You don't have any domains yet." screen when I do have a domain, it's just a mapped domain. This wording could lead to some confusion - maybe we could show a more informative message when the user has domains, but none of them are eligible?

ranh · 2016-03-03T10:19:28Z

@umurkontaci I would change the notice for mapped domains slightly:

Google Apps is not supported on this domain
Only domains registered with WordPress.com are eligible for Google Apps.

aduth · 2016-03-07T19:20:46Z

I did notice there were 0 tests for this subtree. If you can spare a moment, please rebase from master to pick up #3773 that lets us use a single test runner for client/state and add tests for basic cases.

+1 to tests from me as well. Would appreciate some function docs too. Nice-to-haves, but go a long way to help others looking over the code.

umurkontaci · 2016-03-07T21:25:56Z

+1 to tests from me as well. Would appreciate some function docs too. Nice-to-haves, but go a long way to help others looking over the code.

Thanks for the review!

@gwwar and @aduth Added some tests and docs and addressed all the things above!

gwwar · 2016-03-07T21:56:15Z

client/state/google-apps-users/test/reducer.js

+		} );
+
+		it( 'should not have duplicate items', () => {
+			const state = items( [ { email: '[email protected]' } ],


With the reducer tests, we also like to use deepFreeze to make sure we don't accidentally mutate current state. Usage looks a bit like:

import deepFreeze from 'deep-freeze'; const originalState = deepFreeze( [ { email: '[email protected]' } ] ); const state = items( originalState, ...

gwwar · 2016-03-07T21:59:21Z

👍 Thanks for adding tests! ⭐ This redux subtree is good to 🚢 after adding deepFreeze to the reducer tests.

umurkontaci · 2016-03-08T03:54:58Z

@gwwar Thank you for the review! Added deepFreeze and rebased the commits. Waiting for a final green light :)

gwwar · 2016-03-08T15:09:26Z

👍 Redux changes are good to 🚢 I ran into some trouble testing the full flow due to my domain missing WWD info atm, but if @klimeryk has already verified behavior I'm fine with that. Looks like you'll want to rebase, before

…il tab (Fixes #603)

…ase and enable mapped domains to use email forwarding

klimeryk · 2016-03-08T17:03:30Z

Tested again, after the rebase and resolving conflicts - looks A-OK.

Google Apps: Show Current User

umurkontaci added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR G Suite labels Feb 26, 2016

umurkontaci self-assigned this Feb 26, 2016

klimeryk reviewed Feb 27, 2016
View reviewed changes

klimeryk 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 Mar 1, 2016

umurkontaci force-pushed the update/603-current-gapps branch from 5ed15a6 to 445f9d4 Compare March 1, 2016 23:16

umurkontaci 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 Mar 1, 2016

klimeryk reviewed Mar 2, 2016
View reviewed changes

umurkontaci force-pushed the update/603-current-gapps branch from 445f9d4 to a10deea Compare March 4, 2016 18:27

umurkontaci force-pushed the update/603-current-gapps branch 2 times, most recently from cf00a31 to 56ece0b Compare March 7, 2016 21:25

gwwar reviewed Mar 7, 2016
View reviewed changes

umurkontaci force-pushed the update/603-current-gapps branch from 56ece0b to 5cb59e4 Compare March 8, 2016 03:51

umurkontaci added 12 commits March 8, 2016 08:47

WPCOM: Add "googleAppsFilterBySiteId" endpoint (#603)

1ff4533

Redux: Add Google Apps Users to state

2f88e34

Domains: Remove old references to Google Apps data out of Redux (#603)

60e1b03

Domains: Styling for Google Apps Users List (#603)

f0a4a0a

Domains: Refactor Email Page (#603)

a714b81

Domains: Show information about purchased Google Apps licenses on Ema…

75a18b7

…il tab (Fixes #603)

Domains: Add a check for supported domain to ensure they're not banned

25ee5e4

Domains: Add getRegisteredDomains and getMappedDomains functions

d1a083b

Domains: Rename canAddEmail to hasGoogleAppsSupportedDomain

9f23d62

Google Apps: Handle having a mapped domain but no registered domain c…

83c389b

…ase and enable mapped domains to use email forwarding

Google Apps: Use selectedSite.slug instead of selectedSite.domain

f402b0e

State: Google Apps Users tests and docs

492692a

umurkontaci force-pushed the update/603-current-gapps branch from 5cb59e4 to 492692a Compare March 8, 2016 16:53

umurkontaci added a commit that referenced this pull request Mar 8, 2016

Merge pull request #3584 from Automattic/update/603-current-gapps

f6aa008

Google Apps: Show Current User

umurkontaci merged commit f6aa008 into master Mar 8, 2016

umurkontaci removed the [Status] Ready to Merge label Mar 8, 2016

umurkontaci deleted the update/603-current-gapps branch March 8, 2016 17:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Google Apps: Show Current User #3584

Google Apps: Show Current User #3584

umurkontaci commented Feb 26, 2016

klimeryk Feb 27, 2016

umurkontaci Mar 1, 2016

klimeryk commented Mar 1, 2016

umurkontaci commented Mar 1, 2016

umurkontaci commented Mar 1, 2016

klimeryk Mar 2, 2016

umurkontaci Mar 2, 2016

umurkontaci Mar 4, 2016

klimeryk commented Mar 2, 2016

ranh commented Mar 3, 2016

aduth commented Mar 7, 2016

umurkontaci commented Mar 7, 2016

gwwar Mar 7, 2016

gwwar commented Mar 7, 2016

umurkontaci commented Mar 8, 2016

gwwar commented Mar 8, 2016

klimeryk commented Mar 8, 2016

Google Apps: Show Current User #3584

Google Apps: Show Current User #3584

Conversation

umurkontaci commented Feb 26, 2016

Screenshots

Sitewide - Has Google Apps

Per domain - Has Google Apps

Sitewide - No domains

Sitewide - Only mapped domains

Per domain - Not eligible

Testing

klimeryk Feb 27, 2016

Choose a reason for hiding this comment

umurkontaci Mar 1, 2016

Choose a reason for hiding this comment

klimeryk commented Mar 1, 2016

umurkontaci commented Mar 1, 2016

umurkontaci commented Mar 1, 2016

klimeryk Mar 2, 2016

Choose a reason for hiding this comment

umurkontaci Mar 2, 2016

Choose a reason for hiding this comment

umurkontaci Mar 4, 2016

Choose a reason for hiding this comment

klimeryk commented Mar 2, 2016

ranh commented Mar 3, 2016

aduth commented Mar 7, 2016

umurkontaci commented Mar 7, 2016

gwwar Mar 7, 2016

Choose a reason for hiding this comment

gwwar commented Mar 7, 2016

umurkontaci commented Mar 8, 2016

gwwar commented Mar 8, 2016

klimeryk commented Mar 8, 2016