Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: validation rules for disk size and memory #6573

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: validation rules for disk size and memory #6573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 64 additions & 0 deletions packages/renderer/src/lib/preferences/item-formats/EditableConnectionResourceItem.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import '@testing-library/jest-dom/vitest';

import { fireEvent } from '@testing-library/dom';
import { render, screen } from '@testing-library/svelte';
import userEvent from '@testing-library/user-event';
import { expect, test, vi } from 'vitest';
Expand Down Expand Up @@ -120,3 +121,66 @@ test('Expect onSave to be called with initial value if input is cancelled', asyn

expect(onSave).toBeCalledWith('record', 20000000000);
});

test('Expect input to render with the right units', async () => {
const record: IConfigurationPropertyRecordedSchema = {
id: 'record',
title: 'record',
parentId: 'parent.record',
description: 'Disk size',
type: 'number',
format: 'memory',
minimum: 10000000000, // 10 GB
maximum: 1500000000000, // 1.5 TB
};
const value = 10000000000; // 10 GB

const { rerender } = render(EditableConnectionResourceItem, { record, value, onSave: onSave });
expect(screen.getByTestId('editable-item-description')).toHaveTextContent('GB');
expect(screen.getByTestId('editable-item-value')).toHaveTextContent('10');

rerender({ record, value: 1000000000000, onSave: onSave });
expect(screen.getByTestId('editable-item-description')).toHaveTextContent('TB');
expect(screen.getByTestId('editable-item-value')).toHaveTextContent('1');

rerender({ record, value: 1500000000000, onSave: onSave });
expect(screen.getByTestId('editable-item-description')).toHaveTextContent('TB');
expect(screen.getByTestId('editable-item-value')).toHaveTextContent('1.5');
});

test('Expect input to render with the right units when value is updated', async () => {
const user = userEvent.setup();
const record: IConfigurationPropertyRecordedSchema = {
id: 'record',
title: 'record',
parentId: 'parent.record',
description: 'Disk size',
type: 'number',
format: 'memory',
minimum: 10000000000, // 10 GB
maximum: 1500000000000, // 1.5 TB
};
const value = 10000000000; // 10 GB

const onSaveMock = (recordId: string, newValue: number) => {
rerender({ record, value: newValue, onSave: onSaveMock });
};

const { rerender } = render(EditableConnectionResourceItem, { record, value, onSave: onSaveMock });
const buttonEdit = screen.getByRole('button', { name: 'Edit' });
expect(buttonEdit).toBeInTheDocument();
await user.click(buttonEdit);

let input = await screen.findByLabelText('Disk size');
expect(input).toBeInTheDocument();

fireEvent.input(input, { target: { value: '1000' } });
expect(screen.getByTestId('editable-item-description')).toHaveTextContent('TB');
expect(screen.getByTestId('editable-item-value')).toHaveTextContent('1');

await user.click(screen.getByRole('button', { name: 'Edit' }));
input = await screen.findByLabelText('Disk size');
fireEvent.input(input, { target: { value: '0.1' } });
expect(screen.getByTestId('editable-item-description')).toHaveTextContent('GB');
expect(screen.getByTestId('editable-item-value')).toHaveTextContent('100');
});
59 changes: 28 additions & 31 deletions packages/renderer/src/lib/preferences/item-formats/EditableConnectionResourceItem.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,20 @@ let recordValue: DisplayConfigurationValue | undefined;
$: recordValue = getDisplayConfigurationValue(record, value);

function onChangeInput(_recordId: string, _value: number) {
innerOnSave(_recordId, _value);
const displayConfigurationValue = getDisplayConfigurationValue(record, value);
if (displayConfigurationValue) {
onSave(_recordId, normalizeValue(_value));
}
}

function onCancel(_recordId: string, originalValue: number) {
innerOnSave(_recordId, originalValue);
onSave(_recordId, originalValue);
}

interface DisplayConfigurationValue {
value: number;
format?: string;
exponent?: number;
}

