Skip to content

Commit 51bd708

Browse files
kraftbjkraftbj
authored
Settings: Remove the Security tab when the Protect page is on (#53741)
* Settings: Remove the Security tab when the Protect page is on With the protect-dashboard module active, the Settings app drops the Security tab, sends #/security to the Protect page's Settings tab, opens the first remaining tab for #/settings, and answers searches for moved settings with a card linking to Protect. Sites without the Protect page, including WordPress.com sites, keep the tab as it is. My Jetpack's firewall tooltip now links to #/security, so it reaches Protect when the page is on and the Security tab otherwise. * My Jetpack: Leave the Protect card tooltip link to PR 53744 That PR repoints the tooltip to the Protect page directly, and its fallback still reaches the Security tab when the page is off. * Settings: Point Backup and Anti-spam searches to their own pages With the Protect page on, a Settings search for Backup or Akismet found nothing. It now gets a card linking to the Backup page (or My Jetpack's Backup page where the dashboard isn't set up) and to Akismet Anti-spam. Also cover the #/security redirect with a test of Main's router, through a small openProtectSettings() helper, and drop the admin check there: renderMainContent already returns early for non-admins.
1 parent d7ffe66 commit 51bd708

13 files changed

Lines changed: 399 additions & 18 deletions

File tree

‎projects/plugins/jetpack/_inc/client/components/settings-nav-tabs/index.jsx‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
hasAnyOfTheseModules,
1717
hasAnyPerformanceFeature,
1818
hasAnySecurityFeature,
19+
hasProtectPage as _hasProtectPage,
1920
isModuleActivated,
2021
} from 'state/modules';
2122
import { filterSearch, getSearchTerm } from 'state/search';
@@ -33,11 +34,6 @@ const TAB_LABELS = {
3334
'/earn': _x( 'Monetize', 'Navigation item.', 'jetpack' ),
3435
};
3536

