add warning icon when custom and condition weights don't add up, refactor helper methods to make them shareable between components and consistent

Author: danoswaltCL

frontend/projects/upgrade/src/app/core/experiments/condition-helper.service.spec.ts

@@ -7,7 +7,83 @@ import {
   ExperimentCondition,
   UpdateExperimentConditionsRequest,
   ConditionFormData,
+  WeightingMethod,
+  WEIGHTING_METHOD,
 } from './store/experiments.model';
+import { ConditionWeightUpdate } from '../../features/dashboard/experiments/modals/edit-condition-weights-modal/edit-condition-weights-modal.component';
+
+export const WEIGHT_CONFIG = {
+  TOTAL_WEIGHT: 100,
+  DECIMAL_PLACES: 2,
+  VALIDATION_TOLERANCE: 0.1, // Allow rounding errors from equal distribution (e.g., 3 × 33.33 = 99.99)
+} as const;
+
+// ============================================================================
+// Pure Functions (exported for use in selectors and other pure contexts)
+// ============================================================================
+
+/**
+ * Round to configured decimal places (2 decimals)
+ */
+function roundToDecimalPlaces(value: number): number {
+  const multiplier = Math.pow(10, WEIGHT_CONFIG.DECIMAL_PLACES);
+  return Math.round(value * multiplier) / multiplier;
+}
+
+/**
+ * Check if two weights are equal within tolerance
+ */
+function isWeightEqual(actual: number, expected: number): boolean {
+  return Math.abs(actual - expected) < WEIGHT_CONFIG.VALIDATION_TOLERANCE;
+}
+
+/**
+ * Determine the weighting method for a set of conditions.
+ * Returns 'equal' if all weights are the same, 'custom' otherwise.
+ */
+export function determineWeightingMethod(conditions: ExperimentCondition[] | null | undefined): WeightingMethod {
+  if (!conditions || conditions.length === 0) {
+    return WEIGHTING_METHOD.EQUAL; // Default to equal weighting for new experiments
+  }
+
+  const expectedWeight = roundToDecimalPlaces(WEIGHT_CONFIG.TOTAL_WEIGHT / conditions.length);
+  const isEqual = conditions.every((condition) => isWeightEqual(condition.assignmentWeight, expectedWeight));
+
+  return isEqual ? WEIGHTING_METHOD.EQUAL : WEIGHTING_METHOD.CUSTOM;
+}
+
+/**
+ * Distributes weights equally across all conditions.
+ * All conditions get the exact same weight rounded to 2 decimals.
+ * Total may be 99.99% or 100.01% due to rounding - this is acceptable.
+ */
+export function distributeWeightsEqually(conditions: ExperimentCondition[] | ConditionWeightUpdate[]): void {
+  if (!conditions || conditions.length === 0) return;
+
+  const equalWeight = roundToDecimalPlaces(WEIGHT_CONFIG.TOTAL_WEIGHT / conditions.length);
+
+  conditions.forEach((condition) => {
+    condition.assignmentWeight = equalWeight;
+  });
+}
+
+/**
+ * Check if total weights sum to 100 within tolerance.
+ * Used for validation warnings.
+ */
+export function isWeightSumValid(conditions: ExperimentCondition[] | null | undefined): boolean {
+  if (!conditions || conditions.length === 0) {
+    return true;
+  }
+
+  const totalWeight = conditions.reduce((sum, condition) => sum + (condition.assignmentWeight || 0), 0);
+
+  return Math.abs(totalWeight - WEIGHT_CONFIG.TOTAL_WEIGHT) < WEIGHT_CONFIG.VALIDATION_TOLERANCE;
+}
+
+// ============================================================================
+// Service Class (for stateful operations that need dependency injection)
+// ============================================================================
 
 @Injectable({
   providedIn: 'root',
@@ -30,6 +106,12 @@ export class ConditionHelperService {
 
     const updatedConditions = [...currentConditions, newCondition] as ExperimentCondition[];
 
+    // Check if weights should be distributed equally
+    const weightingMethod = determineWeightingMethod(currentConditions);
+    if (weightingMethod === WEIGHTING_METHOD.EQUAL) {
+      distributeWeightsEqually(updatedConditions);
+    }
+
     this.updateExperimentConditions(experiment, updatedConditions);
   }
 
@@ -71,6 +153,14 @@ export class ConditionHelperService {
       ...c,
       order: index + 1,
     }));
