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.

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

Refs #30784 - MemoryAllocation use react helper #8413

Merged
merged 1 commit into from Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 1 addition & 6 deletions app/helpers/form_helper.rb
Expand Up @@ -232,12 +232,7 @@ def autocomplete_f(f, attr, options = {})
end

def byte_size_f(f, attr, options = {})
options[:class] = options[:class].to_s + ' byte_spinner' unless options[:disabled]
options[:label_help] = _("When specifying custom value, add 'MB' or 'GB' at the end. Field is not case sensitive and MB is default if unspecified.")
options[:help_block] ||= soft_limit_warning_block
options[:help_block] += f.hidden_field(attr, :class => "real-hidden-value", :id => nil)

text_f(f, attr, options)
react_form_input('memory', f, attr, options)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, but maybe plugin owners that use it can help test if it also works for them?
@tristanrobert mind take a look for foreman_fog_proxmox, there are several usages there
@shiramax mind take a look for foreman_kubevirt?

If those are the only use cases, @ezr-ondrej how about using only react_form_input so we won't need to maintain another wrapper?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the wrapper is very simple and beneficial - now we are free to change the implementation and the plugins are not required to do any changes in their code, if we would decided we want to implement this component in angular in the future, we can do so.

end

def counter_f(f, attr, options = {})
Expand Down
8 changes: 1 addition & 7 deletions app/views/compute_resources_vms/form/libvirt/_base.html.erb
Expand Up @@ -10,13 +10,7 @@
name: "#{f.object_name}[cpus]"}) %>
</div>
<%= select_f f, :cpu_mode, Foreman::Model::Libvirt::CPU_MODES, :to_s, :to_s, { }, :label => _("CPU mode"), :label_size => "col-md-2" %>
<%= react_component('MemoryAllocationInput',
{label: 'Memory',
disabled: !new_vm,
recommendedMaxValue: compute_resource.max_memory.to_i / 1.megabyte,
defaultValue: f.object.memory.present? ? f.object.memory.to_i / 1.megabytes : 1024,
id: "#{f.object_name.gsub(/[\[\]]/, '[' => '_', ']' => '_')}memory",
name: "#{f.object_name}[memory]"}) %>
<%= byte_size_f(f, :memory, disabled: !new_vm, label: _('Memory'), recommended_max_value: compute_resource.max_memory.to_i) %>
<!--TODO # Move to a helper-->
<% checked = params[:host] && params[:host][:compute_attributes] && params[:host][:compute_attributes][:start] || '1' %>
<%= checkbox_f f, :start, { :checked => (checked == '1'), :help_inline => _("Power ON this machine"), :label => _('Start'), :label_size => "col-md-2"} if new_vm && controller_name != "compute_attributes" %>
Expand Down
7 changes: 1 addition & 6 deletions app/views/compute_resources_vms/form/ovirt/_base.html.erb
Expand Up @@ -52,12 +52,7 @@ disable_mem = !new_vm || (new_vm && instance_type && instance_type.memory.presen
name: "#{f.object_name}[sockets]"}) %>
</div>
<div id="memory-input">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the wrapper div still needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we are using it to select it and change the props (value) from legacy JS

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we need to select it from the jquery code, so there is unfortunatelly no other way then add the wrapper.

<%= react_component('MemoryAllocationInput',
{label: 'Memory',
disabled: disable_mem,
defaultValue: f.object.memory.present? ? f.object.memory.to_i / 1.megabytes : 1024,
id: "#{f.object_name.gsub(/[\[\]]/, '[' => '_', ']' => '_')}memory",
name: "#{f.object_name}[memory]"}) %>
<%= byte_size_f(f, :memory, label: _('Memory'), disabled: disable_mem) %>
</div>

<% checked = params[:host] && params[:host][:compute_attributes] && params[:host][:compute_attributes][:start] || '1' %>
Expand Down
42 changes: 13 additions & 29 deletions webpack/assets/javascripts/compute_resource/ovirt.js
Expand Up @@ -14,8 +14,6 @@ import $ from 'jquery';
import { showSpinner } from '../foreman_tools';
import { testConnection } from '../foreman_compute_resource';

const megabyte = 1024 * 1024;