36-
// Routes that should highlight a different tab (e.g. /settings → /security).
37-
const ROUTE_ALIASES = {
38-
'/settings': '/security',
39-
};
40-
4137
/**
4238
* Settings page tab navigation using `@wordpress/ui` Tabs.
4339
*
@@ -51,6 +47,7 @@ const SettingsNavTabs = props => {
5147
isWoASite,
5248
userCanPublishPosts,
5349
hasSecurityFeature,
50+
hasProtectPage,
5451
hasPerformanceFeature,
5552
hasModules,
5653
isModuleActive,
@@ -61,9 +58,6 @@ const SettingsNavTabs = props => {
6158
const location = useLocation();
6259
const navigate = useNavigate();
6360

64-
// Resolve the active tab value from the current route.
65-
const activeTab = ROUTE_ALIASES[ location.pathname ] || location.pathname;
66-
6761
const trackNavClick = target => {
6862
analytics.tracks.recordJetpackClick( {
6963
target: 'nav_item',
@@ -106,7 +100,7 @@ const SettingsNavTabs = props => {
106100
const visibleTabs = [];
107101

108102
if ( userCanManageModules ) {
109-
if ( hasSecurityFeature ) {
103+
if ( hasSecurityFeature && ! hasProtectPage ) {
110104
visibleTabs.push( '/security' );
111105
}
112106
if ( hasPerformanceFeature ) {
@@ -155,6 +149,18 @@ const SettingsNavTabs = props => {
155149
}
156150
}
157151

152+
// /settings shows Security to admins, so with the Protect page it opens the first tab instead.
153+
const isSettingsRoot = '/settings' === location.pathname;
154+
const firstTab = userCanManageModules && hasProtectPage ? visibleTabs[ 0 ] : undefined;
155+
const activeTab = isSettingsRoot ? ( firstTab ?? '/security' ) : location.pathname;
156+
const shouldOpenFirstTab = isSettingsRoot && !! firstTab && ! searchFromUrl;
157+
158+
useEffect( () => {
159+
if ( shouldOpenFirstTab ) {
160+
navigate( firstTab, { replace: true } );
161+
}
162+
}, [ shouldOpenFirstTab, firstTab, navigate ] );
163+
158164
if ( visibleTabs.length === 0 ) {
159165
return null;
160166
}
@@ -201,6 +207,7 @@ export default connect(
201207
isWoASite: _isWoASite( state ),
202208
userCanPublishPosts: userCanPublish( state ),
203209
hasSecurityFeature: hasAnySecurityFeature( state ),
210+
hasProtectPage: _hasProtectPage( state ),
204211
hasPerformanceFeature: hasAnyPerformanceFeature( state ),
205212
hasModules: modules => hasAnyOfTheseModules( state, modules ),
206213
isModuleActive: module => isModuleActivated( state, module ),

‎projects/plugins/jetpack/_inc/client/components/settings-nav-tabs/test/protect-page.test.jsx‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { MemoryRouter, useLocation } from 'react-router';
2+
import { render, screen } from 'test/test-utils';
3+
import SettingsNavTabs from '../index';
4+
5+
jest.mock( 'components/data/query-site-plugins', () => ( {
6+
__esModule: true,
7+
default: () => null,
8+
} ) );
9+
10+
jest.mock( 'components/search', () => ( {
11+
__esModule: true,
12+
default: () => null,
13+
} ) );
14+
15+
const buildInitialState = protectPage => ( {
16+
jetpack: {
17+
initialState: {
18+
userData: {
19+
currentUser: { permissions: { manage_modules: true, publish_posts: true } },
20+
},
21+
},
22+
modules: {
23+
items: {
24+
protect: { activated: true },
25+
sso: { activated: true },
26+
stats: { activated: true },
27+
...( protectPage && { 'protect-dashboard': { activated: true } } ),
28+
},
29+
},
30+
pluginsData: { items: {} },
31+
search: { searchTerm: '' },
32+
},
33+
} );
34+
35+
const Pathname = () => <output>{ useLocation().pathname }</output>;
36+
37+
const renderAt = ( path, protectPage ) =>
38+
render(
39+
<MemoryRouter initialEntries={ [ path ] }>
40+
<SettingsNavTabs />
41+
<Pathname />
42+
</MemoryRouter>,
43+
{ initialState: buildInitialState( protectPage ) }
44+
);
45+
46+
describe( 'SettingsNavTabs with the Protect page', () => {
47+
it.each( [
48+
[ 'drops the Security tab when the Protect page is on', true, 0 ],
49+
[ 'keeps the Security tab when the Protect page is off', false, 1 ],
50+
] )( '%s', ( _name, protectPage, securityTabs ) => {
51+
renderAt( '/traffic', protectPage );
52+
53+
expect( screen.getByRole( 'tab', { name: 'Traffic' } ) ).toBeInTheDocument();
54+
expect( screen.queryAllByRole( 'tab', { name: 'Security' } ) ).toHaveLength( securityTabs );
55+
} );
56+
57+
it.each( [
58+
[ '/settings', true, '/traffic' ],
59+
[ '/settings', false, '/settings' ],
60+
[ '/settings?term=sso', true, '/settings' ],
61+
] )( 'leaves %s (Protect page %s) on %s', ( path, protectPage, expected ) => {
62+
renderAt( path, protectPage );
63+
64+
expect( screen.getByRole( 'status' ) ).toHaveTextContent( expected );
65+
} );
66+
} );

‎projects/plugins/jetpack/_inc/client/main.jsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import SettingsAdminPage from 'components/settings-admin-page';
1818
import SettingsNavTabs from 'components/settings-nav-tabs';
1919
import Tracker from 'components/tracker';
2020
import analytics from 'lib/analytics';
21+
import { openProtectSettings } from 'security/protect-page';
2122
import SearchableSettings from 'settings/index.jsx';
2223
import {
2324
getSiteConnectionStatus,
@@ -46,6 +47,7 @@ import {
4647
getPluginBaseUrl,
4748
userIsSubscriber,
4849
} from 'state/initial-state';
50+
import { hasProtectPage } from 'state/modules';
4951
import { getRewindStatus } from 'state/rewind';
5052
import { getSearchTerm } from 'state/search';
5153
import { areThereUnsavedSettings, clearUnsavedSettingsFlag } from 'state/settings';
@@ -63,7 +65,7 @@ const settingsRoutes = [
6365
'/privacy',
6466
];
6567

66-
class Main extends Component {
68+
export class Main extends Component {
6769
UNSAFE_componentWillMount() {
6870
this.props.setInitialState();
6971
restApi.setApiRoot( this.props.apiRoot );
@@ -126,6 +128,7 @@ class Main extends Component {
126128
nextProps.location.pathname !== this.props.location.pathname ||
127129
nextProps.searchTerm !== this.props.searchTerm ||
128130
nextProps.rewindStatus !== this.props.rewindStatus ||
131+
nextProps.hasProtectPage !== this.props.hasProtectPage ||
129132
nextProps.areThereUnsavedSettings !== this.props.areThereUnsavedSettings ||
130133
nextProps.isReconnectingSite !== this.props.isReconnectingSite
131134
);
@@ -236,8 +239,13 @@ class Main extends Component {
236239
case '/newsletter':
237240
window.location.href = `${ this.props.siteAdminUrl }admin.php?page=jetpack-newsletter`;
238241
break;
239-
case '/settings':
240242
case '/security':
243+
if ( this.props.hasProtectPage ) {
244+
openProtectSettings( this.props.siteAdminUrl );
245+
break;
246+
}
247+
// Falls through.
248+
case '/settings':
241249
case '/performance':
242250
case '/writing':
243251
case '/sharing':
@@ -408,6 +416,7 @@ export default connect(
408416
connectingUserFeatureLabel: getConnectingUserFeatureLabel( state ),
409417
connectingUserFrom: getConnectingUserFrom( state ),
410418
isSubscriber: userIsSubscriber( state ),
419+
hasProtectPage: hasProtectPage( state ),
411420
};
412421
},
413422
dispatch => ( {

‎projects/plugins/jetpack/_inc/client/security/index.jsx‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import QueryAkismetKeyCheck from 'components/data/query-akismet-key-check';
55
import QuerySite from 'components/data/query-site';
66
import { getVaultPressData } from 'state/at-a-glance';
77
import { isOfflineMode, isUnavailableInOfflineMode, hasConnectedOwner } from 'state/connection';
8-
import { getSiteId } from 'state/initial-state';
9-
import { getModule } from 'state/modules';
8+
import { getSiteId, isMultisite } from 'state/initial-state';
9+
import { getModule, hasProtectPage, isModuleActivated } from 'state/modules';
1010
import { isModuleFound } from 'state/search';
1111
import { getSettings } from 'state/settings';
1212
import { siteHasFeature } from 'state/site';
@@ -17,7 +17,9 @@ import Antispam from './antispam';
1717
import BackupsScan from './backups-scan';
1818
import { JetpackBackup } from './jetpack-backup';
1919
import { Monitor } from './monitor';
20+
import { MovedSettings } from './moved-settings';
2021
import { Protect } from './protect';
22+
import { BACKUP_MODULES, PROTECT_PAGE_MODULES } from './protect-page';
2123
import { SSO } from './sso';
2224
import Waf from './waf';
2325

@@ -72,6 +74,25 @@ export class Security extends Component {
7274
return null;
7375
}
7476

77+
if ( this.props.hasProtectPage ) {
78+
if ( ! isSearchTerm ) {
79+
return null;
80+
}
81+
const showProtect = PROTECT_PAGE_MODULES.some( this.props.isModuleFound ),
82+
showBackup = BACKUP_MODULES.some( this.props.isModuleFound ),
83+
showAntispam = this.isAkismetFound();
84+
85+
return showProtect || showBackup || showAntispam ? (
86+
<MovedSettings
87+
siteAdminUrl={ this.props.siteAdminUrl }
88+
showProtect={ showProtect }
89+
showBackup={ showBackup }
90+
hasBackupPage={ this.props.hasBackupPage }
91+
showAntispam={ showAntispam }
92+
/>
93+
) : null;
94+
}
95+
7596
const foundProtect = this.props.isModuleFound( 'protect' ),
7697
foundAccountProtection = this.props.isModuleFound( 'account-protection' ),
7798
foundSso = this.props.isModuleFound( 'sso' ),
@@ -145,5 +166,12 @@ export default connect( state => {
145166
vaultPressData: getVaultPressData( state ),
146167
hasConnectedOwner: hasConnectedOwner( state ),
147168
blogID: getSiteId( state ),
169+
hasProtectPage: hasProtectPage( state ),
170+
// Mirrors Jetpack::configure_backup_package(), or the standalone plugin's own page.
171+
hasBackupPage:
172+
( isModuleActivated( state, 'backup' ) &&
173+
hasConnectedOwner( state ) &&
174+
! isMultisite( state ) ) ||
175+
isPluginActive( state, 'jetpack-backup/jetpack-backup.php' ),
148176
};
149177
} )( Security );

‎projects/plugins/jetpack/_inc/client/security/moved-settings.jsx‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import { __, _x } from '@wordpress/i18n';
2+
import Card from 'components/card';
3+
import SettingsCard from 'components/settings-card';
4+
import SettingsGroup from 'components/settings-group';
5+
import { getProtectSettingsUrl } from './protect-page';
6+
7+
const PointerCard = ( { header, href, linkText, children } ) => (
8+
<SettingsCard header={ header } hideButton>
9+
<SettingsGroup>{ children }</SettingsGroup>
10+
<Card compact className="jp-settings-card__configure-link" href={ href }>
11+
{ linkText }
12+
</Card>
13+
</SettingsCard>
14+
);
15+
16+
/**
17+
* Search results that send Security tab settings to the pages that now hold them.
18+
*
19+
* @param {object} props - Component props.
20+
* @param {string} props.siteAdminUrl - The site's wp-admin URL.
21+
* @param {boolean} props.showProtect - Whether the search matched a setting on the Protect page.
22+
* @param {boolean} props.showBackup - Whether the search matched Backup.
23+
* @param {boolean} props.hasBackupPage - Whether the Backup page exists; otherwise point to My Jetpack.
24+
* @param {boolean} props.showAntispam - Whether the search matched Akismet, which is active.
25+
* @return {import('react').ReactNode} The cards.
26+
*/
27+
export function MovedSettings( {
28+
siteAdminUrl,
29+
showProtect,
30+
showBackup,
31+
hasBackupPage,
32+
showAntispam,
33+
} ) {
34+
return (
35+
<div>
36+
<h2 className="jp-settings__section-title">{ __( 'Security', 'jetpack' ) }</h2>
37+
{ showProtect && (
38+
<PointerCard
39+
header={ __( 'Security settings', 'jetpack' ) }
40+
href={ getProtectSettingsUrl( siteAdminUrl ) }
41+
linkText={ __( 'Open Protect settings', 'jetpack' ) }
42+
>
43+
{ __(
44+
'Firewall, login protection, account protection, downtime monitoring, and WordPress.com login are set up on the Protect page.',
45+
'jetpack'
46+
) }
47+
</PointerCard>
48+
) }
49+
{ showBackup &&
50+
( hasBackupPage ? (
51+
<PointerCard
52+
header={ _x( 'Jetpack Backup', 'Settings header', 'jetpack' ) }
53+
href={ `${ siteAdminUrl }admin.php?page=jetpack-backup` }
54+
linkText={ __( 'Open Backup', 'jetpack' ) }
55+
>
56+
{ __( 'Your backups and restores are on the Backup page.', 'jetpack' ) }
57+
</PointerCard>
58+
) : (
59+
<PointerCard
60+
header={ _x( 'Jetpack Backup', 'Settings header', 'jetpack' ) }
61+
href={ `${ siteAdminUrl }admin.php?page=my-jetpack#/add-backup` }
62+
linkText={ __( 'Set up Backup', 'jetpack' ) }
63+
>
64+
{ __(
65+
'Jetpack Backup saves your site as you edit it, so you can restore it in one click.',
66+
'jetpack'
67+
) }
68+
</PointerCard>
69+
) ) }
70+
{ showAntispam && (
71+
<PointerCard
72+
header={ _x( 'Anti-spam', 'Settings header', 'jetpack' ) }
73+
href={ `${ siteAdminUrl }admin.php?page=akismet-key-config` }
74+
linkText={ __( 'Open Akismet Anti-spam', 'jetpack' ) }
75+
>
76+
{ __(
77+
'Akismet’s settings, including your API key, are on the Akismet Anti-spam page.',
78+
'jetpack'
79+
) }
80+
</PointerCard>
81+
) }
82+
</div>
83+
);
84+
}