+
+    // Check if weights should be redistributed equally
+    const weightingMethod = determineWeightingMethod(currentConditions);
+
+    if (weightingMethod === WEIGHTING_METHOD.EQUAL && reorderedConditions.length > 0) {
+      distributeWeightsEqually(reorderedConditions);
+    }
+
     this.updateExperimentConditions(updatedExperiment, reorderedConditions);
   }
 

frontend/projects/upgrade/src/app/core/experiments/condition-helper.service.ts

@@ -304,8 +304,16 @@ export interface IExperimentGraphInfo {
 
 export interface ExperimentVM extends Experiment {
   stat: IExperimentEnrollmentDetailStats;
+  weightingMethod?: WeightingMethod;
 }
 
+export type WeightingMethod = 'equal' | 'custom';
+
+export const WEIGHTING_METHOD = {
+  EQUAL: 'equal' as WeightingMethod,
+  CUSTOM: 'custom' as WeightingMethod,
+} as const;
+
 export enum UPSERT_EXPERIMENT_ACTION {
   ADD = 'add',
   EDIT = 'edit',

frontend/projects/upgrade/src/app/core/experiments/store/experiments.model.ts

@@ -1,6 +1,6 @@
 import { createSelector, createFeatureSelector } from '@ngrx/store';
 import { selectAll } from './experiments.reducer';
-import { EXPERIMENT_SEARCH_KEY, ExperimentState, Experiment } from './experiments.model';
+import { EXPERIMENT_SEARCH_KEY, ExperimentState, Experiment, ExperimentVM } from './experiments.model';
 import { selectRouterState } from '../../core.state';
 import { ParticipantListTableRow } from '../../feature-flags/store/feature-flags.model';
 import {
@@ -10,6 +10,7 @@ import {
   CONSISTENCY_RULE_DISPLAY_MAP,
   ASSIGNMENT_UNIT_DISPLAY_MAP,
 } from 'upgrade_types';
+import { determineWeightingMethod, isWeightSumValid } from '../condition-helper.service';
 
 export const selectExperimentState = createFeatureSelector<ExperimentState>('experiments');
 
@@ -32,7 +33,7 @@ export const selectIsLoadingExperimentDetailStats = createSelector(
 export const selectSelectedExperiment = createSelector(
   selectRouterState,
   selectExperimentState,
-  (routerState, experimentState) => {
+  (routerState, experimentState): ExperimentVM => {
     // be very defensive here to make sure routerState is correct
     const experimentId = routerState?.state?.params?.experimentId;
     if (experimentId && experimentState?.entities) {
@@ -45,12 +46,19 @@ export const selectSelectedExperiment = createSelector(
 
       // Merge with stats if available
       const stat = experimentState.stats?.[experimentId] || null;
-      return { ...experiment, stat };
+
+      const weightingMethod = determineWeightingMethod(experiment.conditions || []);
+
+      return { ...experiment, stat, weightingMethod };
     }
     return undefined;
   }
 );
 
+export const selectConditionWeightsValid = createSelector(selectSelectedExperiment, (experiment): boolean => {
+  return isWeightSumValid(experiment?.conditions || []);
+});
+
 export const selectExperimentById = createSelector(
   selectExperimentState,
   (state, { experimentId }) => state.entities[experimentId]

frontend/projects/upgrade/src/app/core/experiments/store/experiments.selectors.ts

@@ -245,7 +245,7 @@ describe('EditConditionWeightsModalComponent', () => {
       };
 
       const { component } = await setupComponent(threeConditionsData);
-      component.distributeWeightsEqually();
+      component.distributeWeightsEquallyInFormControls();
 
       const total = component.getCurrentTotal();
       expect(total).toBe(100);

frontend/projects/upgrade/src/app/features/dashboard/experiments/modals/edit-condition-weights-modal/edit-condition-weights-modal.component.spec.ts

@@ -21,6 +21,12 @@ import { Observable, combineLatest, map, startWith } from 'rxjs';
 import { CommonModalComponent } from '../../../../../shared-standalone-component-lib/components';
 import { CommonFormHelpersService } from '../../../../../shared/services/common-form-helpers.service';
 import { CommonModalConfig } from '../../../../../shared-standalone-component-lib/components/common-modal/common-modal.types';
+import {
+  ExperimentCondition,
+  WeightingMethod,
+  WEIGHTING_METHOD,
+} from '../../../../../core/experiments/store/experiments.model';
+import { distributeWeightsEqually, WEIGHT_CONFIG } from '../../../../../core/experiments/condition-helper.service';
 
 export interface ConditionWeightUpdate {
   conditionId: string;
@@ -52,15 +58,15 @@ export class EditConditionWeightsModalComponent implements OnInit {
   conditions: ConditionWeightUpdate[] = [];
   weightingMethods = [
     {
-      value: 'equal',
+      value: WEIGHTING_METHOD.EQUAL,
       name: this.translate.instant('experiments.edit-condition-weights-modal.equal-assignment-weights.label.text'),
       description: this.translate.instant(
         'experiments.edit-condition-weights-modal.equal-assignment-weights.description.text'
       ),
       disabled: false,
     },
     {
-      value: 'custom',
+      value: WEIGHTING_METHOD.CUSTOM,
       name: this.translate.instant('experiments.edit-condition-weights-modal.custom-percentages.label.text'),
       description: this.translate.instant(
         'experiments.edit-condition-weights-modal.custom-percentages.description.text'
@@ -71,7 +77,10 @@ export class EditConditionWeightsModalComponent implements OnInit {
 
   constructor(
     @Inject(MAT_DIALOG_DATA)
-    public config: CommonModalConfig<{ experimentWeightsArray: ConditionWeightUpdate[] }>,
+    public config: CommonModalConfig<{
+      experimentWeightsArray: ConditionWeightUpdate[];
+      weightingMethod: WeightingMethod;
+    }>,
     public dialog: MatDialog,
     private readonly formBuilder: FormBuilder,
     private translate: TranslateService,
@@ -83,28 +92,25 @@ export class EditConditionWeightsModalComponent implements OnInit {
   }
 
   createconditionWeightForm(): void {
-    const { experimentWeightsArray } = this.config.params;
+    const { experimentWeightsArray, weightingMethod } = this.config.params;
     this.conditions = experimentWeightsArray;
 
-    // Determine initial weighting method based on existing weights
-    const initialWeightingMethod = this.determineInitialWeightingMethod(experimentWeightsArray);
-
     // Create FormArray for conditions with individual validators
     const conditionsFormArray = this.formBuilder.array(
       experimentWeightsArray.map((condition) =>
         this.formBuilder.group({
           conditionCode: [condition.conditionCode],
           assignmentWeight: [
             condition.assignmentWeight,
-            [Validators.required, Validators.min(0), Validators.max(100), this.decimalValidator],
+            [Validators.required, Validators.min(0), Validators.max(WEIGHT_CONFIG.TOTAL_WEIGHT), this.decimalValidator],
           ],
         })
       ),
       [this.totalWeightValidator] // Array-level validator for sum = 100
     );
 
     this.conditionWeightForm = this.formBuilder.group({
-      weightingMethod: [initialWeightingMethod, Validators.required],
+      weightingMethod: [weightingMethod, Validators.required],
       conditions: conditionsFormArray,
     });
 
@@ -115,38 +121,13 @@ export class EditConditionWeightsModalComponent implements OnInit {
     this.watchWeightingMethodChanges();
 
     // Set initial input state based on the determined method
-    if (initialWeightingMethod === 'equal') {
+    if (weightingMethod === WEIGHTING_METHOD.EQUAL) {
       this.disableWeightInputs();
     } else {
       this.enableWeightInputs();
     }
   }
 
-  private determineInitialWeightingMethod(conditions: ConditionWeightUpdate[]): string {
-    if (!conditions || conditions.length === 0) {
-      return 'equal';
-    }
-
-    if (conditions.length === 1) {
-      // Single condition should always be 100%
-      return Math.abs(conditions[0].assignmentWeight - 100) < 0.01 ? 'equal' : 'custom';
-    }
-
-    const expectedEqualWeight = 100 / conditions.length;
-
-    // Check if all weights are close to the expected equal distribution
-    const areWeightsEquallyDistributed = conditions.every(
-      (condition) => Math.abs(condition.assignmentWeight - expectedEqualWeight) < 0.01
-    );
-
-    // Additional check: ensure total is close to 100%
-    const totalWeight = conditions.reduce((sum, condition) => sum + condition.assignmentWeight, 0);
-    const isTotalValid = Math.abs(totalWeight - 100) < 0.01;
-
-    // Return 'equal' only if weights are equally distributed AND total is valid
-    return areWeightsEquallyDistributed && isTotalValid ? 'equal' : 'custom';
-  }
-
   private setupFormValidation(): void {
     this.isPrimaryButtonDisabled$ = combineLatest([
       this.conditionWeightForm.statusChanges.pipe(startWith(this.conditionWeightForm.status)),
@@ -166,10 +147,10 @@ export class EditConditionWeightsModalComponent implements OnInit {
     this.conditionWeightForm.get('weightingMethod')?.valueChanges.subscribe((method) => {
       this.conditionWeightForm.markAsDirty();
 
-      if (method === 'equal') {
-        this.distributeWeightsEqually();
+      if (method === WEIGHTING_METHOD.EQUAL) {
+        this.distributeWeightsEquallyInFormControls();
         this.disableWeightInputs();
-      } else if (method === 'custom') {
+      } else if (method === WEIGHTING_METHOD.CUSTOM) {
         this.enableWeightInputs();
       } else if (method === null) {
         this.disableWeightInputs();
@@ -188,9 +169,9 @@ export class EditConditionWeightsModalComponent implements OnInit {
       return { invalidNumber: true };
     }
 
-    // Allow up to 2 decimal places
+    // Allow up to configured decimal places
     const decimalPlaces = (control.value.toString().split('.')[1] || '').length;
-    if (decimalPlaces > 2) {
+    if (decimalPlaces > WEIGHT_CONFIG.DECIMAL_PLACES) {
       return { tooManyDecimals: true };
     }
 
@@ -210,13 +191,13 @@ export class EditConditionWeightsModalComponent implements OnInit {
       },
       [0, {}] as [number, ValidationErrors]
     );
-    const isValid = Math.abs(total - 100) < 0.01;
+    const isValid = Math.abs(total - WEIGHT_CONFIG.TOTAL_WEIGHT) < WEIGHT_CONFIG.VALIDATION_TOLERANCE;
     const totalValidation = isValid
       ? {}
       : {
           totalWeightInvalid: {
             actualTotal: Math.round(total * 100) / 100,
-            expectedTotal: 100,
+            expectedTotal: WEIGHT_CONFIG.TOTAL_WEIGHT,
           },
         };
     const allErrors = {
@@ -241,18 +222,13 @@ export class EditConditionWeightsModalComponent implements OnInit {
     }, 0);
   }
 
-  distributeWeightsEqually(): void {
-    const equalWeight = Math.round((100 / this.conditions.length) * 100) / 100;
-    let remainingWeight = 100;
+  distributeWeightsEquallyInFormControls(): void {
+    // Use service to calculate equal weights
+    distributeWeightsEqually(this.conditions);
 
+    // Apply to form controls
     this.conditionsFormArray.controls.forEach((control, index) => {
-      if (index === this.conditionsFormArray.controls.length - 1) {
-        // Last condition gets the remaining weight to ensure total = 100
-        control.get('assignmentWeight')?.setValue(Math.round(remainingWeight * 100) / 100);
-      } else {
-        control.get('assignmentWeight')?.setValue(equalWeight);
-        remainingWeight -= equalWeight;
-      }
+      control.get('assignmentWeight')?.setValue(this.conditions[index].assignmentWeight);
     });
   }
 

frontend/projects/upgrade/src/app/features/dashboard/experiments/modals/edit-condition-weights-modal/edit-condition-weights-modal.component.ts

@@ -5,6 +5,7 @@
     [title]="'experiments.details.conditions.card.title.text' | translate"
     [subtitle]="'experiments.details.conditions.card.subtitle.text' | translate"
     [tableRowCount]="experiment.conditions?.length || 0"
+    [warningMessage]="weightWarningText$ | async"
   ></app-common-section-card-title-header>
 
   <!-- header-right -->