Skip to content

Commit 074b36b

Browse files
committed
add warning icon when custom and condition weights don't add up, refactor helper methods to make them shareable between components and consistent
1 parent ce820a9 commit 074b36b

File tree

23 files changed

+784
-106
lines changed

23 files changed

+784
-106
lines changed

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

Lines changed: 468 additions & 0 deletions
Large diffs are not rendered by default.

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

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,83 @@ import {
77
ExperimentCondition,
88
UpdateExperimentConditionsRequest,
99
ConditionFormData,
10+
WeightingMethod,
11+
WEIGHTING_METHOD,
1012
} from './store/experiments.model';
13+
import { ConditionWeightUpdate } from '../../features/dashboard/experiments/modals/edit-condition-weights-modal/edit-condition-weights-modal.component';
14+
15+
export const WEIGHT_CONFIG = {
16+
TOTAL_WEIGHT: 100,
17+
DECIMAL_PLACES: 2,
18+
VALIDATION_TOLERANCE: 0.1, // Allow rounding errors from equal distribution (e.g., 3 × 33.33 = 99.99)
19+
} as const;
20+
21+
// ============================================================================
22+
// Pure Functions (exported for use in selectors and other pure contexts)
23+
// ============================================================================
24+
25+
/**
26+
* Round to configured decimal places (2 decimals)
27+
*/
28+
function roundToDecimalPlaces(value: number): number {
29+
const multiplier = Math.pow(10, WEIGHT_CONFIG.DECIMAL_PLACES);
30+
return Math.round(value * multiplier) / multiplier;
31+
}
32+
33+
/**
34+
* Check if two weights are equal within tolerance
35+
*/
36+
function isWeightEqual(actual: number, expected: number): boolean {
37+
return Math.abs(actual - expected) < WEIGHT_CONFIG.VALIDATION_TOLERANCE;
38+
}
39+
40+
/**
41+
* Determine the weighting method for a set of conditions.
42+
* Returns 'equal' if all weights are the same, 'custom' otherwise.
43+
*/
44+
export function determineWeightingMethod(conditions: ExperimentCondition[] | null | undefined): WeightingMethod {
45+
if (!conditions || conditions.length === 0) {
46+
return WEIGHTING_METHOD.EQUAL; // Default to equal weighting for new experiments
47+
}
48+
49+
const expectedWeight = roundToDecimalPlaces(WEIGHT_CONFIG.TOTAL_WEIGHT / conditions.length);
50+
const isEqual = conditions.every((condition) => isWeightEqual(condition.assignmentWeight, expectedWeight));
51+
52+
return isEqual ? WEIGHTING_METHOD.EQUAL : WEIGHTING_METHOD.CUSTOM;
53+
}
54+
55+
/**
56+
* Distributes weights equally across all conditions.
57+
* All conditions get the exact same weight rounded to 2 decimals.
58+
* Total may be 99.99% or 100.01% due to rounding - this is acceptable.
59+
*/
60+
export function distributeWeightsEqually(conditions: ExperimentCondition[] | ConditionWeightUpdate[]): void {
61+
if (!conditions || conditions.length === 0) return;
62+
63+
const equalWeight = roundToDecimalPlaces(WEIGHT_CONFIG.TOTAL_WEIGHT / conditions.length);
64+
65+
conditions.forEach((condition) => {
66+
condition.assignmentWeight = equalWeight;
67+
});
68+
}
69+
70+
/**
71+
* Check if total weights sum to 100 within tolerance.
72+
* Used for validation warnings.
73+
*/
74+
export function isWeightSumValid(conditions: ExperimentCondition[] | null | undefined): boolean {
75+
if (!conditions || conditions.length === 0) {
76+
return true;
77+
}
78+
79+
const totalWeight = conditions.reduce((sum, condition) => sum + (condition.assignmentWeight || 0), 0);
80+
81+
return Math.abs(totalWeight - WEIGHT_CONFIG.TOTAL_WEIGHT) < WEIGHT_CONFIG.VALIDATION_TOLERANCE;
82+
}
83+
84+
// ============================================================================
85+
// Service Class (for stateful operations that need dependency injection)
86+
// ============================================================================
1187

1288
@Injectable({
1389
providedIn: 'root',
@@ -30,6 +106,12 @@ export class ConditionHelperService {
30106

31107
const updatedConditions = [...currentConditions, newCondition] as ExperimentCondition[];
32108

109+
// Check if weights should be distributed equally
110+
const weightingMethod = determineWeightingMethod(currentConditions);
111+
if (weightingMethod === WEIGHTING_METHOD.EQUAL) {
112+
distributeWeightsEqually(updatedConditions);
113+
}
114+
33115
this.updateExperimentConditions(experiment, updatedConditions);
34116
}
35117

@@ -71,6 +153,14 @@ export class ConditionHelperService {
71153
...c,
72154
order: index + 1,
73155
}));
156+
157+
// Check if weights should be redistributed equally
158+
const weightingMethod = determineWeightingMethod(currentConditions);
159+
160+
if (weightingMethod === WEIGHTING_METHOD.EQUAL && reorderedConditions.length > 0) {
161+
distributeWeightsEqually(reorderedConditions);
162+
}
163+
74164
this.updateExperimentConditions(updatedExperiment, reorderedConditions);
75165
}
76166

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -303,9 +303,17 @@ export interface IExperimentGraphInfo {
303303
}
304304

