RFC: Protect Scan v2 — design + Stage 1 plan (text-only)#48610
RFC: Protect Scan v2 — design + Stage 1 plan (text-only)#48610
Conversation
Empty commit on purpose; the design and Stage 1 implementation plan live in the PR description and pinned comments rather than as tracked files. Open this PR to read. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 🔴 Action required: Please include detailed testing steps, explaining how to test your change, like so: 🔴 Action required: We would recommend that you add a section to the PR description to specify whether this PR includes any changes to data or privacy, like so: Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
github-actions
Bot
added
the
[Status] Needs Author Reply
📋 Stage 1 Implementation Plan — Part 1 of 2 (Phases 0–3)
Protect Scan v2 — Stage 1 Implementation Plan
Goal: Introduce Protect's new Scan dataviews UI (mirroring Architecture: Approach C from the spec — reuse the existing Tech Stack: PHP 8 (Jetpack Scan_Page namespace), TypeScript / React 18, TanStack Query v5 (already a Protect dep), Spec: design.md. Read it first — this plan implements it; it does not re-derive design decisions. Branch: Pre-merge note for the implementer. All commits land with Phase 0 — REST changes in
|
| URL | Expected |
|---|---|
…?page=jetpack-protect#/scan |
Legacy UI (flag off). |
…?page=jetpack-protect&protect-scan-v2=1#/scan |
"Protect Scan v2 — placeholder" + console log. |
…?page=jetpack-protect&protect-scan-v2=1&jpprotect-mock=1#/scan |
Same placeholder; mock-mode is wired but no UI consumer yet (Phase 2 lights it up). |
If anything but legacy renders without the flag, stop and debug — don't continue to Phase 2.
Phase 2 — Read paths + status filter + persistKey
The placeholder <ThreatsScreen /> becomes the real <ThreatsDataViews /> instance, fed by a merged active+history query, with the upstream status toggle inside the table.
Task 2.1: Implement useScanThreatsQuery (merged query hook)
Files:
-
Create:
projects/plugins/protect/src/js/routes/scan/v2/data/use-scan-threats-query.ts -
Step 1: Write the failing test
Create projects/plugins/protect/src/js/routes/scan/v2/data/test/use-scan-threats-query.test.ts:
import { renderHook, waitFor } from '@testing-library/react';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import * as fetchers from '../fetchers';
import { useScanThreatsQuery } from '../use-scan-threats-query';
import type { ReactNode } from 'react';
jest.mock( '../fetchers' );
function wrapper( client: QueryClient ) {
return ( { children }: { children: ReactNode } ) => (
<QueryClientProvider client={ client }>{ children }</QueryClientProvider>
);
}
function freshClient(): QueryClient {
return new QueryClient( { defaultOptions: { queries: { retry: false } } } );
}
describe( 'useScanThreatsQuery', () => {
afterEach( () => jest.resetAllMocks() );
it( 'merges active and history threats keyed by id', async () => {
( fetchers.fetchSiteScan as jest.Mock ).mockResolvedValue( {
state: 'idle',
threats: [ { id: 'a', status: 'current', title: 'A' } ],
} );
( fetchers.fetchSiteScanHistory as jest.Mock ).mockResolvedValue( {
threats: [ { id: 'b', status: 'fixed', title: 'B' } ],
} );
const client = freshClient();
const { result } = renderHook( () => useScanThreatsQuery(), { wrapper: wrapper( client ) } );
await waitFor( () => expect( result.current.isLoading ).toBe( false ) );
expect( result.current.data.map( t => t.id ) ).toEqual( [ 'a', 'b' ] );
expect( result.current.activeError ).toBeNull();
expect( result.current.historyError ).toBeNull();
} );
it( 'surfaces history error without blocking active rows', async () => {
( fetchers.fetchSiteScan as jest.Mock ).mockResolvedValue( {
state: 'idle',
threats: [ { id: 'a', status: 'current', title: 'A' } ],
} );
( fetchers.fetchSiteScanHistory as jest.Mock ).mockRejectedValue( new Error( 'boom' ) );
const client = freshClient();
const { result } = renderHook( () => useScanThreatsQuery(), { wrapper: wrapper( client ) } );
await waitFor( () => expect( result.current.isLoading ).toBe( false ) );
expect( result.current.data.map( t => t.id ) ).toEqual( [ 'a' ] );
expect( result.current.historyError?.message ).toBe( 'boom' );
expect( result.current.activeError ).toBeNull();
} );
it( 'returns the active error when the active query fails', async () => {
( fetchers.fetchSiteScan as jest.Mock ).mockRejectedValue( new Error( 'down' ) );
( fetchers.fetchSiteScanHistory as jest.Mock ).mockResolvedValue( { threats: [] } );
const client = freshClient();
const { result } = renderHook( () => useScanThreatsQuery(), { wrapper: wrapper( client ) } );
await waitFor( () => expect( result.current.isLoading ).toBe( false ) );
expect( result.current.activeError?.message ).toBe( 'down' );
expect( result.current.data ).toEqual( [] );
} );
it( 'dedupes overlapping threats by id, preferring active', async () => {
( fetchers.fetchSiteScan as jest.Mock ).mockResolvedValue( {
state: 'idle',
threats: [ { id: 'x', status: 'current', title: 'Active X' } ],
} );
( fetchers.fetchSiteScanHistory as jest.Mock ).mockResolvedValue( {
threats: [ { id: 'x', status: 'fixed', title: 'History X' } ],
} );
const client = freshClient();
const { result } = renderHook( () => useScanThreatsQuery(), { wrapper: wrapper( client ) } );
await waitFor( () => expect( result.current.isLoading ).toBe( false ) );
expect( result.current.data ).toHaveLength( 1 );
expect( result.current.data[ 0 ].title ).toBe( 'Active X' );
} );
} );- Step 2: Run, expect FAIL (hook not defined)
cd /Users/ilona/jetpack && pnpm jetpack test-js plugins/protect -- --testPathPattern=use-scan-threats-query 2>&1 | tail -30Expected: FAIL Cannot find module '../use-scan-threats-query'.
- Step 3: Implement the hook
Create projects/plugins/protect/src/js/routes/scan/v2/data/use-scan-threats-query.ts:
import { useQuery } from '@tanstack/react-query';
import { useMemo, useCallback } from '@wordpress/element';
import { siteScanQuery, siteScanHistoryQuery } from './query-options';
import type { Threat } from './types';
export type UseScanThreatsResult = {
data: Threat[];
isLoading: boolean;
isFetching: boolean;
activeError: Error | null;
historyError: Error | null;
refetch: () => void;
};
/**
* Merged query backing the Protect Scan v2 threat list. Calls both
* the active and history endpoints in parallel and returns a single
* deduped Threat array. Per spec §5:
* - Active fails: data is [], activeError is set; consumer renders an
* error block.
* - History fails: data has only active rows, historyError is set;
* consumer surfaces a snackbar with retry.
* - Both fail: data is [], both errors set.
*/
export function useScanThreatsQuery(): UseScanThreatsResult {
const active = useQuery( siteScanQuery() );
const history = useQuery( siteScanHistoryQuery() );
const data = useMemo< Threat[] >( () => {
const seen = new Map< string, Threat >();
for ( const t of active.data?.threats ?? [] ) {
seen.set( String( t.id ), t );
}
for ( const t of history.data?.threats ?? [] ) {
if ( ! seen.has( String( t.id ) ) ) {
seen.set( String( t.id ), t );
}
}
return Array.from( seen.values() );
}, [ active.data, history.data ] );
const refetch = useCallback( () => {
active.refetch();
history.refetch();
}, [ active, history ] );
return {
data: active.error ? [] : data,
isLoading: active.isLoading || history.isLoading,
isFetching: active.isFetching || history.isFetching,
activeError: ( active.error as Error | null ) ?? null,
historyError: ( history.error as Error | null ) ?? null,
refetch,
};
}- Step 4: Run, expect PASS
cd /Users/ilona/jetpack && pnpm jetpack test-js plugins/protect -- --testPathPattern=use-scan-threats-query 2>&1 | tail -20Expected: 4 tests pass.
- Step 5: Commit
git add projects/plugins/protect/src/js/routes/scan/v2/data/use-scan-threats-query.ts \
projects/plugins/protect/src/js/routes/scan/v2/data/test/use-scan-threats-query.test.ts
git commit -m "Protect: scan v2 useScanThreatsQuery + tests"Task 2.2: Real <ThreatsScreen /> rendering <ThreatsDataViews />
Files:
-
Modify:
projects/plugins/protect/src/js/routes/scan/v2/screens/threats.tsx -
Step 1: Replace the placeholder with the real component
import { ThreatsDataViews } from '@automattic/jetpack-scan';
import { useScanThreatsQuery } from '../data/use-scan-threats-query';
import { useTrackEvent } from '../data/use-track-event';
import type { Threat } from '../data/types';
const PERSIST_KEY = 'jetpack-protect:scan:view';
const TRACK_PREFIX = 'jetpack_protect_scan_';
const isFixable = ( t: Threat ) => Boolean( t.fixable );
const isCurrent = ( t: Threat ) => t.status === 'current';
const isIgnored = ( t: Threat ) => t.status === 'ignored';
export default function ThreatsScreen() {
const { data, isLoading, activeError } = useScanThreatsQuery();
const trackEvent = useTrackEvent();
if ( isLoading ) {
return null; // Phase 6 replaces with a skeleton.
}
if ( activeError ) {
return (
<div data-testid="protect-scan-v2-error">
{ 'Couldn’t load your threats. Please try again.' }
</div>
);
}
return (
<ThreatsDataViews
data={ data }
showStatusFilter={ true }
filters={ [ { field: 'status', operator: 'isAny', value: [ 'current' ] } ] }
persistKey={ PERSIST_KEY }
isThreatEligibleForFix={ isFixable }
isThreatEligibleForIgnore={ isCurrent }
isThreatEligibleForUnignore={ isIgnored }
onTrackEvent={ ( name, properties ) =>
trackEvent( `${ TRACK_PREFIX }${ name }`, properties )
}
/>
);
}(onFixThreats / onIgnoreThreats / onUnignoreThreats and the Render*Modal props arrive in Phase 3. Empty slot lands in Phase 6. ScanStatus takeover is Phase 5.)
- Step 2: Typecheck
cd /Users/ilona/jetpack && pnpm jetpack typecheck plugins/protect 2>&1 | head -10Expected: clean. If Threat.fixable is an object (not boolean) per upstream, isFixable becomes Boolean( t.fixable ) (already covered) but verify shape.
- Step 3: Manual sanity (after build + rsync)
Visit …?page=jetpack-protect&protect-scan-v2=1&jpprotect-mock=1#/scan. Expected: ThreatsDataViews renders with the in-table Active/History toggle (defaulting to Active), the two active mock threats are listed, and switching to History shows the two historical mock threats.
- Step 4: Commit
git add projects/plugins/protect/src/js/routes/scan/v2/screens/threats.tsx
git commit -m "Protect: scan v2 ThreatsScreen renders ThreatsDataViews"Task 2.3: Mount <ScanV2Route /> shell — QueryClientProvider scope check
Protect's app shell already sits inside a QueryClientProvider (see projects/plugins/protect/src/js/index.tsx:21–27), so useScanThreatsQuery runs against the existing client. No additional provider needed for the v2 route.
- Step 1: Confirm the existing client's
staleTime: Infinityis fine for our queries
Read projects/plugins/protect/src/js/index.tsx:21–27. The default is staleTime: Infinity, which means tabs won't refetch on navigation. That matches packages/scan's expectation; mutations explicitly invalidate the prefix on success (Phase 3).
- Step 2: No code change. Document in
routes/scan/v2/index.tsxmodule comment
Edit projects/plugins/protect/src/js/routes/scan/v2/index.tsx to add at the top of the module comment block:
* The route relies on the QueryClientProvider mounted higher up in
* `src/js/index.tsx`. We do not declare a sub-client here.
- Step 3: Commit
git add projects/plugins/protect/src/js/routes/scan/v2/index.tsx
git commit -m "Protect: document scan v2 query-client scope"Task 2.4: Snackbar list mounted inside the route
Files:
- Create:
projects/plugins/protect/src/js/routes/scan/v2/notices-list.tsx - Modify:
projects/plugins/protect/src/js/routes/scan/v2/index.tsx
Mirrors packages/scan/src/js/notices-list.tsx.
- Step 1: Write
notices-list.tsx
import { SnackbarList } from '@wordpress/components';
import { useDispatch, useSelect } from '@wordpress/data';
import { store as noticesStore } from '@wordpress/notices';
const MAX_VISIBLE = 3;
export default function NoticesList() {
const notices = useSelect(
select => select( noticesStore ).getNotices().filter( n => n.type === 'snackbar' ),
[]
);
const { removeNotice } = useDispatch( noticesStore );
if ( notices.length === 0 ) {
return null;
}
return (
<SnackbarList
notices={ notices.slice( -MAX_VISIBLE ) }
onRemove={ removeNotice }
/>
);
}- Step 2: Mount inside
routes/scan/v2/index.tsx
Replace the body of ScanV2Route() to:
return (
<>
<ThreatsScreen />
<NoticesList />
</>
);Add the import: import NoticesList from './notices-list';
- Step 3: Typecheck and commit
cd /Users/ilona/jetpack && pnpm jetpack typecheck plugins/protect 2>&1 | head -10
git add projects/plugins/protect/src/js/routes/scan/v2/notices-list.tsx \
projects/plugins/protect/src/js/routes/scan/v2/index.tsx
git commit -m "Protect: scan v2 SnackbarList mount"Task 2.5: Mock-mode dev banner
Files:
-
Create:
projects/plugins/protect/src/js/routes/scan/v2/mock-banner.tsx -
Modify:
projects/plugins/protect/src/js/routes/scan/v2/index.tsx -
Step 1: Write
mock-banner.tsx(mirrorpackages/scan/src/js/mock-banner.tsxshape, copy/adapt)
import { Notice } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { isProtectMockMode } from './data/mock';
export default function MockBanner() {
if ( ! isProtectMockMode() ) {
return null;
}
return (
<Notice status="warning" isDismissible={ false }>
{ __( 'Dev mode (?jpprotect-mock=1) — fixtures only, no server requests.', 'jetpack-protect' ) }
</Notice>
);
}- Step 2: Mount in
index.tsx
return (
<>
<MockBanner />
<ThreatsScreen />
<NoticesList />
</>
);Add import: import MockBanner from './mock-banner';
- Step 3: Manual sanity
After build + rsync: with ?jpprotect-mock=1, banner appears; without it, no banner.
- Step 4: Commit
git add projects/plugins/protect/src/js/routes/scan/v2/mock-banner.tsx \
projects/plugins/protect/src/js/routes/scan/v2/index.tsx
git commit -m "Protect: scan v2 mock-mode banner"Phase 3 — Row actions + four Render*Modal modals
Single-threat ignore/unignore/fix flows + view-details modal, all wired through the Render*Modal props.
Task 3.1: Mutations — useThreatMutations.ts
Files:
-
Create:
projects/plugins/protect/src/js/routes/scan/v2/data/use-threat-mutations.ts -
Test:
projects/plugins/protect/src/js/routes/scan/v2/data/test/use-threat-mutations.test.ts -
Step 1: Write the failing test (sketch — verify coverage of cache invalidation)
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { renderHook, act, waitFor } from '@testing-library/react';
import * as fetchers from '../fetchers';
import {
useFixThreatsMutation,
useIgnoreThreatMutation,
useUnignoreThreatMutation,
useEnqueueScanMutation,
} from '../use-threat-mutations';
import { SCAN_QUERY_PREFIX } from '../query-options';
jest.mock( '../fetchers' );
function wrap( client: QueryClient ) {
return ( { children }: { children: React.ReactNode } ) => (
<QueryClientProvider client={ client }>{ children }</QueryClientProvider>
);
}
describe( 'threat mutations', () => {
it( 'fixThreats invalidates SCAN_QUERY_PREFIX on success', async () => {
( fetchers.fixThreats as jest.Mock ).mockResolvedValue( { ok: true, threat_ids: [ 1 ] } );
const client = new QueryClient();
const spy = jest.spyOn( client, 'invalidateQueries' );
const { result } = renderHook( () => useFixThreatsMutation(), { wrapper: wrap( client ) } );
await act( async () => {
await result.current.mutateAsync( [ 1 ] );
} );
expect( spy ).toHaveBeenCalledWith( { queryKey: SCAN_QUERY_PREFIX } );
} );
// Repeat shape for ignore / unignore / enqueue.
} );(Repeat the same body for useIgnoreThreatMutation, useUnignoreThreatMutation, useEnqueueScanMutation — each calls its respective fetcher and invalidates the same prefix.)
- Step 2: Run, expect FAIL
cd /Users/ilona/jetpack && pnpm jetpack test-js plugins/protect -- --testPathPattern=use-threat-mutations 2>&1 | tail -20Expected: FAIL — module not found.
- Step 3: Implement the hooks
import { useMutation, useQueryClient } from '@tanstack/react-query';
import {
fixThreats,
ignoreThreat,
unignoreThreat,
enqueueScan,
} from './fetchers';
import { SCAN_QUERY_PREFIX } from './query-options';
export function useFixThreatsMutation() {
const queryClient = useQueryClient();
return useMutation( {
mutationFn: ( threatIds: Array< string | number > ) => fixThreats( threatIds ),
onSuccess: () => queryClient.invalidateQueries( { queryKey: SCAN_QUERY_PREFIX } ),
} );
}
export function useIgnoreThreatMutation() {
const queryClient = useQueryClient();
return useMutation( {
mutationFn: ( threatId: string | number ) => ignoreThreat( threatId ),
onSuccess: () => queryClient.invalidateQueries( { queryKey: SCAN_QUERY_PREFIX } ),
} );
}
export function useUnignoreThreatMutation() {
const queryClient = useQueryClient();
return useMutation( {
mutationFn: ( threatId: string | number ) => unignoreThreat( threatId ),
onSuccess: () => queryClient.invalidateQueries( { queryKey: SCAN_QUERY_PREFIX } ),
} );
}
export function useEnqueueScanMutation() {
const queryClient = useQueryClient();
return useMutation( {
mutationFn: () => enqueueScan(),
onSuccess: () => queryClient.invalidateQueries( { queryKey: SCAN_QUERY_PREFIX } ),
} );
}- Step 4: Run tests, expect PASS
cd /Users/ilona/jetpack && pnpm jetpack test-js plugins/protect -- --testPathPattern=use-threat-mutations 2>&1 | tail -10- Step 5: Commit
git add projects/plugins/protect/src/js/routes/scan/v2/data/use-threat-mutations.ts \
projects/plugins/protect/src/js/routes/scan/v2/data/test/use-threat-mutations.test.ts
git commit -m "Protect: scan v2 threat mutations"Task 3.2: Fix-status polling — useFixThreatsStatusQuery
Files:
- Create:
projects/plugins/protect/src/js/routes/scan/v2/data/use-fix-threats-status.ts - Test:
projects/plugins/protect/src/js/routes/scan/v2/data/test/use-fix-threats-status.test.ts
Mirror packages/scan/src/js/data/use-fix-threats-status.ts exactly. The polling logic + isFixComplete predicate should be identical (re-export from upstream if exposed; otherwise copy with attribution comment).
- Step 1: Read the upstream and copy/adapt
cat projects/packages/scan/src/js/data/use-fix-threats-status.ts-
Step 2: Implement the hook + write 5 unit tests covering the
isFixCompletepredicate (undefined / empty / partial / terminal / unknown — same shape aspackages/scan/src/js/data/test/use-fix-threats-status.test.ts) -
Step 3: Run, expect PASS, then commit.
cd /Users/ilona/jetpack && pnpm jetpack test-js plugins/protect -- --testPathPattern=use-fix-threats-status
git add projects/plugins/protect/src/js/routes/scan/v2/data/use-fix-threats-status.ts \
projects/plugins/protect/src/js/routes/scan/v2/data/test/use-fix-threats-status.test.ts
git commit -m "Protect: scan v2 fix-status polling"Task 3.3: <FixThreatModal /> (single-threat fix)
Files:
-
Create:
projects/plugins/protect/src/js/routes/scan/v2/screens/fix-threat-modal.tsx -
Step 1: Read the upstream for shape
cat projects/packages/scan/src/js/screens/overview/fix-threat-modal.tsx- Step 2: Implement Protect's version (same UX, Protect copy + tracks namespace)
import { Button, Modal, Notice } from '@wordpress/components';
import { useDispatch } from '@wordpress/data';
import { useState } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { store as noticesStore } from '@wordpress/notices';
import type { RenderModalProps } from '@wordpress/dataviews';
import type { Threat } from '../data/types';
import { useFixThreatsMutation } from '../data/use-threat-mutations';
import { useFixThreatsStatusQuery } from '../data/use-fix-threats-status';
import { useTrackEvent } from '../data/use-track-event';
export default function FixThreatModal( {
items,
closeModal,
}: RenderModalProps< Threat > ) {
const threat = items[ 0 ];
const trackEvent = useTrackEvent();
const { createSuccessNotice, createErrorNotice } = useDispatch( noticesStore );
const fixMutation = useFixThreatsMutation();
const [ submitted, setSubmitted ] = useState( false );
const fixStatus = useFixThreatsStatusQuery(
submitted ? [ threat.id ] : []
);
const isComplete = fixStatus.data?.every(
s => s.status === 'fixed' || s.status === 'not_fixed'
);
if ( submitted && isComplete ) {
const ok = fixStatus.data?.every( s => s.status === 'fixed' );
if ( ok ) {
createSuccessNotice( __( 'Threat fixed.', 'jetpack-protect' ), { type: 'snackbar' } );
} else {
createErrorNotice( __( 'We couldn’t fix that threat.', 'jetpack-protect' ), {
type: 'snackbar',
} );
}
closeModal();
}
const onConfirm = async () => {
trackEvent( 'jetpack_protect_scan_fix_threat_modal_click', { threat_id: threat.id } );
try {
await fixMutation.mutateAsync( [ threat.id ] );
setSubmitted( true );
} catch {
createErrorNotice( __( 'Could not start the fix.', 'jetpack-protect' ), {
type: 'snackbar',
} );
}
};
return (
<Modal title={ __( 'Fix threat', 'jetpack-protect' ) } onRequestClose={ closeModal }>
<p>{ threat.title }</p>
{ submitted && ! isComplete && (
<Notice status="info" isDismissible={ false }>
{ __( 'Fixing…', 'jetpack-protect' ) }
</Notice>
) }
<div className="protect-modal-actions">
<Button variant="tertiary" onClick={ closeModal }>
{ __( 'Cancel', 'jetpack-protect' ) }
</Button>
<Button
variant="primary"
onClick={ onConfirm }
isBusy={ fixMutation.isPending || ( submitted && ! isComplete ) }
>
{ __( 'Fix threat', 'jetpack-protect' ) }
</Button>
</div>
</Modal>
);
}- Step 3: Typecheck and commit
cd /Users/ilona/jetpack && pnpm jetpack typecheck plugins/protect 2>&1 | head -10
git add projects/plugins/protect/src/js/routes/scan/v2/screens/fix-threat-modal.tsx
git commit -m "Protect: scan v2 FixThreatModal"Task 3.4: <IgnoreThreatModal />
Files:
- Create:
projects/plugins/protect/src/js/routes/scan/v2/screens/ignore-threat-modal.tsx
Same shape as Task 3.3 but for ignore. Mirror upstream packages/scan/src/js/screens/overview/ignore-threat-modal.tsx. Use useIgnoreThreatMutation, fire jetpack_protect_scan_ignore_threat_modal_click, success snackbar __( 'Threat ignored.', 'jetpack-protect' ). Modal title __( 'Ignore threat', 'jetpack-protect' ). Single-step mutation — no polling.
Read upstream, write component, typecheck, commit.
- Step 1: Read upstream:
cat projects/packages/scan/src/js/screens/overview/ignore-threat-modal.tsx - Step 2: Write Protect's version with
jetpack-protecttext-domain + Protect tracks namespace - Step 3: Typecheck, commit
git add projects/plugins/protect/src/js/routes/scan/v2/screens/ignore-threat-modal.tsx
git commit -m "Protect: scan v2 IgnoreThreatModal"Task 3.5: <UnignoreThreatModal />
Same as Task 3.4 with useUnignoreThreatMutation, modal title __( 'Unignore threat', 'jetpack-protect' ), snackbar __( 'Threat unignored.', 'jetpack-protect' ), tracks jetpack_protect_scan_unignore_threat_modal_click.
- Step 1: Read upstream
unignore-threat-modal.tsx - Step 2: Write + typecheck
- Step 3: Commit
Protect: scan v2 UnignoreThreatModal
Task 3.6: <ViewDetailsModal />
Files:
- Create:
projects/plugins/protect/src/js/routes/scan/v2/screens/view-details-modal.tsx
Read-only modal. Title: __( 'Threat details', 'jetpack-protect' ). Renders threat metadata: severity badge (ThreatSeverityBadge from @automattic/jetpack-scan), title, description, signature, filename + diff (if present), first detected, fixed-on (if status === 'fixed').
- Step 1: Read upstream
view-details-modal.tsx - Step 2: Write Protect's version. Modal
size="large". No mutation, just display. - Step 3: Typecheck, commit
Protect: scan v2 ViewDetailsModal
Task 3.7: Wire all four Render*Modal props into <ThreatsScreen />
Files:
-
Modify:
projects/plugins/protect/src/js/routes/scan/v2/screens/threats.tsx -
Step 1: Update
<ThreatsDataViews />props
import FixThreatModal from './fix-threat-modal';
import IgnoreThreatModal from './ignore-threat-modal';
import UnignoreThreatModal from './unignore-threat-modal';
import ViewDetailsModal from './view-details-modal';
// inside the return:
<ThreatsDataViews
data={ data }
showStatusFilter={ true }
filters={ [ { field: 'status', operator: 'isAny', value: [ 'current' ] } ] }
persistKey={ PERSIST_KEY }
isThreatEligibleForFix={ isFixable }
isThreatEligibleForIgnore={ isCurrent }
isThreatEligibleForUnignore={ isIgnored }
onTrackEvent={ ( name, properties ) =>
trackEvent( `${ TRACK_PREFIX }${ name }`, properties )
}
RenderFixModal={ FixThreatModal }
RenderIgnoreModal={ IgnoreThreatModal }
RenderUnignoreModal={ UnignoreThreatModal }
RenderViewModal={ ViewDetailsModal }
/>- Step 2: Manual sanity
After build + rsync, with mock mode + flag:
- Click "Fix" on the active fixable threat → modal opens, confirm → snackbar fires → threat moves to History as fixed.
- Click "Ignore" on the second active threat → modal opens, confirm → snackbar fires → threat moves to History as ignored.
- Toggle to History, click "Unignore" → reverse flow.
- Click "View details" on any row → read-only modal renders with all the threat metadata.
(In mock mode, the table won't actually update because mock data is static. The mutations resolve immediately and snackbars fire, but the data stays put. That's expected — full round-trip happens in live mode.)
- Step 3: Commit
git add projects/plugins/protect/src/js/routes/scan/v2/screens/threats.tsx
git commit -m "Protect: scan v2 wire row-action modals"
📋 Stage 1 Implementation Plan — Part 2 of 2 (Phases 4–8 + assembly)
Phase 4 — Bulk fix CTA +
|
1 participant
Protect: adopt new Scan dataviews UI
Date: 2026-05-07
Status: Design — pending parallel review
Owner: ilona
Related: #48456 (Scan port tracking issue), #48458 (merged Scan port), #48160 (UI Modernization umbrella)
Reference: Figma
IA - UI Unificationnode6171-87963(designer-approved Protect-Scan-tab mockup)Goal (from user perspective)
Non-goals
jetpack-protect/v1/*REST surface (issue Port Scan overview to Jetpack wp-admin as a native page #48456 decision Create a readme.md for the Jetpack GitHub Repository. #5 — follow-up PR)Architecture (Approach C: reuse REST, recreate JS in Protect)
Approaches considered
packages/scancontroller (decoupled from filter)packages/scancontroller (decoupled from filter)packages/scan/src/js/data/@automattic/jetpack-scan-pagepackages/scan/src/js/data/@automattic/jetpack-scan-page@automattic/jetpack-scan(already a dep)@automattic/jetpack-scan@automattic/jetpack-scan-pagepackagepackages/scan-pageto library APIpackages/scan(REST ungate)Why C
ProtectApp/AdminPage;packages/scanmounts at a top-level wp-admin submenu.HeaderActionsProvider,AdminPagechrome, mock-mode flag, and route shape all diverge enough that "share the screens" leaks abstractions. Better that Protect own its glue.packages/scan's public API or release cadence. We touchpackages/scanonce, narrowly, to ungate REST registration.Design
1. PHP — narrow change in
packages/scanVerified
class-jetpack-scan.php:75–98: the currentinitialize()early-returns on! apply_filters( MODERNIZATION_FILTER, false )and so blocksload_wp_build(),fix_boot_import_map_ordering(),bridge_wp_build_enqueue(),admin_menu,rest_api_init, thejetpack_package_versionsfilter, and the terminaldo_action( 'jetpack_scan_page_initialized' )together. The split has to be explicit — not just "remove the early return":do_action( 'jetpack_scan_page_initialized' )continues to fire whether the admin UI is gated or not — listeners that care about REST availability stay correct; listeners that care about the admin UI can re-check the filter themselves.Add one PHPUnit test in
projects/packages/scan/tests/php/Jetpack_Scan_Bridges_Test.phpasserting routes register when the filter isfalse.Add a changelog entry to
projects/packages/scan/changelog/(addpatch type).Verified risk: route exposure when filter is off.
Rest_Controller::permissions_check()requirescurrent_user_can( 'manage_options' ). With that gate, exposure to non-admin users is a 403, not data leakage. Confirmed in PHP review.2. PHP — Protect plugin
No new REST routes. Protect's
class-rest-controller.phpis untouched by this PR.The legacy
jetpack-protect/v1/*endpoints continue to register so other surfaces (Setup wizard) keep working. Their deprecation is a follow-up.User-auth check at mutation time. Phase 3 mutations use
Client::wpcom_json_api_request_as_user. PHP review flagged a risk:as_usermay silently fall back to blog auth if no current user has a connection token, which would let a privileged-but-not-user-connected admin trigger writes that mis-attribute. Mitigation in this PR: hardenRest_Controller::permissions_check_user()(the callback used by mutation routes —threat/{id}/ignore,threat/{id}/unignore,threats/fix) to also require( new Connection_Manager() )->is_user_connected(). Read paths keep the existing admin-only check. Adds one test toJetpack_Scan_Bridges_Test.Initial state: Protect's existing
window.jetpackProtectInitialStatecarries the REST nonce (apiNonceatclass-jetpack-protect.php:223); the new client uses that. No new initial-state blob is needed. Documented in the new entry-file's module comment so the dependency is explicit.3. JS — file tree
4. Routing inside Protect's app
/scan→ renders<ThreatsScreen />fromroutes/scan/v2/index.tsxinstead of the legacy<ScanRoute />./scan/history→ removed; HashRouter redirects to/scanonce on first hit (handled in the route definition, not server-side)./firewall,/settings,/setup) are untouched.5. Threat list — single
ThreatsDataViewsinstance, status filter insideData flow.
useScanThreatsQuerycalls TanStack Query forsiteScanQuery()andsiteScanHistoryQuery()in parallel. Returns:Partial-failure UX.
<ErrorState />(full-block) — active threats are the user's primary intent on this tab."Couldn't load scan history"with a "Retry".<ErrorState />with "Retry" wired to mergedrefetch().Mutations invalidate the merged query prefix on success so both endpoints refetch.
Scan progress takeover. While
scanData.stateisenqueuedorrunning,<ScanStatus />replaces the entireThreatsDataViewsblock.6. Modals
Four modals, wired through the
Render*Modalprops:v2/screens/)fix-threat-modal.tsxprotect/src/js/components/fix-threat-modal/RenderFixModalbulk-fix-modal.tsxprotect'sFIX_ALL_THREATSmodal inthreats-list/index.jsx<BulkFixModal />mountignore-threat-modal.tsxprotect/src/js/components/ignore-threat-modal/RenderIgnoreModalunignore-threat-modal.tsxprotect/src/js/components/unignore-threat-modal/RenderUnignoreModalview-details-modal.tsxRenderViewModal7. Header CTA — inline strip, no
HeaderActionsProviderProtect's shell (
protect-app/index.jsx:90–117) usesJetpackAdminPage+Tabs.Root+<Outlet />— it has no header-action slot and refactoringJetpackAdminPageto add one is wider blast radius than this PR should take.Resolution: the Scan tab renders its CTAs inline at the top of the tab content, above
ThreatsDataViews. No provider, no context.fixableCount=data.filter( t => t.status === 'current' && t.fixable ).length. Independent of which status the toggle is on (we still only auto-fix active fixable rows).8. Free-tier —
<EmptyState />dispatch (the C3 commitment)Reads Protect's existing
usePlan()hook. The empty slot usesContextualUpgradeTriggerfrom@automattic/jetpack-components— already imported infree-list.jsx,scan-footer.jsx, andfirewall/index.jsx. Card-shaped (not the wideSeventyFiveLayouthero), fits inside the DataViews empty slot.<VStack>heading +<ContextualUpgradeTrigger>(copy reused fromscan-footer.jsx:48–66)On the C3 product decision. Today
FreeListshows vulnerability records to free users. C3 explicitly stops that surface in favor of an upsell. Documented as a deliberate product choice (per the design discussion that produced this RFC). If reversal is wanted, we add a Free-only vulnerability surface in a follow-up PR — independent of this work.9. Tracks events
Single namespace:
jetpack_protect_scan_*. No dual-firing ofjetpack_scan_*.jetpack_protect_scan_now{}<ScanNowButton />clickjetpack_protect_scan_fix_threats_cta_click{ threat_count }jetpack_protect_scan_bulk_fix_threats_modal_open{ threat_count }jetpack_protect_scan_bulk_fix_threats_modal_click{ threat_count }jetpack_protect_scan_bulk_fix_threats_modal_success{ threat_count, fixed_count, failed_count }jetpack_protect_scan_bulk_fix_threats_modal_failed{ threat_count }jetpack_protect_scan_search{ has_query }'search'jetpack_protect_scan_layout_changed{ layout }'layout_changed'jetpack_protect_scan_page_change{ page }'page_change'jetpack_protect_scan_filter_change{}'filter_change'jetpack_protect_scan_view_change{}'view_change'(catch-all)Transport via
@automattic/jetpack-analytics.10. Mock mode
?jpprotect-mock=1(separate from?jps-mock=1). Each fetcher short-circuits viaisProtectMockMode(). Yellow "Dev mode" banner inside the Scan tab when active.11. Notices
<SnackbarList />mounted inroutes/scan/v2/notices-list.tsx, subscribed tocore/noticesstore, filtered totype === 'snackbar', sliced to last 3.File-level impact summary
Stage 1 PR
New files (~20):
projects/plugins/protect/src/js/routes/scan/v2/tree as enumerated in §3.Modified:
projects/packages/scan/src/class-jetpack-scan.php— explicit clean split of admin-UI gating vs REST registration (§1).projects/packages/scan/src/class-rest-controller.php—permissions_check_user()requiresis_user_connected()for mutation routes (§2).projects/packages/scan/tests/php/Jetpack_Scan_Bridges_Test.php— two new tests.projects/packages/scan/changelog/— new entry.projects/plugins/protect/src/js/index.tsx— wire/scanto either<ScanRoute />or<ScanV2Route />based on flag.projects/plugins/protect/src/class-jetpack-protect.php— readJETPACK_PROTECT_SCAN_V2constant, hydrate into initial state.projects/plugins/protect/changelog/— new entry.Verified (not modified):
projects/plugins/protect/package.json—@tanstack/react-queryalready a dep.Stage 2 PR
Modified:
index.tsx(default-on +/scan/historyredirect),class-jetpack-protect.php(drop constant),scan-navigation/index.jsx(drop history link), changelog.Deleted: legacy threat-list components, modals, hooks,
scan-button/,scan-admin-section-hero.tsx,scan-footer.jsx,routes/scan/index.jsx,routes/scan/history/, thedata/scan/use-*-mutation.tsfiles.useProtectData,usePlan,ContextualUpgradeTriggerconsumers in Setup/Firewall are kept.Test plan
Mock mode
pnpm jetpack build plugins/jetpack --deps.…?page=jetpack-protect&jpprotect-mock=1#/scan(with the v2 flag also on). Yellow banner, ThreatsDataViews with Active/History toggle, fixture rows visible.persistKey.…#/scan/history→ redirects to…#/scan(Stage 2 only).Live mode
<EmptyState />upsell card.<ScanStatus />takeover → back to threats.jetpack_protect_scan_*Tracks events fire.Automated
packages/scan): routes register with filter off; mutation routes 403 without user connection.useScanThreatsQuerypartial-failure cases;EmptyStateplan dispatch;useThreatActionssnackbars;isFixCompletepolling edges.Risks (post-review)
permissions_check()is admin-only; non-admin exposure is 403, not leak.is_user_connected().ContextualUpgradeTrigger(card-shaped)./scan/historyURL) — LOW. All in-repo refs are inside Protect's own JS; HashRouter URLs aren't reachable as public docs.Known limitations (deferred polish)
ThreatsDataViewsdoesn't exposeview/onChangeViewfor external observation, so<EmptyState />can't tell whether the user has the toggle on Active or History. Phase 2 follow-up.manage_optionscheck.Decisions (locked 2026-05-07)
jetpack_protect_scan_*. Legacy events drop with the legacy files in Stage 2. Data-team P2 / Slack note when Stage 1 lands.?protect-scan-v2=1URL flag andJETPACK_PROTECT_SCAN_V2PHP constant default-off. Stage 2 flips the default + cleanup.?jpprotect-mock=1.v2/folder name —routes/scan/v2/(default in spec). Bikeshed; can revisit during code review.Adversarial review log (2026-05-07)
Three parallel review teams attacked this design. Findings + dispositions:
Resolved in spec body
ProtectAppis_user_connected()check on mutation routes.emptyslotContextualUpgradeTrigger(card-shape).Rebutted (misread of upstream)
ThreatsStatusToggleGroupControlrequires externally-suppliedview/onChangeViewThreatsDataViewsmanagesviewinternally and passes its ownview/onChangeViewto the toggle whenshowStatusFilter={true}. Verified atindex.tsx:670–687.Accepted as known limitations / deferred
packages/scan; upstream issue.@tanstack/react-queryis already a Protect dep; new code is ~20 small files./scan/historyURL removal customer impactv2/folder namingAcceptance criteria
ThreatsDataViewswith in-table Active/History toggle (default Active).jetpack_protect_scan_*.