function getDisplayConfigurationValue(
Expand All @@ -31,13 +35,14 @@ function getDisplayConfigurationValue(
): DisplayConfigurationValue | undefined {
if (configurationKey.format === 'memory' || configurationKey.format === 'diskSize') {
const fileSizeItem = value
? filesize(value as number)
: filesize(getNormalizedDefaultNumberValue(configurationKey));
const fileSizeItems = fileSizeItem.split(' ');
// the value returned consists of a number and its format (e.g 20 GB). We return the number as value and the format as description for the editableItem component
? filesize(value as number, { output: 'object' })
: filesize(getNormalizedDefaultNumberValue(configurationKey), { output: 'object' });
// the value returned consists of a number and its format (e.g 20 GB) and its exponent(2 for MB, 3 for GB, etc).
// We return the number as value and the format as description for the editableItem component
return {
value: Number(fileSizeItems[0]),
format: fileSizeItems[1],
value: Number(fileSizeItem.value),
format: fileSizeItem.symbol,
exponent: fileSizeItem.exponent,
};
} else if (configurationKey.format === 'cpu') {
return {
Expand All @@ -48,56 +53,48 @@ function getDisplayConfigurationValue(
}

function getFileSizeValue(fileSizeItem: string): number {
return parseInt(fileSizeItem.split(' ')[0]);
return parseFloat(fileSizeItem.split(' ')[0]);
}

function normalizeDiskAndMemoryConfigurationKey(
configurationKey: IConfigurationPropertyRecordedSchema,
recordValue?: DisplayConfigurationValue,
): IConfigurationPropertyRecordedSchema {
const configurationKeyClone = Object.assign({}, configurationKey);
// if configurationKey is memory or diskSize let's convert the values in bytes to GB so, in case of errors, the message is displayed correctly to the user
// instad of having "the value must be less than 16000000000" will be ".... less than 16"
if (configurationKey.format === 'memory' || configurationKey.format === 'diskSize') {
// if configurationKey is memory or diskSize let's convert the values in bytes to the most appropriate unit so, in case of errors, the message is displayed correctly to the user
// instead of having "the value must be less than 16000000000" will be ".... less than 16"
if ((configurationKey.format === 'memory' || configurationKey.format === 'diskSize') && recordValue?.exponent) {
configurationKeyClone.maximum =
typeof configurationKey.maximum === 'number'
? getFileSizeValue(filesize(configurationKey.maximum))
? getFileSizeValue(filesize(configurationKey.maximum, { exponent: recordValue.exponent }))
: configurationKey.maximum;
configurationKeyClone.minimum = configurationKey.minimum
? getFileSizeValue(filesize(configurationKey.minimum))
? getFileSizeValue(filesize(configurationKey.minimum, { exponent: recordValue.exponent }))
: configurationKey.minimum;
configurationKeyClone.default =
typeof configurationKey.maximum === 'number' && configurationKey.default
? getFileSizeValue(filesize(configurationKey.default))
? getFileSizeValue(filesize(configurationKey.default, { exponent: recordValue.exponent }))
: configurationKey.default;
return configurationKeyClone;
}
return configurationKey;
}

function innerOnSave(_recordId: string, _value: number) {
// convert value to byte to be saved
function normalizeValue(originalValue: number): number {
const displayConfigurationValue = getDisplayConfigurationValue(record, value);
if (displayConfigurationValue) {
switch (displayConfigurationValue.format) {
case 'GB': {
_value = _value * 1000 * 1000 * 1000;
break;
}
case 'MB': {
_value = _value * 1000 * 1000;
break;
}
}
onSave(_recordId, _value);
if (!displayConfigurationValue) {
return originalValue;
}
return originalValue * Math.pow(1000, displayConfigurationValue.exponent || 0);
}
</script>

{#if record && recordValue}
<EditableItem
record="{normalizeDiskAndMemoryConfigurationKey(record)}"
record="{normalizeDiskAndMemoryConfigurationKey(record, recordValue)}"
value="{recordValue.value}"
description="{recordValue.format}"
onCancel="{onCancel}"
onChange="{onChangeInput}" />
onChange="{onChangeInput}"
normalizeOriginalValue="{normalizeValue}" />
{/if}
20 changes: 15 additions & 5 deletions packages/renderer/src/lib/preferences/item-formats/EditableItem.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export let description: string | undefined = undefined;
export let onSave = (_recordId: string, _value: number) => {};
export let onChange = (_recordId: string, _value: number) => {};
export let onCancel = (_recordId: string, _originalValue: number) => {};
export let normalizeOriginalValue: ((_originalValue: number) => number) | undefined = undefined;

let editingInProgress = false;
let editedValue: number;
Expand All @@ -38,7 +39,7 @@ function onSwitchToInProgress(e: MouseEvent) {
e.preventDefault();
// we set the originalValue to keep a record of the initial value
// if the updating is cancelled, we can reset to it
originalValue = value;
originalValue = normalizeOriginalValue ? normalizeOriginalValue(value) : value;
editingInProgress = true;
}

Expand All @@ -52,8 +53,17 @@ function onSaveClick(e: MouseEvent) {

function onCancelClick(e: MouseEvent) {
e.preventDefault();
// we set the value to the initial one - the value that was set when the edit mode was enabled
editedValue = originalValue;
if (normalizeOriginalValue) {
// Make sure we don't save the "normalized" value if it's the same as the original value(means value didn't change)
// This is important for the case when the value is 100 GB for instance, but the normalized value is 100000000000
// So we end up setting the normalized value 100000000000 as the input value which doesn't make a whole lot of sense to the end user
if (normalizeOriginalValue(value) !== originalValue) {
// we set the value to the initial one - the value that was set when the edit mode was enabled
editedValue = originalValue;
}
} else {
editedValue = originalValue;
}
editingInProgress = false;
if (record.id) {
onCancel(record.id, originalValue);
Expand All @@ -63,7 +73,7 @@ function onCancelClick(e: MouseEvent) {

<div class="flex flex-row ml-1 items-center">
{#if !editingInProgress}
{value}
<span data-testid="editable-item-value">{value}</span>
{:else}
<FloatNumberItem
record="{record}"
Expand All @@ -72,7 +82,7 @@ function onCancelClick(e: MouseEvent) {
invalidRecord="{invalidRecord}" />
{/if}
{#if description}
<span class="ml-1" aria-label="description">
<span data-testid="editable-item-description" class="ml-1" aria-label="description">
{description}
</span>
{/if}
Expand Down