From d32774f4394fb7c8c90012c2421d9fbec15d95db Mon Sep 17 00:00:00 2001 From: Anton Nikolaev Date: Mon, 16 Dec 2019 16:35:49 -0800 Subject: [PATCH] Skip Android health-checks when the "Android Developer" option is disabled in Flipper settings Summary: Skip Android health-checks when the "Android Developer" option is disabled in Flipper settings. Also made some refactoring to use immer for healthcheck reducer. Reviewed By: mweststrate Differential Revision: D19088322 fbshipit-source-id: 801d874b6e7e5af80802b4bf4313d98f1cee13f6 --- src/chrome/DoctorBar.tsx | 49 +++--- src/chrome/DoctorSheet.tsx | 76 ++++------ src/reducers/__tests__/healthchecks.node.tsx | 32 +++- src/reducers/healthchecks.tsx | 137 +++++++++-------- src/utils/icons.js | 1 + src/utils/runHealthchecks.tsx | 149 +++++++++++++------ 6 files changed, 256 insertions(+), 188 deletions(-) diff --git a/src/chrome/DoctorBar.tsx b/src/chrome/DoctorBar.tsx index 5fb29738e..90370c5a7 100644 --- a/src/chrome/DoctorBar.tsx +++ b/src/chrome/DoctorBar.tsx @@ -18,29 +18,23 @@ import { import {State as Store} from '../reducers/index'; import {ButtonGroup, Button} from 'flipper'; import {FlexColumn, FlexRow} from 'flipper'; -import runHealthchecks from '../utils/runHealthchecks'; +import runHealthchecks, { + HealthcheckSettings, + HealthcheckEventsHandler, +} from '../utils/runHealthchecks'; import { initHealthcheckReport, - updateHealthcheckReportItem, + updateHealthcheckReportItemStatus, + updateHealthcheckReportCategoryStatus, startHealthchecks, finishHealthchecks, - HealthcheckReport, - HealthcheckReportItem, } from '../reducers/healthchecks'; -type StateFromProps = {}; +type StateFromProps = HealthcheckSettings; type DispatchFromProps = { setActiveSheet: (payload: ActiveSheet) => void; - initHealthcheckReport: (report: HealthcheckReport) => void; - updateHealthcheckReportItem: ( - categoryIdx: number, - itemIdx: number, - item: HealthcheckReportItem, - ) => void; - startHealthchecks: () => void; - finishHealthchecks: () => void; -}; +} & HealthcheckEventsHandler; type State = { visible: boolean; @@ -58,12 +52,7 @@ class DoctorBar extends Component { this.showMessageIfChecksFailed(); } async showMessageIfChecksFailed() { - const result = await runHealthchecks({ - initHealthcheckReport: this.props.initHealthcheckReport, - updateHealthcheckReportItem: this.props.updateHealthcheckReportItem, - startHealthchecks: this.props.startHealthchecks, - finishHealthchecks: this.props.finishHealthchecks, - }); + const result = await runHealthchecks(this.props); if (!result) { this.setVisible(true); } @@ -106,13 +95,19 @@ class DoctorBar extends Component { } } -export default connect(null, { - setActiveSheet, - initHealthcheckReport, - updateHealthcheckReportItem, - startHealthchecks, - finishHealthchecks, -})(DoctorBar); +export default connect( + ({settingsState}) => ({ + enableAndroid: settingsState.enableAndroid, + }), + { + setActiveSheet, + initHealthcheckReport, + updateHealthcheckReportItemStatus, + updateHealthcheckReportCategoryStatus, + startHealthchecks, + finishHealthchecks, + }, +)(DoctorBar); const Container = styled.div({ boxShadow: '2px 2px 2px #ccc', diff --git a/src/chrome/DoctorSheet.tsx b/src/chrome/DoctorSheet.tsx index 5fd1084c4..ce9a6db17 100644 --- a/src/chrome/DoctorSheet.tsx +++ b/src/chrome/DoctorSheet.tsx @@ -28,27 +28,22 @@ import { HealthcheckReportItem, HealthcheckReport, initHealthcheckReport, - updateHealthcheckReportItem, + updateHealthcheckReportItemStatus, + updateHealthcheckReportCategoryStatus, startHealthchecks, finishHealthchecks, } from '../reducers/healthchecks'; -import runHealthchecks from '../utils/runHealthchecks'; +import runHealthchecks, { + HealthcheckSettings, + HealthcheckEventsHandler, +} from '../utils/runHealthchecks'; import {shell} from 'electron'; type StateFromProps = { report: HealthcheckReport; -}; +} & HealthcheckSettings; -type DispatchFromProps = { - initHealthcheckReport: (report: HealthcheckReport) => void; - updateHealthcheckReportItem: ( - categoryIdx: number, - itemIdx: number, - item: HealthcheckReportItem, - ) => void; - startHealthchecks: () => void; - finishHealthchecks: () => void; -}; +type DispatchFromProps = HealthcheckEventsHandler; const Container = styled(FlexColumn)({ padding: 20, @@ -87,7 +82,7 @@ const SideContainer = styled(FlexBox)({ const SideContainerText = styled(Text)({ display: 'block', - 'word-wrap': 'break-word', + wordWrap: 'break-word', }); const HealthcheckLabel = styled(Text)({ @@ -102,6 +97,15 @@ function HealthcheckIcon(props: {check: HealthcheckResult}) { switch (props.check.status) { case 'IN_PROGRESS': return ; + case 'SKIPPED': + return ( + + ); case 'SUCCESS': return ( { ...prevState, }; }); - await runHealthchecks({ - initHealthcheckReport: this.props.initHealthcheckReport, - updateHealthcheckReportItem: this.props.updateHealthcheckReportItem, - startHealthchecks: this.props.startHealthchecks, - finishHealthchecks: this.props.finishHealthchecks, - }); + await runHealthchecks(this.props); } hasProblems() { return this.props.report.categories.some(cat => - cat.checks.some(chk => chk.status != 'SUCCESS'), + cat.checks.some( + chk => chk.status === 'FAILED' || chk.status === 'WARNING', + ), ); } - getHealthcheckCategoryReportItem( - state: HealthcheckReportCategory, - ): HealthcheckReportItem { - return { - label: state.label, - ...(state.checks.some(c => c.status === 'IN_PROGRESS') - ? {status: 'IN_PROGRESS'} - : state.checks.every(c => c.status === 'SUCCESS') - ? {status: 'SUCCESS'} - : state.checks.some(c => c.status === 'FAILED') - ? { - status: 'FAILED', - message: 'Doctor discovered problems with the current installation', - } - : { - status: 'WARNING', - message: - 'Doctor discovered non-blocking problems with the current installation', - }), - }; - } - render() { return ( @@ -242,10 +221,7 @@ class DoctorSheet extends Component { (category, categoryIdx) => { return ( - + {category.checks.map((check, checkIdx) => ( { } export default connect( - ({healthchecks: {healthcheckReport}}) => ({ + ({healthchecks: {healthcheckReport}, settingsState}) => ({ report: healthcheckReport, + enableAndroid: settingsState.enableAndroid, }), { initHealthcheckReport, - updateHealthcheckReportItem, + updateHealthcheckReportItemStatus, + updateHealthcheckReportCategoryStatus, startHealthchecks, finishHealthchecks, }, diff --git a/src/reducers/__tests__/healthchecks.node.tsx b/src/reducers/__tests__/healthchecks.node.tsx index 46b5d5437..bfd04744b 100644 --- a/src/reducers/__tests__/healthchecks.node.tsx +++ b/src/reducers/__tests__/healthchecks.node.tsx @@ -14,7 +14,8 @@ import { HealthcheckReportCategory, HealthcheckReportItem, finishHealthchecks, - updateHealthcheckReportItem, + updateHealthcheckReportItemStatus, + updateHealthcheckReportCategoryStatus, } from '../healthchecks'; const HEALTHCHECK_ITEM: HealthcheckReportItem = { @@ -70,14 +71,14 @@ test('updateHealthcheck', () => { let res = reducer(undefined, initHealthcheckReport(report)); res = reducer( res, - updateHealthcheckReportItem(0, 0, { - label: 'Updated Test Item', + updateHealthcheckReportItemStatus(0, 0, { + message: 'Updated Test Message', status: 'SUCCESS', }), ); expect(res.healthcheckReport.isHealthcheckInProgress).toBeTruthy(); - expect(res.healthcheckReport.categories[0].checks[0].label).toEqual( - 'Updated Test Item', + expect(res.healthcheckReport.categories[0].checks[0].message).toEqual( + 'Updated Test Message', ); expect(res.healthcheckReport.categories[0].checks[0].status).toEqual( 'SUCCESS', @@ -89,3 +90,24 @@ test('updateHealthcheck', () => { 'WARNING', ); }); + +test('updateHealthcheckCategoryStatus', () => { + const report = { + isHealthcheckInProgress: true, + categories: [HEALTHCHECK_CATEGORY, HEALTHCHECK_CATEGORY], + }; + let res = reducer(undefined, initHealthcheckReport(report)); + res = reducer( + res, + updateHealthcheckReportCategoryStatus(1, { + status: 'FAILED', + message: 'Error message', + }), + ); + expect(res.healthcheckReport.isHealthcheckInProgress).toBeTruthy(); + expect(res.healthcheckReport.categories[0].label).toEqual('Test Category'); + expect(res.healthcheckReport.categories[0].status).toEqual('WARNING'); + expect(res.healthcheckReport.categories[1].label).toEqual('Test Category'); + expect(res.healthcheckReport.categories[1].status).toEqual('FAILED'); + expect(res.healthcheckReport.categories[1].message).toEqual('Error message'); +}); diff --git a/src/reducers/healthchecks.tsx b/src/reducers/healthchecks.tsx index b8f16a20a..a8f43e601 100644 --- a/src/reducers/healthchecks.tsx +++ b/src/reducers/healthchecks.tsx @@ -8,6 +8,7 @@ */ import {Actions} from './'; +import {produce} from 'flipper'; export type State = { healthcheckReport: HealthcheckReport; @@ -19,11 +20,18 @@ export type Action = payload: HealthcheckReport; } | { - type: 'UPDATE_HEALTHCHECK_REPORT_ITEM'; + type: 'UPDATE_HEALTHCHECK_REPORT_ITEM_STATUS'; payload: { categoryIdx: number; itemIdx: number; - item: HealthcheckReportItem; + status: HealthcheckResult; + }; + } + | { + type: 'UPDATE_HEALTHCHECK_REPORT_CATEGORY_STATUS'; + payload: { + categoryIdx: number; + status: HealthcheckResult; }; } | { @@ -44,6 +52,7 @@ export type HealthcheckStatus = | 'IN_PROGRESS' | 'SUCCESS' | 'FAILED' + | 'SKIPPED' | 'WARNING'; export type HealthcheckResult = { @@ -58,73 +67,64 @@ export type HealthcheckReportItem = { export type HealthcheckReportCategory = { label: string; - status: HealthcheckStatus; checks: Array; -}; +} & HealthcheckResult; export type HealthcheckReport = { isHealthcheckInProgress: boolean; categories: Array; }; +const updateReportItem = produce( + ( + draft: State, + payload: { + categoryIdx: number; + itemIdx: number; + status: HealthcheckResult; + }, + ) => { + Object.assign( + draft.healthcheckReport.categories[payload.categoryIdx].checks[ + payload.itemIdx + ], + payload.status, + ); + }, +); + +const updateCategoryStatus = produce( + (draft: State, payload: {categoryIdx: number; status: HealthcheckResult}) => { + Object.assign( + draft.healthcheckReport.categories[payload.categoryIdx], + payload.status, + ); + }, +); + +const initReport = produce((draft: State, report: HealthcheckReport) => { + draft.healthcheckReport = report; +}); + +const setIsInProgress = produce((draft: State, isInProgress: boolean) => { + draft.healthcheckReport.isHealthcheckInProgress = isInProgress; +}); + export default function reducer( - state: State | undefined = INITIAL_STATE, + draft: State | undefined = INITIAL_STATE, action: Actions, ): State { - if (action.type === 'INIT_HEALTHCHECK_REPORT') { - return { - ...state, - healthcheckReport: action.payload, - }; - } else if (action.type === 'START_HEALTHCHECKS') { - return { - ...state, - healthcheckReport: { - ...state.healthcheckReport, - isHealthcheckInProgress: true, - }, - }; - } else if (action.type === 'FINISH_HEALTHCHECKS') { - return { - ...state, - healthcheckReport: { - ...state.healthcheckReport, - isHealthcheckInProgress: false, - }, - }; - } else if (action.type === 'UPDATE_HEALTHCHECK_REPORT_ITEM') { - return { - ...state, - healthcheckReport: { - ...state.healthcheckReport, - categories: [ - ...state.healthcheckReport.categories.slice( - 0, - action.payload.categoryIdx, - ), - { - ...state.healthcheckReport.categories[action.payload.categoryIdx], - checks: [ - ...state.healthcheckReport.categories[ - action.payload.categoryIdx - ].checks.slice(0, action.payload.itemIdx), - { - ...action.payload.item, - }, - ...state.healthcheckReport.categories[ - action.payload.categoryIdx - ].checks.slice(action.payload.itemIdx + 1), - ], - }, - ...state.healthcheckReport.categories.slice( - action.payload.categoryIdx + 1, - ), - ], - }, - }; - } else { - return state; - } + return action.type === 'INIT_HEALTHCHECK_REPORT' + ? initReport(draft, action.payload) + : action.type === 'START_HEALTHCHECKS' + ? setIsInProgress(draft, true) + : action.type === 'FINISH_HEALTHCHECKS' + ? setIsInProgress(draft, false) + : action.type === 'UPDATE_HEALTHCHECK_REPORT_ITEM_STATUS' + ? updateReportItem(draft, action.payload) + : action.type === 'UPDATE_HEALTHCHECK_REPORT_CATEGORY_STATUS' + ? updateCategoryStatus(draft, action.payload) + : draft; } export const initHealthcheckReport = (report: HealthcheckReport): Action => ({ @@ -132,16 +132,27 @@ export const initHealthcheckReport = (report: HealthcheckReport): Action => ({ payload: report, }); -export const updateHealthcheckReportItem = ( +export const updateHealthcheckReportItemStatus = ( categoryIdx: number, itemIdx: number, - item: HealthcheckReportItem, + status: HealthcheckResult, ): Action => ({ - type: 'UPDATE_HEALTHCHECK_REPORT_ITEM', + type: 'UPDATE_HEALTHCHECK_REPORT_ITEM_STATUS', payload: { categoryIdx, itemIdx, - item, + status, + }, +}); + +export const updateHealthcheckReportCategoryStatus = ( + categoryIdx: number, + status: HealthcheckResult, +): Action => ({ + type: 'UPDATE_HEALTHCHECK_REPORT_CATEGORY_STATUS', + payload: { + categoryIdx, + status, }, }); diff --git a/src/utils/icons.js b/src/utils/icons.js index cdd3200f1..1d4d0d40d 100644 --- a/src/utils/icons.js +++ b/src/utils/icons.js @@ -86,6 +86,7 @@ const ICONS = { 'life-event-major': [16], target: [12, 16], tools: [12, 20], + question: [16], underline: [12], 'washing-machine': [12], 'watch-tv': [12], diff --git a/src/utils/runHealthchecks.tsx b/src/utils/runHealthchecks.tsx index f176597b3..cd7812bc6 100644 --- a/src/utils/runHealthchecks.tsx +++ b/src/utils/runHealthchecks.tsx @@ -10,7 +10,6 @@ import { HealthcheckResult, HealthcheckReport, - HealthcheckReportItem, HealthcheckReportCategory, } from '../reducers/healthchecks'; import {getHealthchecks, getEnvInfo} from 'flipper-doctor'; @@ -20,37 +19,69 @@ let runningHealthcheck: Promise; export type HealthcheckEventsHandler = { initHealthcheckReport: (report: HealthcheckReport) => void; - updateHealthcheckReportItem: ( + updateHealthcheckReportItemStatus: ( categoryIdx: number, itemIdx: number, - item: HealthcheckReportItem, + status: HealthcheckResult, + ) => void; + updateHealthcheckReportCategoryStatus: ( + categoryIdx: number, + status: HealthcheckResult, ) => void; startHealthchecks: () => void; finishHealthchecks: () => void; }; +export type HealthcheckSettings = { + enableAndroid: boolean; +}; + +export type HealthcheckOptions = HealthcheckEventsHandler & HealthcheckSettings; + async function launchHealthchecks( - dispatch: HealthcheckEventsHandler, + options: HealthcheckOptions, ): Promise { - let hasProblems: boolean = true; - dispatch.startHealthchecks(); + let healthchecksResult: boolean = true; + options.startHealthchecks(); try { - const initialState: HealthcheckResult = { + const inProgressResult: HealthcheckResult = { status: 'IN_PROGRESS', message: 'The healthcheck is in progress', }; + const androidSkippedResult: HealthcheckResult = { + status: 'SKIPPED', + message: + 'The healthcheck was skipped because Android development is disabled in the Flipper settings', + }; + const failedResult: HealthcheckResult = { + status: 'FAILED', + message: 'The healthcheck failed', + }; + const warningResult: HealthcheckResult = { + status: 'WARNING', + message: 'The optional healthcheck failed', + }; + const succeededResult: HealthcheckResult = { + status: 'SUCCESS', + message: undefined, + }; + const healthchecks = getHealthchecks(); const hcState: HealthcheckReport = { isHealthcheckInProgress: true, - categories: Object.values(getHealthchecks()) - .map(category => { + categories: Object.entries(healthchecks) + .map(([categoryKey, category]) => { if (!category) { return null; } + const state: HealthcheckResult = + categoryKey === 'android' && !options.enableAndroid + ? androidSkippedResult + : inProgressResult; return { - ...initialState, + ...state, label: category.label, checks: category.healthchecks.map(x => ({ - ...initialState, + ...state, label: x.label, })), }; @@ -58,54 +89,84 @@ async function launchHealthchecks( .filter(x => !!x) .map(x => x as HealthcheckReportCategory), }; - dispatch.initHealthcheckReport(hcState); + options.initHealthcheckReport(hcState); const environmentInfo = await getEnvInfo(); - const categories = Object.values(getHealthchecks()); - for (let cIdx = 0; cIdx < categories.length; cIdx++) { - const c = categories[cIdx]; - if (!c) { + const categories = Object.entries(healthchecks); + for (const [categoryIdx, [categoryKey, category]] of categories.entries()) { + if (!category) { continue; } - for (let hIdx = 0; hIdx < c.healthchecks.length; hIdx++) { - const h = c.healthchecks[hIdx]; - const result = await h.run(environmentInfo); - if (result.hasProblem) { - hasProblems = false; + const isSkippedAndroidCategory = + categoryKey === 'android' && !options.enableAndroid; + const allResults: HealthcheckResult[] = []; + for ( + let healthcheckIdx = 0; + healthcheckIdx < category.healthchecks.length; + healthcheckIdx++ + ) { + const h = category.healthchecks[healthcheckIdx]; + if (isSkippedAndroidCategory) { + options.updateHealthcheckReportItemStatus( + categoryIdx, + healthcheckIdx, + androidSkippedResult, + ); + allResults.push(androidSkippedResult); + } else { + const result = await h.run(environmentInfo); + if (result.hasProblem && h.isRequired) { + healthchecksResult = false; + } + const status: HealthcheckResult = + result.hasProblem && h.isRequired + ? { + ...failedResult, + helpUrl: result.helpUrl, + } + : result.hasProblem && !h.isRequired + ? { + ...warningResult, + helpUrl: result.helpUrl, + } + : succeededResult; + options.updateHealthcheckReportItemStatus( + categoryIdx, + healthcheckIdx, + status, + ); + allResults.push(status); } - dispatch.updateHealthcheckReportItem(cIdx, hIdx, { - ...h, - ...(result.hasProblem && h.isRequired - ? { - status: 'FAILED', - message: 'The healthcheck failed', - helpUrl: result.helpUrl, - } - : result.hasProblem && !h.isRequired - ? { - status: 'WARNING', - message: 'Doctor discovered a problem during the healthcheck', - helpUrl: result.helpUrl, - } - : { - status: 'SUCCESS', - message: 'The healthcheck completed succesfully', - }), - }); } + const categoryStatus = { + label: category.label, + ...(allResults.some(c => c.status === 'IN_PROGRESS') + ? inProgressResult + : allResults.every(c => c.status === 'SUCCESS') + ? succeededResult + : allResults.every(c => c.status === 'SKIPPED') + ? androidSkippedResult + : allResults.some(c => c.status === 'FAILED') + ? failedResult + : warningResult), + }; + options.updateHealthcheckReportCategoryStatus( + categoryIdx, + categoryStatus, + ); } } catch { } finally { - dispatch.finishHealthchecks(); + options.finishHealthchecks(); } - return hasProblems; + return healthchecksResult; } export default async function runHealthchecks( - dispatch: HealthcheckEventsHandler, + options: HealthcheckOptions, ): Promise { if (healthcheckIsRunning) { return runningHealthcheck; } - runningHealthcheck = launchHealthchecks(dispatch); + runningHealthcheck = launchHealthchecks(options); return runningHealthcheck; }