Sharing: prevent notice for non-admins

[jkudish]

Currently non-admins (specifically authors and editors) receive the following notice when visiting /sharing:

We only need site settings for buttons, not for connections. So this PR seeks to move the call to fetch the site settings to the buttons controller. This prevents the unnecessary notice for non-admins.

cc @aduth for review

[aduth]

The connections page needs to have access to site settings for Eventbrite:

wp-calypso/client/my-sites/sharing/connections/services/eventbrite.js

Lines 17 to 27 in eb9131a

	getConnections: function( connections, serviceName, siteId ) {
	var site = siteId ? sites.getSite( siteId ) : sites.getSelectedSite();
	if ( ! site || ! site.settings ) {
	return [];
	}
	return connections.filter( function( connection ) {
	return site.settings.eventbrite_api_token === connection.keyring_connection_ID;
	} );
	},

I think what we need to do is perform fetchSettings and show the Eventbrite connection option only if site.user_can_manage is true.

[jkudish]

Ah good point, missed the EB part of this. Any idea how to make the request without showing the notice and without doing a huge refactor of how the Site object currently works?

[aduth]

@jkudish - I pushed revisions to exclude the Eventbrite service from the Connections screen if the user cannot manage site settings: 6a09680

[jkudish]

Noting that with #1700 applied, I am getting this warning on the post editor as well (with an a user who's an author). That was without this PR applied. Going to check the combination of both shortly.

[jkudish]

With this and #1700 both applied, as an author on a Jetpack site:

• I can fully manage my connections w/out a notice 👍 :
• I still get a notice in the Post Editor 👎

[jkudish]

Testing as an Editor on a JP Site, I get the following warning in the console:

Warning: Failed propType: Invalid prop `label` supplied to `SharingServiceExample`. Check the render method of `SharingServiceExamples`.

[jkudish]

Testing as a Contributor on a JP site. I wouldn't expect the /sharing page to load at all, but it does. However, it doesn't appear in the sidebar, and I cannot actually add connections:

I think we just need to add an error message/block the page from loading services when ! current_user_can( 'publish_posts' )

[jkudish]

As an Author on a WP.com site, I still see Eventbrite on the connections page. However, it correctly doesn't let me add the connection:

Confirming that as an Administrator I can still manage everything as expected, however.

[aduth]

I still get a notice in the Post Editor 👎

Let's address this separately.

Testing as an Editor on a JP Site, I get the following warning in the console:

This is also true in master, so let's similarly address separately.

Testing as a Contributor on a JP site. I wouldn't expect the /sharing page to load at all, but it does. However, it doesn't appear in the sidebar, and I cannot actually add connections:

This is partially covered by #429, but I think we can do a bit more here with the filtering of eligible services since we're already making changes anyways. I'll look to update shortly.

As an Author on a WP.com site, I still see Eventbrite on the connections page. However, it correctly doesn't let me add the connection:

This sounds like a bug. I'll take a look.

[aduth]

As an Author on a WP.com site, I still see Eventbrite on the connections page. However, it correctly doesn't let me add the connection:

Can you double-check this? I'm unable to reproduce, and have found sometimes in testing my extra accounts I accidentally test against WordPress.com instead of calypso.localhost .

[aduth]

Now hiding Publicize services for contributors in 4ac7b95.

[aduth]

Rebased and squashed a few commits. I decided to go ahead and fix #1750 here since it was a very simple fix. See 77cf51a (and docs on PropTypes.node).

[jkudish]

Re-tested this PR with the latest code and all the different combinations of WP.com/Jetpack site + different permissions. It now works exactly as expected everywhere. Thanks for pushing through on this.

Reviewed the code too, LGTM 🚢

I think it'd be good to update the REST API's /meta/external-services endpoint to return required_capability so that we don't have to hardcode these values in Calypso. Of course, this would be addressed separately.

[aduth]

I think it'd be good to update the REST API's /meta/external-services endpoint to return required_capability so that we don't have to hardcode these values in Calypso. Of course, this would be addressed separately.

Yeah, that sounds like a good idea.