export function templateSelected(item) {
const template = $(item).val();

Expand All @@ -34,17 +32,8 @@ export function templateSelected(item) {
// As Instance Type values will take precence over templates values,
// we don't update memory/cores values if instance type is already selected
if (!$('#host_compute_attributes_instance_type').val()) {
const memoryInputElement = document
.getElementById('memory-input')
.getElementsByTagName('foreman-react-component')[0];
memoryInputElement.setAttribute(
'data-props',
JSON.stringify({
...memoryInputElement.reactProps,
defaultValue: result.memory / megabyte,
})
);
updateCoresAndSockets(result);
setMemoryInputProps({ value: result.memory });
$('[id$=_ha]').prop('checked', result.ha);
}
$('#network_interfaces')
Expand Down Expand Up @@ -85,27 +74,12 @@ export function instanceTypeSelected(item) {
url,
data: `instance_type_id=${instanceType}`,
success(result) {
const memoryInputElement = document
.getElementById('memory-input')
.getElementsByTagName('foreman-react-component')[0];
if (result.name != null) {
memoryInputElement.setAttribute(
'data-props',
JSON.stringify({
...memoryInputElement.reactProps,
defaultValue: result.memory / megabyte,
})
);
setMemoryInputProps({ value: result.memory });
updateCoresAndSockets(result);
$('[id$=_ha]').prop('checked', result.ha);
}
memoryInputElement.setAttribute(
'data-props',
JSON.stringify({
...memoryInputElement.reactProps,
disabled: result.name != null,
})
);
setMemoryInputProps({ disabled: result.name != null });
disableCoresAndSockets(result);
['_ha'].forEach(name =>
$(`[id$=${name}]`).prop('readOnly', result.name != null)
Expand Down Expand Up @@ -168,6 +142,16 @@ function addVolume({
.hide();
}

function setMemoryInputProps(props) {
const memoryInputElement = document
.getElementById('memory-input')
.getElementsByTagName('foreman-react-component')[0];
memoryInputElement.reactProps = {
...memoryInputElement.reactProps,
...props,
};
}

function updateCoresAndSockets(result) {
const coresInputElement = document
.getElementById('cores-input')
Expand Down
@@ -1,121 +1,81 @@
import RCInputNumber from 'rc-input-number';
import React, { useEffect, useState } from 'react';
import PropTypes from 'prop-types';
import { WarningTriangleIcon, ErrorCircleOIcon } from '@patternfly/react-icons';
import { HelpBlock, Grid, Col, Row } from 'patternfly-react';
import { translate as __ } from '../../common/I18n';
import { sprintf, translate as __ } from '../../common/I18n';
import { MB_FORMAT, MEGABYTES } from './constants';
import './memoryAllocationInput.scss';
import '../common/forms/NumericInput.scss';
import { noop } from '../../common/helpers';
import NumericInput from '../common/forms/NumericInput';

const MemoryAllocationInput = ({
defaultValue,
label,
value,
onChange,
maxValue,
minValue,
recommendedMaxValue,
name,
id,
disabled,
setError,
setWarning,
}) => {
const [validationState, setValidationState] = useState(undefined);
const [value, setValue] = useState(defaultValue);
const [valueMB, setValueMB] = useState(value / MEGABYTES);

useEffect(() => {
setValue(defaultValue);
}, [defaultValue]);

useEffect(() => {
if (value > maxValue && maxValue !== undefined) {
setValidationState('error');
} else if (
value > recommendedMaxValue &&
recommendedMaxValue !== undefined
) {
setValidationState('warning');
const valueBytes = valueMB * MEGABYTES;
if (maxValue && valueBytes > maxValue) {
setWarning(null);
setError(
sprintf(
__('Specified value is higher than maximum value %s'),
`${maxValue / MEGABYTES} ${MB_FORMAT}`
)
);
} else if (recommendedMaxValue && valueBytes > recommendedMaxValue) {
setError(null);
setWarning(
sprintf(
__('Specified value is higher than recommended maximum %s'),
`${recommendedMaxValue / MEGABYTES} ${MB_FORMAT}`
)
);
} else {
setValidationState(undefined);
setWarning(null);
}
}, [recommendedMaxValue, maxValue, value]);

const handleValueIncrease = () => {
setValue(value * 2);
};

const handleValueDecrease = () => {
setValue(value / 2);
};

const handleTypedValue = v => {
setValue(v);
};
}, [valueMB, recommendedMaxValue, maxValue, setError, setWarning]);

const handleChange = v => {
if (v === value + 1) {
handleValueIncrease();
} else if (v === value - 1) {
handleValueDecrease();
} else {
handleTypedValue(v);
}
onChange(value);
};

const format = v => `${v} ${MB_FORMAT}`;

const helpBlock = () => {
if (validationState === 'warning') {
return (
<HelpBlock>
<WarningTriangleIcon className="warning-icon" />
{__('Specified value is higher than recommended maximum')}
</HelpBlock>
);
} else if (validationState === 'error') {
return (
<HelpBlock>
<ErrorCircleOIcon className="error-icon" />
{__('Specified value is higher than maximum value')}
</HelpBlock>
);
if (v === valueMB + 1) {
v = valueMB * 2;
} else if (v === valueMB - 1) {
v = Math.floor(valueMB / 2);
}
return undefined;
setValueMB(v);
onChange(v * MEGABYTES);
};

const parser = str => str.replace(/\D/g, '');
return (
<Grid>
<Row>
<Col>
<NumericInput
value={value}
id={id}
format={format}
parser={parser}
onChange={handleChange}
label={label}
disabled={disabled}
minValue={minValue}
/>
<input type="hidden" name={name} value={value * MEGABYTES} />
</Col>
</Row>
<Row>
<Col md={2} />
<Col md={4} className="form-group">
{validationState !== undefined && helpBlock()}
</Col>
</Row>
</Grid>
<>
<RCInputNumber
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uses the external component directly, as the FormInput component takes care of the labels and other stuff.

value={valueMB}
id={id}
formatter={v => `${v} ${MB_FORMAT}`}
parser={str => str.replace(/\D/g, '')}
onChange={handleChange}
disabled={disabled}
min={minValue && minValue / MEGABYTES}
step={1}
precision={0}
name=""
prefixCls="foreman-numeric-input"
/>
<input type="hidden" name={name} value={valueMB * MEGABYTES} />
</>
);
};

MemoryAllocationInput.propTypes = {
/** Set the label of the memory allocation input */
label: PropTypes.string,
/** Set the default value of the memory allocation input */
defaultValue: PropTypes.number,
value: PropTypes.number,
/** Set the recommended max value of the numeric input */
recommendedMaxValue: PropTypes.number,
/** Set the max value of the numeric input */
Expand All @@ -124,24 +84,29 @@ MemoryAllocationInput.propTypes = {
minValue: PropTypes.number,
/** Set the onChange function of the numeric input */
onChange: PropTypes.func,
/** Set the name of the numeric input */
name: NumericInput.propTypes.name,
/** Set the name of the input holding the value in bytes */
name: PropTypes.string,
/** Set the id of the numeric input */
id: NumericInput.propTypes.id,
id: PropTypes.string,
/** Set whether the numeric input will be disabled or not */
disabled: NumericInput.propTypes.disabled,
disabled: PropTypes.bool,
/** Component passes the validation error to this function */
setError: PropTypes.func,
/** Component passes the validation warning to this function */
setWarning: PropTypes.func,
};

MemoryAllocationInput.defaultProps = {
label: __('Memory'),
defaultValue: 1,
value: 2048 * MEGABYTES,
onChange: noop,
recommendedMaxValue: undefined,
maxValue: undefined,
recommendedMaxValue: null,
maxValue: null,
minValue: 1,
name: '',
id: '',
disabled: false,
setError: noop,
setWarning: noop,
};

export default MemoryAllocationInput;
@@ -1,6 +1,7 @@
import React from 'react';
import { number, text } from '@storybook/addon-knobs';
import { number } from '@storybook/addon-knobs';
import { action } from '@storybook/addon-actions';
import { MEGABYTES } from './constants';
import MemoryAllocationInput from './MemoryAllocationInput';

export default {
Expand All @@ -13,10 +14,9 @@ export default {

export const UseMemoryAllocationInput = () => (
<MemoryAllocationInput
label={text('Label', 'Memory')}
defaultValue={number('DefaultValue', 1024)}
value={number('Value[B]', 1024 * MEGABYTES)}
onChange={action('Value was changed')}
recommendedMaxValue={number('RecommendedMaxValue', 10240)}
maxValue={number('MaxValue', 20480)}
recommendedMaxValue={number('RecommendedMaxValue', 10240 * MEGABYTES)}
maxValue={number('MaxValue', 20480 * MEGABYTES)}
/>
);
);
@@ -1,29 +1,31 @@
import React from 'react';
import { mount } from '@theforeman/test';
import { Provider } from 'react-redux';
import { MEGABYTES } from '../constants';
import MemoryAllocationInput from '../';
import Store from "../../../redux";


describe('MemoryAllocationInput', () => {

it('warning alert', async () => {
const setWarning = jest.fn();
const component = mount(
<Provider store={Store}>
<MemoryAllocationInput defaultValue={11264} recommendedMaxValue={10240} />
</Provider>
<MemoryAllocationInput
value={11264*MEGABYTES}
recommendedMaxValue={10240}
setWarning={setWarning}
/>
);
expect(component.find('.foreman-numeric-input-input').prop('value')).toEqual('11264 MB');
expect(component.find('.warning-icon').exists()).toBeTruthy();
expect(setWarning.mock.calls.length).toBe(1);
});

it('error alert', async () => {
const setError = jest.fn();
const component = mount(
<Provider store={Store}>
<MemoryAllocationInput defaultValue={21504} maxValue={20480} />
</Provider>
<MemoryAllocationInput value={21504*MEGABYTES} maxValue={20480*MEGABYTES} setError={setError} />
);
expect(component.find('.foreman-numeric-input-input').prop('value')).toEqual('21504 MB');
expect(component.find('.error-icon').exists()).toBeTruthy();
expect(setError.mock.calls.length).toBe(1);
});
});