‎projects/plugins/jetpack/_inc/client/security/protect-page.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Modules whose Security tab settings live on the Protect page, when it is on.
2+
export const PROTECT_PAGE_MODULES = [ 'waf', 'protect', 'account-protection', 'monitor', 'sso' ];
3+
4+
// Modules whose Security tab card points to the Backup page instead, when the Protect page is on.
5+
export const BACKUP_MODULES = [ 'backup', 'vaultpress' ];
6+
7+
// Modules with no Settings card at all once the Protect page replaces the Security tab.
8+
export const HIDDEN_WITH_PROTECT_PAGE = [ 'protect-dashboard' ];
9+
10+
/**
11+
* Build the URL of the Protect page's Settings tab.
12+
*
13+
* @param {string} siteAdminUrl - The site's wp-admin URL, with a trailing slash.
14+
* @return {string} The URL.
15+
*/
16+
export function getProtectSettingsUrl( siteAdminUrl ) {
17+
return `${ siteAdminUrl }admin.php?page=jetpack-protect&p=${ encodeURIComponent(
18+
'/?tab=settings'
19+
) }`;
20+
}
21+
22+
/**
23+
* Leave Settings for the Protect page's Settings tab, without a history entry for the old route.
24+
*
25+
* @param {string} siteAdminUrl - The site's wp-admin URL, with a trailing slash.
26+
*/
27+
export function openProtectSettings( siteAdminUrl ) {
28+
window.location.replace( getProtectSettingsUrl( siteAdminUrl ) );
29+
}

0 commit comments

Comments
 (0)