Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Simplify
  • Loading branch information
dkmyta authored and nateweller committed Dec 5, 2024
commit 600a05e64a1e3bff40bc13a189f9a31ae11558ae
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,6 @@ import {
wordpress as coreIcon,
} from '@wordpress/icons';

export const THREAT_STATUSES: { value: string; label: string; variant?: 'success' | 'warning' }[] =
[
{ value: 'current', label: __( 'Active', 'jetpack' ), variant: 'warning' },
{ value: 'fixed', label: __( 'Fixed', 'jetpack' ), variant: 'success' },
{ value: 'ignored', label: __( 'Ignored', 'jetpack' ) },
];

export const THREAT_TYPES = [
{ value: 'plugins', label: __( 'Plugins', 'jetpack' ) },
{ value: 'themes', label: __( 'Themes', 'jetpack' ) },
Expand All @@ -29,22 +22,9 @@ export const THREAT_ICONS = {
default: shieldIcon,
};

export const THREAT_FIELD_THREAT = 'threat';
export const THREAT_FIELD_TITLE = 'title';
export const THREAT_FIELD_DESCRIPTION = 'description';
export const THREAT_FIELD_ICON = 'icon';
export const THREAT_FIELD_STATUS = 'status';
export const THREAT_FIELD_TYPE = 'type';
export const THREAT_FIELD_EXTENSION = 'extension';
export const THREAT_FIELD_PLUGIN = 'plugin';
export const THREAT_FIELD_THEME = 'theme';
export const THREAT_FIELD_SAFETY = 'safety';
export const THREAT_FIELD_SIGNATURE = 'signature';
export const THREAT_FIELD_FIRST_DETECTED = 'first-detected';
export const THREAT_FIELD_FIXED_ON = 'fixed-on';
export const THREAT_FIELD_AUTO_FIX = 'auto-fix';
export const THREAT_FIELD_UPDATE = 'update';

export const THREAT_ACTION_FIX = 'fix';
export const THREAT_ACTION_IGNORE = 'ignore';
export const THREAT_ACTION_UNIGNORE = 'unignore';
export const THREAT_FIELD_VERSION = 'version';
23 changes: 10 additions & 13 deletions projects/js-packages/components/components/scan-report/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ import {
filterSortAndPaginate,
} from '@wordpress/dataviews';
import { __ } from '@wordpress/i18n';
import { Icon, check } from '@wordpress/icons';
import { Icon } from '@wordpress/icons';
import { useCallback, useMemo, useState } from 'react';
import {
THREAT_FIELD_EXTENSION,
THREAT_FIELD_VERSION,
THREAT_FIELD_ICON,
THREAT_FIELD_SAFETY,
THREAT_FIELD_UPDATE,
THREAT_FIELD_TYPE,
THREAT_TYPES,
THREAT_ICONS,
Expand All @@ -33,7 +33,6 @@ import styles from './styles.module.scss';
* @return {JSX.Element} The ScanReport component.
*/
export default function ScanReport( { data, filters, onChangeSelection } ): JSX.Element {
// TODO: Add types
const baseView = {
search: '',
filters: filters || [],
Expand All @@ -55,17 +54,17 @@ export default function ScanReport( { data, filters, onChangeSelection } ): JSX.
THREAT_FIELD_SAFETY,
THREAT_FIELD_TYPE,
THREAT_FIELD_EXTENSION,
THREAT_FIELD_UPDATE,
THREAT_FIELD_VERSION,
],
layout: {
primaryField: THREAT_FIELD_SAFETY,
},
},
list: {
...baseView,
fields: [ THREAT_FIELD_SAFETY, THREAT_FIELD_TYPE, THREAT_FIELD_UPDATE ],
fields: [ THREAT_FIELD_SAFETY, THREAT_FIELD_EXTENSION, THREAT_FIELD_VERSION ],
layout: {
primaryField: THREAT_FIELD_EXTENSION,
primaryField: THREAT_FIELD_TYPE,
mediaField: THREAT_FIELD_ICON,
},
},
Expand Down Expand Up @@ -144,15 +143,14 @@ export default function ScanReport( { data, filters, onChangeSelection } ): JSX.
enableGlobalSearch: true,
render( { item } ) {
return item.name ? item.name : '';
// TODO: Account for file and db?
},
},
{
id: THREAT_FIELD_UPDATE,
label: __( 'Update Available', 'jetpack' ),
render() {
return <Icon className={ styles.check } icon={ check } />;
// TODO: Is this possible to determine?
id: THREAT_FIELD_VERSION,
label: __( 'Version', 'jetpack' ),
enableGlobalSearch: true,
render( { item } ) {
return item.version ? item.version : '';
},
},
{
Expand Down Expand Up @@ -195,7 +193,6 @@ export default function ScanReport( { data, filters, onChangeSelection } ): JSX.
* @see https://developer.wordpress.org/block-editor/reference-guides/packages/packages-dataviews/#getitemid-function
*/
const getItemId = useCallback( ( item: Threat ) => item.id.toString(), [] );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const getItemId = useCallback( ( item: Threat ) => item.id.toString(), [] );
const getItemId = useCallback( ( item: Extension ) => item.slug, [] );

Copy link
Contributor Author

@dkmyta dkmyta Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How I've got this to work in Protect is by spreading the core, plugins, themes, and a modified files variable from the useProtectData hook as data and passing it to this component. The core and files variables will no include a slug so instead I've just combined the set and indexed them like so:

	const {
		results: { core, plugins, themes, files },
	} = useProtectData();

	const data = [
		...core,
		...plugins,
		...themes,
		{ checked: true, threats: files, type: 'files' },
	].map( ( item, index ) => {
		return { id: index + 1, ...item };
	} );

Obviously the Threat doesn't apply here, but Extension won't either. Any thought on how we could approach this differently?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The useProtectData hook is being removed in add/protect/core, we will be referencing data from protect-status here which should return the full Extension.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(We could add a custom type if Extension is not applicable, i.e. ScanReportExtension)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do go with generating an ID, I would recommend using the slug or any other unique identifiers whenever possible as opposed to index.

Copy link
Contributor Author

@dkmyta dkmyta Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The useProtectData hook is being removed in add/protect/core, we will be referencing data from protect-status here which should return the full Extension.

Ah good to know, I missed that entirely. How will core and file threats be formatted? Will all follow the same structure? Or will they remain unchanged.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should remain the same, with core being an Extension with threats, and files being an array of file threats.

Copy link
Contributor Author

@dkmyta dkmyta Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added/applied a custom ScanReportExtension type. I've kept with generating an ID for now, because for core the slug will always be null, and for the variable we'll have to generate from files and pass to the component, it seems a little awkward to predefine the slug in someway. Perhaps we can add a followup to investigate an improved way to use unique identifier, unless you have a different recommendation for now?

// TODO: Do we need this? dataset doesn't come with an id for each

return (
<DataViews
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export default {

export const Default = args => <ScanReport { ...args } />;
Default.args = {
// TODO: Need to pass all core, plugins, themes, files, and database as a flat array
data: [
{
id: 1,
Expand Down Expand Up @@ -52,36 +51,3 @@ Default.args = {
],
filters: [],
};

// data: {
// core: {
// name: 'WordPress',
// slug: null,
// version: '6.7.1',
// threats: [],
// checked: true,
// type: 'core',
// },
// plugins: [
// {
// name: 'Jetpack',
// slug: 'jetpack/jetpack.php',
// version: '14.1-a.7',
// threats: [],
// checked: false,
// type: 'plugins',
// },
// ],
// themes: [
// {
// name: 'Twenty Fifteen',
// slug: 'twentyfifteen',
// version: '14.1-a.7',
// threats: [],
// checked: true,
// type: 'themes',
// },
// ],
// files: [],
// database: [],
// }