From b1bc26d773395a689f98d5c7d2e721d5ce9d1fe3 Mon Sep 17 00:00:00 2001 From: Crystal Lai Date: Wed, 2 Sep 2020 10:59:43 +0200 Subject: [PATCH 1/7] Preliminary refactor into function --- gsa/src/web/pages/tasks/details.js | 404 ++++++++++++++--------------- 1 file changed, 201 insertions(+), 203 deletions(-) diff --git a/gsa/src/web/pages/tasks/details.js b/gsa/src/web/pages/tasks/details.js index 7c8f55c6bc..4221ce92e2 100644 --- a/gsa/src/web/pages/tasks/details.js +++ b/gsa/src/web/pages/tasks/details.js @@ -15,7 +15,7 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . */ -import React from 'react'; +import React, {useEffect} from 'react'; import {connect} from 'react-redux'; @@ -69,247 +69,245 @@ export const compareAlerts = (alertA, alertB) => { return 0; }; -class TaskDetails extends React.Component { - componentDidMount() { - const {entity} = this.props; +const TaskDetails = props => { + useEffect(() => { + const {entity} = props; if (hasValue(entity.config)) { - this.props.loadScanConfig(entity.config.id); + props.loadScanConfig(entity.config.id); } if (hasValue(entity.schedule)) { - this.props.loadSchedule(entity.schedule.id); + props.loadSchedule(entity.schedule.id); } - } - - render() { - const {links = true, entity} = this.props; - const { - alerts, - applyOverrides, - autoDelete, - autoDeleteData, - averageDuration, - config, - hostsOrdering, - inAssets, - reports, - minQod, - scanner, - target, - maxChecks, - maxHosts, - schedule, - sourceIface = '', - } = entity; - - const {lastReport} = reports; - - let dur; - const hasDuration = hasValue(lastReport) && hasValue(lastReport.scanStart); - if (hasDuration) { - if (hasValue(lastReport.scanEnd)) { - const diff = lastReport.scanEnd.diff(lastReport.scanStart); // Seems like scanEnd is not a valid date object or something. I get an error sometimes. - dur = duration(diff).humanize(); - } else { - dur = _('Not finished yet'); - } + }, []); + + const {links = true, entity} = props; + const { + alerts, + applyOverrides, + autoDelete, + autoDeleteData, + averageDuration, + config, + hostsOrdering, + inAssets, + reports, + minQod, + scanner, + target, + maxChecks, + maxHosts, + schedule, + sourceIface = '', + } = entity; + + const {lastReport} = reports; + + let dur; + const hasDuration = hasValue(lastReport) && hasValue(lastReport.scanStart); + if (hasDuration) { + if (hasValue(lastReport.scanEnd)) { + const diff = lastReport.scanEnd.diff(lastReport.scanStart); // Seems like scanEnd is not a valid date object or something. I get an error sometimes. + dur = duration(diff).humanize(); } else { - dur = _('No scans yet'); + dur = _('Not finished yet'); } + } else { + dur = _('No scans yet'); + } - const hasAvDuration = hasValue(averageDuration) && averageDuration > 0; - const avDuration = hasAvDuration ? averageDuration.humanize() : ''; - - return ( - - {hasValue(target) && ( - - - {target.name} - - - )} - - {hasValue(alerts) && alerts.length > 0 && ( - - - {alerts.sort(compareAlerts).map(alert => ( - - - {alert.name} - - - ))} - - - )} - - {hasValue(scanner) && ( - - - + const hasAvDuration = hasValue(averageDuration) && averageDuration > 0; + const avDuration = hasAvDuration ? averageDuration.humanize() : ''; + + return ( + + {hasValue(target) && ( + + + {target.name} + + + )} + + {hasValue(alerts) && alerts.length > 0 && ( + + + {alerts.sort(compareAlerts).map(alert => ( + + + {alert.name} + + + ))} + + + )} + + {hasValue(scanner) && ( + + + + + {_('Name')} + + + + {scanner.name} + + + + + + {_('Type')} + {scannerTypeName(scanner.scannerType)} + + {hasValue(config) && ( - {_('Name')} + {_('Scan Config')} - {scanner.name} + {config.name} - - {_('Type')} - {scannerTypeName(scanner.scannerType)} - - {hasValue(config) && ( + )} + {hasValue(config) && + config.scanConfigType === OPENVAS_SCAN_CONFIG_TYPE && + hasValue(hostsOrdering) && ( + + {_('Order for target hosts')} + {hostsOrdering} + + )} + {hasValue(config) && + config.scanConfigType === OPENVAS_SCAN_CONFIG_TYPE && ( + + {_('Network Source Interface')} + {sourceIface} + + )} + {hasValue(config) && + config.scanConfigType === OPENVAS_SCAN_CONFIG_TYPE && + hasValue(maxChecks) && ( - {_('Scan Config')} - - - {config.name} - - + {_('Maximum concurrently executed NVTs per host')} + {maxChecks} )} - {hasValue(config) && - config.scanConfigType === OPENVAS_SCAN_CONFIG_TYPE && - hasValue(hostsOrdering) && ( - - {_('Order for target hosts')} - {hostsOrdering} - - )} - {hasValue(config) && - config.scanConfigType === OPENVAS_SCAN_CONFIG_TYPE && ( - - {_('Network Source Interface')} - {sourceIface} - - )} - {hasValue(config) && - config.scanConfigType === OPENVAS_SCAN_CONFIG_TYPE && - hasValue(maxChecks) && ( - - - {_('Maximum concurrently executed NVTs per host')} - - {maxChecks} - - )} - {hasValue(config) && - config.scanConfigType === OPENVAS_SCAN_CONFIG_TYPE && - hasValue(maxHosts) && ( - - - {_('Maximum concurrently scanned hosts')} - - {maxHosts} - - )} - - - - )} - - - - - - {_('Add to Assets')} - {renderYesNo(inAssets)} - - - {inAssets === YES_VALUE && ( - - {_('Apply Overrides')} - {renderYesNo(applyOverrides)} - - )} - - {inAssets === YES_VALUE && ( - - {_('Min QoD')} - {minQod + ' %'} - - )} - - - - - {hasValue(schedule) && ( - - - - - {_('Name')} - - - - {schedule.name} - - - - - {hasValue(schedule.event) && ( + {hasValue(config) && + config.scanConfigType === OPENVAS_SCAN_CONFIG_TYPE && + hasValue(maxHosts) && ( - {_('Next')} - {dateTimeWithTimeZone(schedule.event.nextDate)} + {_('Maximum concurrently scanned hosts')} + {maxHosts} )} - - - - )} + + + + )} + + + + + + {_('Add to Assets')} + {renderYesNo(inAssets)} + - + {inAssets === YES_VALUE && ( + + {_('Apply Overrides')} + {renderYesNo(applyOverrides)} + + )} + + {inAssets === YES_VALUE && ( + + {_('Min QoD')} + {minQod + ' %'} + + )} + + + + + {hasValue(schedule) && ( + - {_('Duration of last Scan')} - {dur} + {_('Name')} + + + + {schedule.name} + + + - {hasAvDuration && ( + {hasValue(schedule.event) && ( - {_('Average Scan duration')} - {avDuration} + {_('Next')} + + {dateTimeWithTimeZone(schedule.event.nextDate)} + )} - - {_('Auto delete Reports')} - - {autoDelete === 'keep' - ? _( - 'Automatically delete oldest reports but always keep ' + - 'newest {{nr}} reports', - {nr: autoDeleteData}, - ) - : _('Do not automatically delete reports')} - - - - ); - } -} + )} + + + + + + {_('Duration of last Scan')} + {dur} + + {hasAvDuration && ( + + {_('Average Scan duration')} + {avDuration} + + )} + + {_('Auto delete Reports')} + + {autoDelete === 'keep' + ? _( + 'Automatically delete oldest reports but always keep ' + + 'newest {{nr}} reports', + {nr: autoDeleteData}, + ) + : _('Do not automatically delete reports')} + + + + + + + ); +}; TaskDetails.propTypes = { entity: PropTypes.model.isRequired, From e2fdb39a5328424cc77921ec4c95a842d8f4cecb Mon Sep 17 00:00:00 2001 From: Crystal Lai Date: Wed, 2 Sep 2020 16:15:06 +0200 Subject: [PATCH 2/7] Replace matchDispatchToProps with useDispatch --- gsa/src/web/pages/tasks/details.js | 31 ++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/gsa/src/web/pages/tasks/details.js b/gsa/src/web/pages/tasks/details.js index 4221ce92e2..b26f02a176 100644 --- a/gsa/src/web/pages/tasks/details.js +++ b/gsa/src/web/pages/tasks/details.js @@ -15,9 +15,9 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . */ -import React, {useEffect} from 'react'; +import React, {useEffect, useCallback} from 'react'; -import {connect} from 'react-redux'; +import {connect, useDispatch} from 'react-redux'; import _ from 'gmp/locale'; import {dateTimeWithTimeZone} from 'gmp/locale/date'; @@ -31,12 +31,12 @@ import {OPENVAS_SCAN_CONFIG_TYPE} from 'gmp/models/scanconfig'; import {scannerTypeName} from 'gmp/models/scanner'; import { - loadEntity as loadSchedule, + loadEntity as scheduleLoader, selector as scheduleSelector, } from 'web/store/entities/schedules'; import { - loadEntity as loadScanConfig, + loadEntity as scanConfigLoader, selector as scanConfigSelector, } from 'web/store/entities/scanconfigs'; @@ -70,14 +70,26 @@ export const compareAlerts = (alertA, alertB) => { }; const TaskDetails = props => { + const dispatch = useDispatch(); + + // Loaders + const loadScanConfig = useCallback( + id => dispatch(scanConfigLoader(props.gmp)(id)), + [props.gmp, dispatch], + ); + const loadSchedule = useCallback( + id => dispatch(scheduleLoader(props.gmp)(id)), + [props.gmp, dispatch], + ); + useEffect(() => { const {entity} = props; if (hasValue(entity.config)) { - props.loadScanConfig(entity.config.id); + loadScanConfig(entity.config.id); } if (hasValue(entity.schedule)) { - props.loadSchedule(entity.schedule.id); + loadSchedule(entity.schedule.id); } }, []); @@ -332,14 +344,9 @@ const mapStateToProps = (rootState, {entity = {}}) => { }; }; -const mapDispatchToProps = (dispatch, {gmp}) => ({ - loadScanConfig: id => dispatch(loadScanConfig(gmp)(id)), - loadSchedule: id => dispatch(loadSchedule(gmp)(id)), -}); - export default compose( withGmp, - connect(mapStateToProps, mapDispatchToProps), + connect(mapStateToProps, undefined), )(TaskDetails); // vim: set ts=2 sw=2 tw=80: From aaae2eb53b33040d081d3931d031b6666ac3094f Mon Sep 17 00:00:00 2001 From: Crystal Lai Date: Wed, 2 Sep 2020 16:37:43 +0200 Subject: [PATCH 3/7] Get rid of withGmp --- gsa/src/web/pages/tasks/details.js | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/gsa/src/web/pages/tasks/details.js b/gsa/src/web/pages/tasks/details.js index b26f02a176..14521752fa 100644 --- a/gsa/src/web/pages/tasks/details.js +++ b/gsa/src/web/pages/tasks/details.js @@ -42,7 +42,7 @@ import { import PropTypes from 'web/utils/proptypes'; import compose from 'web/utils/compose'; -import withGmp from 'web/utils/withGmp'; +import useGmp from 'web/utils/useGmp'; import {renderYesNo} from 'web/utils/render'; import HorizontalSep from 'web/components/layout/horizontalsep'; @@ -70,17 +70,18 @@ export const compareAlerts = (alertA, alertB) => { }; const TaskDetails = props => { + const gmp = useGmp(); const dispatch = useDispatch(); // Loaders const loadScanConfig = useCallback( - id => dispatch(scanConfigLoader(props.gmp)(id)), - [props.gmp, dispatch], - ); - const loadSchedule = useCallback( - id => dispatch(scheduleLoader(props.gmp)(id)), - [props.gmp, dispatch], + id => dispatch(scanConfigLoader(gmp)(id)), + [gmp, dispatch], ); + const loadSchedule = useCallback(id => dispatch(scheduleLoader(gmp)(id)), [ + gmp, + dispatch, + ]); useEffect(() => { const {entity} = props; @@ -344,9 +345,6 @@ const mapStateToProps = (rootState, {entity = {}}) => { }; }; -export default compose( - withGmp, - connect(mapStateToProps, undefined), -)(TaskDetails); +export default compose(connect(mapStateToProps, undefined))(TaskDetails); // vim: set ts=2 sw=2 tw=80: From e2142ef774a0bf4fc26c282097654a6624b074cd Mon Sep 17 00:00:00 2001 From: Crystal Lai Date: Wed, 2 Sep 2020 16:40:25 +0200 Subject: [PATCH 4/7] Remove props that are no longer used --- gsa/src/web/pages/tasks/details.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/gsa/src/web/pages/tasks/details.js b/gsa/src/web/pages/tasks/details.js index 14521752fa..9f3aaca792 100644 --- a/gsa/src/web/pages/tasks/details.js +++ b/gsa/src/web/pages/tasks/details.js @@ -69,7 +69,7 @@ export const compareAlerts = (alertA, alertB) => { return 0; }; -const TaskDetails = props => { +const TaskDetails = ({entity, links = true, ...props}) => { const gmp = useGmp(); const dispatch = useDispatch(); @@ -84,17 +84,14 @@ const TaskDetails = props => { ]); useEffect(() => { - const {entity} = props; - if (hasValue(entity.config)) { loadScanConfig(entity.config.id); } if (hasValue(entity.schedule)) { loadSchedule(entity.schedule.id); } - }, []); + }, [loadScanConfig, loadSchedule]); // eslint-disable-line react-hooks/exhaustive-deps - const {links = true, entity} = props; const { alerts, applyOverrides, @@ -324,10 +321,7 @@ const TaskDetails = props => { TaskDetails.propTypes = { entity: PropTypes.model.isRequired, - gmp: PropTypes.gmp.isRequired, links: PropTypes.bool, - loadScanConfig: PropTypes.func.isRequired, - loadSchedule: PropTypes.func.isRequired, scanConfig: PropTypes.object, schedule: PropTypes.object, }; From 61f2ea1e1785d9f939c10c8154066cd93dd0dfc6 Mon Sep 17 00:00:00 2001 From: Crystal Lai Date: Wed, 2 Sep 2020 16:45:24 +0200 Subject: [PATCH 5/7] Get rid of mapStateToProps as props.scanConfig and props.schedule does not seem used anywhere --- gsa/src/web/pages/tasks/details.js | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/gsa/src/web/pages/tasks/details.js b/gsa/src/web/pages/tasks/details.js index 9f3aaca792..5014b05559 100644 --- a/gsa/src/web/pages/tasks/details.js +++ b/gsa/src/web/pages/tasks/details.js @@ -17,7 +17,7 @@ */ import React, {useEffect, useCallback} from 'react'; -import {connect, useDispatch} from 'react-redux'; +import {useDispatch} from 'react-redux'; import _ from 'gmp/locale'; import {dateTimeWithTimeZone} from 'gmp/locale/date'; @@ -30,18 +30,11 @@ import {duration} from 'gmp/models/date'; import {OPENVAS_SCAN_CONFIG_TYPE} from 'gmp/models/scanconfig'; import {scannerTypeName} from 'gmp/models/scanner'; -import { - loadEntity as scheduleLoader, - selector as scheduleSelector, -} from 'web/store/entities/schedules'; +import {loadEntity as scheduleLoader} from 'web/store/entities/schedules'; -import { - loadEntity as scanConfigLoader, - selector as scanConfigSelector, -} from 'web/store/entities/scanconfigs'; +import {loadEntity as scanConfigLoader} from 'web/store/entities/scanconfigs'; import PropTypes from 'web/utils/proptypes'; -import compose from 'web/utils/compose'; import useGmp from 'web/utils/useGmp'; import {renderYesNo} from 'web/utils/render'; @@ -326,19 +319,6 @@ TaskDetails.propTypes = { schedule: PropTypes.object, }; -const mapStateToProps = (rootState, {entity = {}}) => { - const scheduleSel = scheduleSelector(rootState); - const scanConfigSel = scanConfigSelector(rootState); - return { - scanConfig: hasValue(entity.config) - ? scanConfigSel.getEntity(entity.config.id) - : undefined, - schedule: hasValue(entity.schedule) - ? scheduleSel.getEntity(entity.schedule.id) - : undefined, - }; -}; - -export default compose(connect(mapStateToProps, undefined))(TaskDetails); +export default TaskDetails; // vim: set ts=2 sw=2 tw=80: From f6cee87ef612cd80f26285a455a4ea2f18a8dcbd Mon Sep 17 00:00:00 2001 From: Crystal Lai Date: Wed, 2 Sep 2020 16:56:25 +0200 Subject: [PATCH 6/7] Remove unused props from proptype validation --- gsa/src/web/pages/tasks/details.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/gsa/src/web/pages/tasks/details.js b/gsa/src/web/pages/tasks/details.js index 5014b05559..424ce0d6d3 100644 --- a/gsa/src/web/pages/tasks/details.js +++ b/gsa/src/web/pages/tasks/details.js @@ -82,7 +82,7 @@ const TaskDetails = ({entity, links = true, ...props}) => { } if (hasValue(entity.schedule)) { loadSchedule(entity.schedule.id); - } + } // entity being in deps array will result in excessive rerenders }, [loadScanConfig, loadSchedule]); // eslint-disable-line react-hooks/exhaustive-deps const { @@ -315,8 +315,6 @@ const TaskDetails = ({entity, links = true, ...props}) => { TaskDetails.propTypes = { entity: PropTypes.model.isRequired, links: PropTypes.bool, - scanConfig: PropTypes.object, - schedule: PropTypes.object, }; export default TaskDetails; From c187c07f0087219b555d97403b804c2b21432a7a Mon Sep 17 00:00:00 2001 From: Crystal Lai Date: Wed, 2 Sep 2020 17:14:33 +0200 Subject: [PATCH 7/7] Remove unnecessary props var --- gsa/src/web/pages/tasks/details.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gsa/src/web/pages/tasks/details.js b/gsa/src/web/pages/tasks/details.js index 424ce0d6d3..79809825a6 100644 --- a/gsa/src/web/pages/tasks/details.js +++ b/gsa/src/web/pages/tasks/details.js @@ -62,7 +62,7 @@ export const compareAlerts = (alertA, alertB) => { return 0; }; -const TaskDetails = ({entity, links = true, ...props}) => { +const TaskDetails = ({entity, links = true}) => { const gmp = useGmp(); const dispatch = useDispatch();