Skip to content

Commit

Permalink
fix: enforce weight precision to 1 decimal (#2749)
Browse files Browse the repository at this point in the history
## About the changes
According to our docs, we only support up to 1 decimal place for
weights. This is to use integers to represent the percentages (we divide
them by 10) and supporting more decimals results in bad maths due to
floating point arithmetics.

This PRs adds Frontend and Backend validations to enforce this
restriction

Closes #2222

## Discussion points
Should we reconsider supporting more decimal places, that door remains
open, but for now we'll just adhere to our documentation because that
change would require some development.
  • Loading branch information
Gastón Fournier committed Jan 5, 2023
1 parent 1653b04 commit 29be130
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 1 deletion.
Expand Up @@ -274,6 +274,7 @@ export const EnvironmentVariantModal = ({
const isValidPercentage = (percentage: string) => {
if (!customPercentage) return true;
if (percentage === '') return false;
if (percentage.match(/\.[0-9]{2,}$/)) return false;

const percentageNumber = Number(percentage);
return percentageNumber >= 0 && percentageNumber <= 100;
Expand Down
50 changes: 50 additions & 0 deletions src/lib/schema/feature-schema.test.ts
Expand Up @@ -70,6 +70,56 @@ test('should allow weightType=fix', () => {
expect(value).toEqual(toggle);
});

test('should not allow weightType=fix with floats', () => {
const toggle = {
name: 'app.name',
type: 'release',
project: 'default',
enabled: false,
impressionData: false,
stale: false,
archived: false,
strategies: [{ name: 'default' }],
variants: [
{
name: 'variant-a',
weight: 1.5,
weightType: 'fix',
stickiness: 'default',
},
],
};

const { error } = featureSchema.validate(toggle);
expect(error.details[0].message).toEqual('Weight only supports 1 decimal');
});

test('should not allow weightType=fix with more than 1000', () => {
const toggle = {
name: 'app.name',
type: 'release',
project: 'default',
enabled: false,
impressionData: false,
stale: false,
archived: false,
strategies: [{ name: 'default' }],
variants: [
{
name: 'variant-a',
weight: 1001,
weightType: 'fix',
stickiness: 'default',
},
],
};

const { error } = featureSchema.validate(toggle);
expect(error.details[0].message).toEqual(
'"variants[0].weight" must be less than or equal to 1000',
);
});

test('should disallow weightType=unknown', () => {
const toggle = {
name: 'app.name',
Expand Down
8 changes: 7 additions & 1 deletion src/lib/schema/feature-schema.ts
Expand Up @@ -50,7 +50,13 @@ export const variantValueSchema = joi

export const variantsSchema = joi.object().keys({
name: nameType,
weight: joi.number().min(0).max(1000).required(),
weight: joi
.number()
.integer()
.message('Weight only supports 1 decimal')
.min(0)
.max(1000)
.required(),
weightType: joi.string().valid('variable', 'fix').default('variable'),
payload: joi
.object()
Expand Down

0 comments on commit 29be130

Please sign in to comment.