From b3915b7736c588dff6cf32622754a84aa0acaf42 Mon Sep 17 00:00:00 2001
From: Jeremy Herve <jeremy@jeremy.hu>
Date: Thu, 13 May 2021 13:00:24 +0200
Subject: [PATCH] Admin Page: display Sharing as new default tab for non-admins

This commit changes the behaviour of the Settings screen for connected non-admins.

Non-admins currently need access to the Settings screen for 2 reasons:
- Publicize (Sharing tab)
- Post By Email (Writing tab)

Until now, we always defaulted to the Writing tab for connected non-admins. This has 2 problems:
- We did not check if the Post By Email module was actually enabled, so when the module was disabled editors loading the settings page would only see a Writing tab and its title, but no content.
- Publicize is more popular than Post By Email today, so it's more likely that editors will be interested in the Publicize settings first, even if both PBE and Publicize are enabled.

This commit changes that default behaviour, and introduces checks for the active modules to avoid displaying tabs when we do not need to.

Previous work on this: #13541
---
 .../components/navigation-settings/index.jsx  | 41 +++++-----
 .../jetpack/_inc/client/settings/index.jsx    | 75 +++++++++++--------
 2 files changed, 68 insertions(+), 48 deletions(-)

diff --git a/projects/plugins/jetpack/_inc/client/components/navigation-settings/index.jsx b/projects/plugins/jetpack/_inc/client/components/navigation-settings/index.jsx
index d2a0116df37a0..b269d8b7df954 100644
--- a/projects/plugins/jetpack/_inc/client/components/navigation-settings/index.jsx
+++ b/projects/plugins/jetpack/_inc/client/components/navigation-settings/index.jsx
@@ -106,7 +106,7 @@ export const NavigationSettings = createReactClass( {
 	},
 
 	render: function () {
-		let navItems, sharingTab;
+		let navItems, sharingTab, writingTab;
 		if ( this.props.userCanManageModules ) {
 			navItems = (
 				<NavTabs selectedText={ this.props.routeName }>
@@ -192,6 +192,7 @@ export const NavigationSettings = createReactClass( {
 		} else if ( this.props.isSubscriber ) {
 			navItems = false;
 		} else {
+			// Show a sharing tab if the Publicize module is active and the user can publish.
 			if ( ! this.props.isModuleActivated( 'publicize' ) || ! this.props.userCanPublish ) {
 				sharingTab = '';
 			} else {
@@ -199,30 +200,34 @@ export const NavigationSettings = createReactClass( {
 					<NavItem
 						path="#sharing"
 						onClick={ this.handleClickForTracking( 'sharing' ) }
-						selected={ this.props.location.pathname === '/sharing' }
+						selected={
+							this.props.location.pathname === '/sharing' ||
+							this.props.location.pathname === '/settings'
+						}
 					>
 						{ _x( 'Sharing', 'Navigation item.', 'jetpack' ) }
 					</NavItem>
 				);
 			}
+
+			// Show a Writing tab if the Post By Email module is active and the user can publish.
+			if ( ! this.props.isModuleActivated( 'post-by-email' ) || ! this.props.userCanPublish ) {
+				writingTab = '';
+			} else {
+				writingTab = this.props.hasAnyOfTheseModules( [ 'post-by-email' ] ) && (
+					<NavItem
+						path="#writing"
+						onClick={ this.handleClickForTracking( 'writing' ) }
+						selected={ this.props.location.pathname === '/writing' }
+					>
+						{ _x( 'Writing', 'Navigation item.', 'jetpack' ) }
+					</NavItem>
+				);
+			}
 			navItems = (
 				<NavTabs selectedText={ this.props.routeName }>
-					{ this.props.hasAnyOfTheseModules( [ 'post-by-email' ] ) && (
-						<NavItem
-							path="#writing"
-							onClick={ this.handleClickForTracking( 'writing' ) }
-							selected={
-								this.props.location.pathname === '/writing' ||
-								this.props.location.pathname === '/settings'
-							}
-						>
-							{ _x( 'Writing', 'Navigation item.', 'jetpack' ) }
-						</NavItem>
-					) }
-					{
-						// Give only Publicize to non admin users
-						sharingTab
-					}
+					{ writingTab }
+					{ sharingTab }
 				</NavTabs>
 			);
 		}
diff --git a/projects/plugins/jetpack/_inc/client/settings/index.jsx b/projects/plugins/jetpack/_inc/client/settings/index.jsx
index 119d7fd4a1b32..db953b16b1c29 100644
--- a/projects/plugins/jetpack/_inc/client/settings/index.jsx
+++ b/projects/plugins/jetpack/_inc/client/settings/index.jsx
@@ -1,11 +1,10 @@
 /**
  * External dependencies
- *
- * @format
  */
-
 import React from 'react';
 import { __, sprintf } from '@wordpress/i18n';
+import { connect } from 'react-redux';
+import { withRouter } from 'react-router-dom';
 
 /**
  * Internal dependencies
@@ -18,71 +17,87 @@ import Security from 'security';
 import Sharing from 'sharing';
 import Traffic from 'traffic';
 import Writing from 'writing';
-import { withRouter } from 'react-router-dom';
+import { isModuleActivated as isModuleActivatedSelector } from 'state/modules';
 
 class Settings extends React.Component {
 	static displayName = 'SearchableSettings';
 
 	render() {
+		const {
+			location = { pathname: '' },
+			rewindStatus,
+			searchTerm,
+			siteAdminUrl,
+			siteRawUrl,
+			userCanManageModules,
+		} = this.props;
+		const { pathname } = location;
 		const commonProps = {
-			searchTerm: this.props.searchTerm,
-			rewindStatus: this.props.rewindStatus,
-			userCanManageModules: this.props.userCanManageModules,
+			searchTerm,
+			rewindStatus,
+			userCanManageModules,
 		};
 
 		return (
 			<div className="jp-settings-container">
 				<div className="jp-no-results">
-					{ commonProps.searchTerm
+					{ searchTerm
 						? sprintf(
 								/* translators: placeholder is a searchterm entered in searchform. */
 								__( 'No search results found for %s', 'jetpack' ),
-								commonProps.searchTerm
+								searchTerm
 						  )
 						: __( 'Enter a search term to find settings or close search.', 'jetpack' ) }
 				</div>
 				<Security
-					siteAdminUrl={ this.props.siteAdminUrl }
-					siteRawUrl={ this.props.siteRawUrl }
+					siteAdminUrl={ siteAdminUrl }
+					siteRawUrl={ siteRawUrl }
 					active={
-						'/security' === this.props.location.pathname ||
-						( '/settings' === this.props.location.pathname && commonProps.userCanManageModules )
+						'/security' === pathname || ( '/settings' === pathname && userCanManageModules )
 					}
 					{ ...commonProps }
 				/>
 				<Discussion
-					siteRawUrl={ this.props.siteRawUrl }
-					active={ '/discussion' === this.props.location.pathname }
-					{ ...commonProps }
-				/>
-				<Performance
-					active={ '/performance' === this.props.location.pathname }
+					siteRawUrl={ siteRawUrl }
+					active={ '/discussion' === pathname }
 					{ ...commonProps }
 				/>
+				<Performance active={ '/performance' === pathname } { ...commonProps } />
 				<Traffic
-					siteRawUrl={ this.props.siteRawUrl }
-					siteAdminUrl={ this.props.siteAdminUrl }
-					active={ '/traffic' === this.props.location.pathname }
+					siteRawUrl={ siteRawUrl }
+					siteAdminUrl={ siteAdminUrl }
+					active={ '/traffic' === pathname }
 					{ ...commonProps }
 				/>
 				<Writing
-					siteAdminUrl={ this.props.siteAdminUrl }
+					siteAdminUrl={ siteAdminUrl }
 					active={
-						'/writing' === this.props.location.pathname ||
-						( '/settings' === this.props.location.pathname && ! commonProps.userCanManageModules )
+						'/writing' === pathname ||
+						( ! userCanManageModules &&
+							this.props.isModuleActivated( 'post-by-email' ) &&
+							! this.props.isModuleActivated( 'publicize' ) )
 					}
 					{ ...commonProps }
 				/>
 				<Sharing
-					siteAdminUrl={ this.props.siteAdminUrl }
-					active={ '/sharing' === this.props.location.pathname }
+					siteAdminUrl={ siteAdminUrl }
+					active={
+						'/sharing' === pathname ||
+						( '/settings' === pathname &&
+							! userCanManageModules &&
+							this.props.isModuleActivated( 'publicize' ) )
+					}
 					{ ...commonProps }
 				/>
-				<Privacy active={ '/privacy' === this.props.location.pathname } { ...commonProps } />
-				<SearchableModules searchTerm={ this.props.searchTerm } />
+				<Privacy active={ '/privacy' === pathname } { ...commonProps } />
+				<SearchableModules searchTerm={ searchTerm } />
 			</div>
 		);
 	}
 }
 
-export default withRouter( Settings );
+export default connect( state => {
+	return {
+		isModuleActivated: module => isModuleActivatedSelector( state, module ),
+	};
+} )( withRouter( Settings ) );