Do not show Doctor warning on startup for already seen problems

Summary: Doctor sometimes can show false-positives and in such case there is no way to suppress showing warning message on each startup. To reduce annoyance I've added an option to save the problems already seen by user and show the Doctor warning only for new problems. Reviewed By: mweststrate Differential Revision: D19187095 fbshipit-source-id: 14c1fcc9674f47fbe0b5b0f2d5d1bceb47f7b45d
2020-01-02 08:53:45 -08:00
parent 74daa3fb1a
commit 2bee021966
11 changed files with 635 additions and 405 deletions
--- a/src/reducers/tests/healthchecks.node.tsx
+++ b/src/reducers/tests/healthchecks.node.tsx
@@ -9,105 +9,195 @@

 import {
  default as reducer,
-  initHealthcheckReport,
  startHealthchecks,
-  HealthcheckReportCategory,
-  HealthcheckReportItem,
  finishHealthchecks,
-  updateHealthcheckReportItemStatus,
-  updateHealthcheckReportCategoryStatus,
+  updateHealthcheckResult,
+  acknowledgeProblems,
 } from '../healthchecks';
+import {Healthchecks} from 'flipper-doctor';
+import {EnvironmentInfo} from 'flipper-doctor/lib/environmentInfo';

-const HEALTHCHECK_ITEM: HealthcheckReportItem = {
-  label: 'Test Check',
-  status: 'WARNING',
-  message: "Something didn't quite work.",
+const HEALTHCHECKS: Healthchecks = {
+  ios: {
+    label: 'iOS',
+    isSkipped: false,
+    isRequired: true,
+    healthchecks: [
+      {
+        label: 'SDK Installed',
+        run: async (_env: EnvironmentInfo) => {
+          return {hasProblem: false};
+        },
+      },
+    ],
+  },
+  android: {
+    label: 'Android',
+    isSkipped: false,
+    isRequired: true,
+    healthchecks: [
+      {
+        label: 'SDK Installed',
+        run: async (_env: EnvironmentInfo) => {
+          return {hasProblem: true};
+        },
+      },
+    ],
+  },
+  common: {
+    label: 'Common',
+    isSkipped: false,
+    isRequired: false,
+    healthchecks: [
+      {
+        label: 'OpenSSL Istalled',
+        run: async (_env: EnvironmentInfo) => {
+          return {hasProblem: false};
+        },
+      },
+    ],
+  },
 };

-const HEALTHCHECK_CATEGORY: HealthcheckReportCategory = {
-  label: 'Test Category',
-  status: 'WARNING',
-  checks: [HEALTHCHECK_ITEM],
-};
-
-test('initHealthcheckReport', () => {
-  const report = {
-    isHealthcheckInProgress: false,
-    categories: [],
-  };
-  const res = reducer(undefined, initHealthcheckReport(report));
-  expect(res.healthcheckReport).toEqual(report);
-});
-
 test('startHealthCheck', () => {
-  const report = {
-    isHealthcheckInProgress: false,
-    categories: [HEALTHCHECK_CATEGORY],
-  };
-  let res = reducer(undefined, initHealthcheckReport(report));
-  res = reducer(res, startHealthchecks());
-  expect(res.healthcheckReport.isHealthcheckInProgress).toBeTruthy();
-  // This seems trivial, but by getting the spread wrong, it's easy
-  // to break this.
-  expect(res.healthcheckReport.categories).toEqual([HEALTHCHECK_CATEGORY]);
+  const res = reducer(undefined, startHealthchecks(HEALTHCHECKS));
+  expect(res.healthcheckReport.status).toBe('IN_PROGRESS');
+  expect(res.healthcheckReport.categories.length).toBe(3);
+  expect(res.healthcheckReport.categories[0].status).toEqual('IN_PROGRESS');
+  expect(res.healthcheckReport.categories[0].label).toEqual('iOS');
+  expect(res.healthcheckReport.categories[0].checks.length).toEqual(1);
+  expect(res.healthcheckReport.categories[0].checks[0].label).toEqual(
+    'SDK Installed',
+  );
+  expect(res.healthcheckReport.categories[0].checks[0].status).toEqual(
+    'IN_PROGRESS',
+  );
 });

-test('finish', () => {
-  const report = {
-    isHealthcheckInProgress: true,
-    categories: [HEALTHCHECK_CATEGORY],
-  };
-  let res = reducer(undefined, initHealthcheckReport(report));
-  res = reducer(res, finishHealthchecks());
-  expect(res.healthcheckReport.isHealthcheckInProgress).toBeFalsy();
-  expect(res.healthcheckReport.categories).toEqual([HEALTHCHECK_CATEGORY]);
-});
-
-test('updateHealthcheck', () => {
-  const report = {
-    isHealthcheckInProgress: true,
-    categories: [HEALTHCHECK_CATEGORY, HEALTHCHECK_CATEGORY],
-  };
-  let res = reducer(undefined, initHealthcheckReport(report));
+test('updateHealthcheckResult', () => {
+  let res = reducer(undefined, startHealthchecks(HEALTHCHECKS));
  res = reducer(
    res,
-    updateHealthcheckReportItemStatus(0, 0, {
+    updateHealthcheckResult(0, 0, {
      message: 'Updated Test Message',
      status: 'SUCCESS',
    }),
  );
-  expect(res.healthcheckReport.isHealthcheckInProgress).toBeTruthy();
+  expect(res.healthcheckReport.status).toBe('IN_PROGRESS');
  expect(res.healthcheckReport.categories[0].checks[0].message).toEqual(
    'Updated Test Message',
  );
  expect(res.healthcheckReport.categories[0].checks[0].status).toEqual(
    'SUCCESS',
  );
-  expect(res.healthcheckReport.categories[1].checks[0].label).toEqual(
-    'Test Check',
+  expect(res.healthcheckReport.categories[0].status).toEqual('IN_PROGRESS');
+  expect(res.healthcheckReport.categories[1].checks[0].message).toBeUndefined();
+  expect(res.healthcheckReport.categories[1].checks[0].status).toEqual(
+    'IN_PROGRESS',
+  );
+  expect(res.healthcheckReport.categories[1].status).toEqual('IN_PROGRESS');
+});
+
+test('finish', () => {
+  let res = reducer(undefined, startHealthchecks(HEALTHCHECKS));
+  res = reducer(
+    res,
+    updateHealthcheckResult(0, 0, {
+      message: 'Updated Test Message',
+      status: 'SUCCESS',
+    }),
+  );
+  res = reducer(
+    res,
+    updateHealthcheckResult(1, 0, {
+      message: 'Updated Test Message',
+      status: 'SUCCESS',
+    }),
+  );
+  res = reducer(
+    res,
+    updateHealthcheckResult(2, 0, {
+      message: 'Updated Test Message',
+      status: 'SUCCESS',
+    }),
+  );
+  res = reducer(res, finishHealthchecks());
+  expect(res.healthcheckReport.status).toBe('SUCCESS');
+  expect(res.healthcheckReport.categories.map(c => c.status)).toEqual([
+    'SUCCESS',
+    'SUCCESS',
+    'SUCCESS',
+  ]);
+});
+
+test('statuses updated after healthchecks finished', () => {
+  let res = reducer(undefined, startHealthchecks(HEALTHCHECKS));
+  res = reducer(
+    res,
+    updateHealthcheckResult(1, 0, {
+      message: 'Updated Test Message',
+      status: 'FAILED',
+    }),
+  );
+  res = reducer(
+    res,
+    updateHealthcheckResult(0, 0, {
+      message: 'Updated Test Message',
+      status: 'SUCCESS',
+    }),
+  );
+  res = reducer(
+    res,
+    updateHealthcheckResult(2, 0, {
+      message: 'Updated Test Message',
+      status: 'SUCCESS',
+    }),
+  );
+  res = reducer(res, finishHealthchecks());
+  expect(res.healthcheckReport.status).toBe('FAILED');
+  expect(res.healthcheckReport.categories.map(c => c.status)).toEqual([
+    'SUCCESS',
+    'FAILED',
+    'SUCCESS',
+  ]);
+  expect(res.healthcheckReport.categories[1].checks[0].message).toEqual(
+    'Updated Test Message',
  );
  expect(res.healthcheckReport.categories[1].checks[0].status).toEqual(
-    'WARNING',
+    'FAILED',
  );
 });

-test('updateHealthcheckCategoryStatus', () => {
-  const report = {
-    isHealthcheckInProgress: true,
-    categories: [HEALTHCHECK_CATEGORY, HEALTHCHECK_CATEGORY],
-  };
-  let res = reducer(undefined, initHealthcheckReport(report));
+test('acknowledgeProblems', () => {
+  let res = reducer(undefined, startHealthchecks(HEALTHCHECKS));
  res = reducer(
    res,
-    updateHealthcheckReportCategoryStatus(1, {
+    updateHealthcheckResult(0, 0, {
      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');
+  res = reducer(
+    res,
+    updateHealthcheckResult(1, 0, {
+      status: 'SUCCESS',
+    }),
+  );
+  res = reducer(
+    res,
+    updateHealthcheckResult(2, 0, {
+      status: 'FAILED',
+    }),
+  );
+  res = reducer(res, finishHealthchecks());
+  res = reducer(res, acknowledgeProblems());
+  expect(res.healthcheckReport.categories[0].status).toEqual(
+    'FAILED_ACKNOWLEDGED',
+  );
+  expect(res.healthcheckReport.categories[0].checks[0].status).toEqual(
+    'FAILED_ACKNOWLEDGED',
+  );
+  expect(res.healthcheckReport.categories[1].status).toEqual('SUCCESS');
+  expect(res.healthcheckReport.categories[2].status).toEqual(
+    'FAILED_ACKNOWLEDGED',
+  );
 });
--- a/src/reducers/healthchecks.tsx
+++ b/src/reducers/healthchecks.tsx
@@ -7,51 +7,51 @@
 * @format
 */

-import {produce} from 'immer';
 import {Actions} from './';
+import {produce} from 'flipper';
+import {Healthchecks} from 'flipper-doctor';

 export type State = {
  healthcheckReport: HealthcheckReport;
+  acknowledgedProblems: string[];
 };

 export type Action =
-  | {
-      type: 'INIT_HEALTHCHECK_REPORT';
-      payload: HealthcheckReport;
-    }
-  | {
-      type: 'UPDATE_HEALTHCHECK_REPORT_ITEM_STATUS';
-      payload: {
-        categoryIdx: number;
-        itemIdx: number;
-        status: HealthcheckResult;
-      };
-    }
-  | {
-      type: 'UPDATE_HEALTHCHECK_REPORT_CATEGORY_STATUS';
-      payload: {
-        categoryIdx: number;
-        status: HealthcheckResult;
-      };
-    }
  | {
      type: 'START_HEALTHCHECKS';
+      payload: Healthchecks;
    }
  | {
      type: 'FINISH_HEALTHCHECKS';
+    }
+  | {
+      type: 'UPDATE_HEALTHCHECK_RESULT';
+      payload: {
+        categoryIdx: number;
+        itemIdx: number;
+        result: HealthcheckResult;
+      };
+    }
+  | {
+      type: 'ACKNOWLEDGE_PROBLEMS';
+    }
+  | {
+      type: 'RESET_ACKNOWLEDGED_PROBLEMS';
    };

 const INITIAL_STATE: State = {
  healthcheckReport: {
-    isHealthcheckInProgress: false,
+    status: 'IN_PROGRESS',
    categories: [],
  },
+  acknowledgedProblems: [],
 };

 export type HealthcheckStatus =
  | 'IN_PROGRESS'
  | 'SUCCESS'
  | 'FAILED'
+  | 'FAILED_ACKNOWLEDGED'
  | 'SKIPPED'
  | 'WARNING';

@@ -71,95 +71,194 @@ export type HealthcheckReportCategory = {
 } & HealthcheckResult;

 export type HealthcheckReport = {
-  isHealthcheckInProgress: boolean;
+  status: HealthcheckStatus;
  categories: Array<HealthcheckReportCategory>;
 };

-const updateReportItem = produce(
+function getHealthcheckIdentifier(
+  category: HealthcheckReportCategory,
+  item: HealthcheckReportItem,
+) {
+  return `${category.label} : ${item.label}`;
+}
+
+function recomputeHealthcheckStatus(draft: State): void {
+  draft.healthcheckReport.status = computeAggregatedStatus(
+    draft.healthcheckReport.categories.map(c => c.status),
+  );
+}
+
+function computeAggregatedStatus(
+  statuses: HealthcheckStatus[],
+): HealthcheckStatus {
+  return statuses.some(s => s === 'IN_PROGRESS')
+    ? 'IN_PROGRESS'
+    : statuses.every(s => s === 'SUCCESS')
+    ? 'SUCCESS'
+    : statuses.some(s => s === 'FAILED')
+    ? 'FAILED'
+    : statuses.some(s => s === 'FAILED_ACKNOWLEDGED')
+    ? 'FAILED_ACKNOWLEDGED'
+    : statuses.some(s => s === 'WARNING')
+    ? 'WARNING'
+    : 'SKIPPED';
+}
+
+const updateCheckResult = produce(
  (
    draft: State,
-    payload: {
+    {
+      categoryIdx,
+      itemIdx,
+      result,
+    }: {
      categoryIdx: number;
      itemIdx: number;
-      status: HealthcheckResult;
+      result: HealthcheckResult;
    },
  ) => {
-    Object.assign(
-      draft.healthcheckReport.categories[payload.categoryIdx].checks[
-        payload.itemIdx
-      ],
-      payload.status,
-    );
+    const category = draft.healthcheckReport.categories[categoryIdx];
+    const item = category.checks[itemIdx];
+    Object.assign(item, result);
+    if (
+      result.status === 'FAILED' &&
+      draft.acknowledgedProblems.includes(
+        getHealthcheckIdentifier(category, item),
+      )
+    ) {
+      item.status = 'FAILED_ACKNOWLEDGED';
+    }
  },
 );

-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 start = produce((draft: State, healthchecks: Healthchecks) => {
+  draft.healthcheckReport = {
+    status: 'IN_PROGRESS',
+    categories: Object.values(healthchecks)
+      .map(category => {
+        if (category.isSkipped) {
+          return {
+            status: 'SKIPPED',
+            label: category.label,
+            checks: [],
+            message: category.skipReason,
+          };
+        }
+        return {
+          status: 'IN_PROGRESS',
+          label: category.label,
+          checks: category.healthchecks.map(x => ({
+            status: 'IN_PROGRESS',
+            label: x.label,
+          })),
+        };
+      })
+      .filter(x => !!x)
+      .map(x => x as HealthcheckReportCategory),
+  };
 });

-const setIsInProgress = produce((draft: State, isInProgress: boolean) => {
-  draft.healthcheckReport.isHealthcheckInProgress = isInProgress;
+const finish = produce((draft: State) => {
+  draft.healthcheckReport.categories
+    .filter(cat => cat.status !== 'SKIPPED')
+    .forEach(cat => {
+      cat.message = undefined;
+      cat.status = computeAggregatedStatus(cat.checks.map(c => c.status));
+    });
+  recomputeHealthcheckStatus(draft);
+  if (draft.healthcheckReport.status === 'SUCCESS') {
+    setAcknowledgedProblemsToEmpty(draft);
+  }
+});
+
+const acknowledge = produce((draft: State) => {
+  draft.acknowledgedProblems = ([] as string[]).concat(
+    ...draft.healthcheckReport.categories.map(cat =>
+      cat.checks
+        .filter(
+          chk =>
+            chk.status === 'FAILED' ||
+            chk.status === 'FAILED_ACKNOWLEDGED' ||
+            chk.status === 'WARNING',
+        )
+        .map(chk => getHealthcheckIdentifier(cat, chk)),
+    ),
+  );
+  draft.healthcheckReport.categories.forEach(cat => {
+    if (cat.status === 'FAILED') {
+      cat.status = 'FAILED_ACKNOWLEDGED';
+    }
+    cat.checks.forEach(chk => {
+      if (chk.status == 'FAILED') {
+        chk.status = 'FAILED_ACKNOWLEDGED';
+      }
+    });
+  });
+  recomputeHealthcheckStatus(draft);
+});
+
+function setAcknowledgedProblemsToEmpty(draft: State) {
+  draft.acknowledgedProblems = [];
+  draft.healthcheckReport.categories.forEach(cat => {
+    if (cat.status === 'FAILED_ACKNOWLEDGED') {
+      cat.status = 'FAILED';
+    }
+    cat.checks.forEach(chk => {
+      if (chk.status == 'FAILED_ACKNOWLEDGED') {
+        chk.status = 'FAILED';
+      }
+    });
+  });
+}
+
+const resetAcknowledged = produce((draft: State) => {
+  setAcknowledgedProblemsToEmpty(draft);
+  recomputeHealthcheckStatus(draft);
 });

 export default function reducer(
  draft: State | undefined = INITIAL_STATE,
  action: Actions,
 ): State {
-  return action.type === 'INIT_HEALTHCHECK_REPORT'
-    ? initReport(draft, action.payload)
-    : action.type === 'START_HEALTHCHECKS'
-    ? setIsInProgress(draft, true)
+  return action.type === 'START_HEALTHCHECKS'
+    ? start(draft, action.payload)
    : 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)
+    ? finish(draft)
+    : action.type === 'UPDATE_HEALTHCHECK_RESULT'
+    ? updateCheckResult(draft, action.payload)
+    : action.type === 'ACKNOWLEDGE_PROBLEMS'
+    ? acknowledge(draft)
+    : action.type === 'RESET_ACKNOWLEDGED_PROBLEMS'
+    ? resetAcknowledged(draft)
    : draft;
 }

-export const initHealthcheckReport = (report: HealthcheckReport): Action => ({
-  type: 'INIT_HEALTHCHECK_REPORT',
-  payload: report,
-});
-
-export const updateHealthcheckReportItemStatus = (
+export const updateHealthcheckResult = (
  categoryIdx: number,
  itemIdx: number,
-  status: HealthcheckResult,
+  result: HealthcheckResult,
 ): Action => ({
-  type: 'UPDATE_HEALTHCHECK_REPORT_ITEM_STATUS',
+  type: 'UPDATE_HEALTHCHECK_RESULT',
  payload: {
    categoryIdx,
    itemIdx,
-    status,
+    result,
  },
 });

-export const updateHealthcheckReportCategoryStatus = (
-  categoryIdx: number,
-  status: HealthcheckResult,
-): Action => ({
-  type: 'UPDATE_HEALTHCHECK_REPORT_CATEGORY_STATUS',
-  payload: {
-    categoryIdx,
-    status,
-  },
-});
-
-export const startHealthchecks = (): Action => ({
+export const startHealthchecks = (healthchecks: Healthchecks): Action => ({
  type: 'START_HEALTHCHECKS',
+  payload: healthchecks,
 });

 export const finishHealthchecks = (): Action => ({
  type: 'FINISH_HEALTHCHECKS',
 });
+
+export const acknowledgeProblems = (): Action => ({
+  type: 'ACKNOWLEDGE_PROBLEMS',
+});
+
+export const resetAcknowledgedProblems = (): Action => ({
+  type: 'RESET_ACKNOWLEDGED_PROBLEMS',
+});
--- a/src/reducers/index.tsx
+++ b/src/reducers/index.tsx
@@ -92,7 +92,7 @@ export type State = {
  launcherSettingsState: LauncherSettingsState & PersistPartial;
  supportForm: SupportFormState;
  pluginManager: PluginManagerState;
-  healthchecks: HealthcheckState;
+  healthchecks: HealthcheckState & PersistPartial;
 };

 export type Store = ReduxStore<State, Actions>;
@@ -159,5 +159,12 @@ export default combineReducers<State, Actions>({
    },
    launcherSettings,
  ),
-  healthchecks,
+  healthchecks: persistReducer<HealthcheckState, Actions>(
+    {
+      key: 'healthchecks',
+      storage,
+      whitelist: ['acknowledgedProblems'],
+    },
+    healthchecks,
+  ),
 });