305305
export interface ExperimentVM extends Experiment {
306-
stat: IExperimentEnrollmentDetailStats;
306+
stat?: IExperimentEnrollmentDetailStats;
307+
weightingMethod?: WeightingMethod;
307308
}
308309

310+
export type WeightingMethod = 'equal' | 'custom';
311+
312+
export const WEIGHTING_METHOD = {
313+
EQUAL: 'equal' as WeightingMethod,
314+
CUSTOM: 'custom' as WeightingMethod,
315+
} as const;
316+
309317
export enum UPSERT_EXPERIMENT_ACTION {
310318
ADD = 'add',
311319
EDIT = 'edit',
@@ -533,7 +541,7 @@ export const EXPERIMENT_TRANSLATION_KEYS = {
533541

534542
export const EXPERIMENT_ROOT_DISPLAYED_COLUMNS = Object.values(EXPERIMENT_ROOT_COLUMN_NAMES);
535543

536-
export interface ExperimentState extends EntityState<Experiment> {
544+
export interface ExperimentState extends EntityState<ExperimentVM> {
537545
isLoadingExperiment: boolean;
538546
isLoadingExperimentDetailStats: boolean;
539547
isPollingExperimentDetailStats: boolean;

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ import {
44
EXPERIMENT_SEARCH_KEY,
55
SORT_AS_DIRECTION,
66
EXPERIMENT_SORT_KEY,
7+
ExperimentVM,
78
} from './experiments.model';
89
import { createReducer, on, Action } from '@ngrx/store';
910
import * as experimentsAction from './experiments.actions';
1011
import { createEntityAdapter, EntityAdapter } from '@ngrx/entity';
1112

12-
export const adapter: EntityAdapter<Experiment> = createEntityAdapter<Experiment>();
13+
export const adapter: EntityAdapter<ExperimentVM> = createEntityAdapter<ExperimentVM>();
1314

1415
export const { selectIds, selectEntities, selectAll, selectTotal } = adapter.getSelectors();
1516

@@ -36,6 +37,8 @@ export const initialState: ExperimentState = adapter.getInitialState({
3637
isLoadingContextMetaData: false,
3738
currentUserSelectedContext: null,
3839
isLoadingExperimentDelete: false,
40+
stat: null,
41+
weightingMethod: 'equal',
3942
});
4043

4144
const reducer = createReducer(

frontend/projects/upgrade/src/app/core/experiments/store/experiments.selector.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,8 @@ describe('Experiments Selectors', () => {
206206
],
207207
type: EXPERIMENT_TYPE.SIMPLE,
208208
assignmentAlgorithm: ASSIGNMENT_ALGORITHM.RANDOM,
209+
stat: null,
210+
weightingMethod: 'equal',
209211
},
210212
},
211213
isLoadingExperiment: false,

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { createSelector, createFeatureSelector } from '@ngrx/store';
22
import { selectAll } from './experiments.reducer';
3-
import { EXPERIMENT_SEARCH_KEY, ExperimentState, Experiment } from './experiments.model';
3+
import { EXPERIMENT_SEARCH_KEY, ExperimentState, Experiment, ExperimentVM } from './experiments.model';
44
import { selectRouterState } from '../../core.state';
55
import { ParticipantListTableRow } from '../../feature-flags/store/feature-flags.model';
66
import {
@@ -10,6 +10,7 @@ import {
1010
CONSISTENCY_RULE_DISPLAY_MAP,
1111
ASSIGNMENT_UNIT_DISPLAY_MAP,
1212
} from 'upgrade_types';
13+
import { determineWeightingMethod, isWeightSumValid } from '../condition-helper.service';
1314

1415
export const selectExperimentState = createFeatureSelector<ExperimentState>('experiments');
1516

@@ -32,7 +33,7 @@ export const selectIsLoadingExperimentDetailStats = createSelector(
3233
export const selectSelectedExperiment = createSelector(
3334
selectRouterState,
3435
selectExperimentState,
35-
(routerState, experimentState) => {
36+
(routerState, experimentState): ExperimentVM => {
3637
// be very defensive here to make sure routerState is correct
3738
const experimentId = routerState?.state?.params?.experimentId;
3839
if (experimentId && experimentState?.entities) {
@@ -45,12 +46,19 @@ export const selectSelectedExperiment = createSelector(
4546

4647
// Merge with stats if available
4748
const stat = experimentState.stats?.[experimentId] || null;
48-
return { ...experiment, stat };
49+
50+
const weightingMethod = determineWeightingMethod(experiment.conditions || []);
51+
52+
return { ...experiment, stat, weightingMethod };
4953
}
5054
return undefined;
5155
}
5256
);
5357

58+
export const selectConditionWeightsValid = createSelector(selectSelectedExperiment, (experiment): boolean => {
59+
return isWeightSumValid(experiment?.conditions || []);
60+
});
61+
5462
export const selectExperimentById = createSelector(
5563
selectExperimentState,
5664
(state, { experimentId }) => state.entities[experimentId]

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ describe('EditConditionWeightsModalComponent', () => {
245245
};
246246

247247
const { component } = await setupComponent(threeConditionsData);
248-
component.distributeWeightsEqually();
248+
component.distributeWeightsEquallyInFormControls();
249249

250250
const total = component.getCurrentTotal();
251251
expect(total).toBe(100);

0 commit comments

Comments
 (0)