Fix organization prior settings (#2518)

growthbook · May 14, 2024 · 2e41201 · 2e41201
1 parent bd82f3d
commit 2e41201
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 12 deletions.
diff --git a/packages/front-end/components/FactTables/FactMetricModal.tsx b/packages/front-end/components/FactTables/FactMetricModal.tsx
@@ -6,6 +6,7 @@ import {
   DEFAULT_FACT_METRIC_WINDOW,
   DEFAULT_LOSE_RISK_THRESHOLD,
   DEFAULT_METRIC_WINDOW_DELAY_HOURS,
+  DEFAULT_PROPER_PRIOR_STDDEV,
   DEFAULT_REGRESSION_ADJUSTMENT_DAYS,
   DEFAULT_REGRESSION_ADJUSTMENT_ENABLED,
   DEFAULT_WIN_RISK_THRESHOLD,
@@ -393,7 +394,14 @@ export default function FactMetricModal({
         existing?.regressionAdjustmentDays ||
         (settings.regressionAdjustmentDays ??
           DEFAULT_REGRESSION_ADJUSTMENT_DAYS),
-      priorSettings: existing?.priorSettings || metricDefaults.priorSettings,
+      priorSettings:
+        existing?.priorSettings ||
+        (metricDefaults.priorSettings ?? {
+          override: false,
+          proper: false,
+          mean: 0,
+          stddev: DEFAULT_PROPER_PRIOR_STDDEV,
+        }),
     },
   });
 

diff --git a/packages/front-end/components/Metrics/MetricForm/index.tsx b/packages/front-end/components/Metrics/MetricForm/index.tsx
@@ -9,6 +9,7 @@ import { useFieldArray, useForm } from "react-hook-form";
 import { FaArrowRight, FaExternalLinkAlt, FaTimes } from "react-icons/fa";
 import {
   DEFAULT_LOSE_RISK_THRESHOLD,
+  DEFAULT_PROPER_PRIOR_STDDEV,
   DEFAULT_REGRESSION_ADJUSTMENT_DAYS,
   DEFAULT_REGRESSION_ADJUSTMENT_ENABLED,
   DEFAULT_WIN_RISK_THRESHOLD,
@@ -347,7 +348,14 @@ const MetricForm: FC<MetricFormProps> = ({
         current.regressionAdjustmentDays ??
         settings.regressionAdjustmentDays ??
         DEFAULT_REGRESSION_ADJUSTMENT_DAYS,
-      priorSettings: current.priorSettings || metricDefaults.priorSettings,
+      priorSettings:
+        current.priorSettings ||
+        (metricDefaults.priorSettings ?? {
+          override: false,
+          proper: false,
+          mean: 0,
+          stddev: DEFAULT_PROPER_PRIOR_STDDEV,
+        }),
     },
   });
 

diff --git a/packages/front-end/components/Settings/BayesianPriorSettings.tsx b/packages/front-end/components/Settings/BayesianPriorSettings.tsx
@@ -1,5 +1,6 @@
 import { useFormContext } from "react-hook-form";
 import { DEFAULT_PROPER_PRIOR_STDDEV } from "shared/constants";
+import { MetricDefaults } from "@back-end/types/organization";
 import { hasFileConfig } from "@/services/env";
 import Field from "@/components/Forms/Field";
 import Toggle from "@/components/Forms/Toggle";
@@ -9,6 +10,10 @@ const percentFormatter = new Intl.NumberFormat(undefined, {
   maximumFractionDigits: 2,
 });
 
+interface FormValues {
+  metricDefaults: MetricDefaults;
+}
+
 export default function BayesianPriorSettings({
   defaultMean,
   defaultStdDev,
@@ -18,7 +23,7 @@ export default function BayesianPriorSettings({
   defaultStdDev?: number;
   labelText?: string;
 }) {
-  const form = useFormContext();
+  const form = useFormContext<FormValues>();
   return (
     <div className="form-group mb-0 mr-2">
       <label>

diff --git a/packages/front-end/hooks/useOrganizationMetricDefaults.ts b/packages/front-end/hooks/useOrganizationMetricDefaults.ts
@@ -106,8 +106,6 @@ export type OrganizationSettingsWithMetricDefaults = Omit<
     minimumSampleSize: number;
     maxPercentageChange: number;
     minPercentageChange: number;
-    windowSettings: MetricWindowSettings;
-    cappingSettings: MetricCappingSettings;
     priorSettings: MetricPriorSettings;
   };
 };
@@ -124,7 +122,6 @@ export const useOrganizationMetricDefaults = (): OrganizationMetricDefaults => {
     }),
     [orgSettings]
   );
-
   /**
    * @link OrganizationMetricDefaults#getMaxPercentageChangeForMetric
    */

diff --git a/packages/front-end/pages/settings/index.tsx b/packages/front-end/pages/settings/index.tsx
@@ -65,7 +65,6 @@ const GeneralSettingsPage = (): React.ReactElement => {
   const hasStickyBucketFeature = hasCommercialFeature("sticky-bucketing");
 
   const { metricDefaults } = useOrganizationMetricDefaults();
-
   const form = useForm<OrganizationSettingsWithMetricDefaults>({
     defaultValues: {
       visualEditorEnabled: false,
@@ -131,14 +130,13 @@ const GeneralSettingsPage = (): React.ReactElement => {
     },
   });
   const { apiCall } = useAuth();
-
-  const value = {
+  const value: OrganizationSettingsWithMetricDefaults = {
     visualEditorEnabled: form.watch("visualEditorEnabled"),
     pastExperimentsMinLength: form.watch("pastExperimentsMinLength"),
     metricAnalysisDays: form.watch("metricAnalysisDays"),
     metricDefaults: {
-      minimumSampleSize: form.watch("metricDefaults.minimumSampleSize"),
       priorSettings: form.watch("metricDefaults.priorSettings"),
+      minimumSampleSize: form.watch("metricDefaults.minimumSampleSize"),
       maxPercentageChange: form.watch("metricDefaults.maxPercentageChange"),
       minPercentageChange: form.watch("metricDefaults.minPercentageChange"),
     },
@@ -193,8 +191,15 @@ const GeneralSettingsPage = (): React.ReactElement => {
       const newVal = { ...form.getValues() };
       Object.keys(newVal).forEach((k) => {
         const hasExistingMetrics = typeof settings?.[k] !== "undefined";
-        newVal[k] = settings?.[k] || newVal[k];
-
+        if (k === "metricDefaults") {
+          // Metric defaults are nested, so take existing metric defaults only if
+          // they exist and are not empty
+          Object.keys(newVal[k]).forEach((kk) => {
+            newVal[k][kk] = settings?.[k]?.[k] ?? newVal[k][kk];
+          });
+        } else {
+          newVal[k] = settings?.[k] || newVal[k];
+        }
         // Existing values are stored as a multiplier, e.g. 50% on the UI is stored as 0.5
         // Transform these values from the UI format
         if (k === "metricDefaults" && hasExistingMetrics) {