fix: fixes react UI issues which prevented host creation and edit (#337)

* fix: Rearrange CPU flags according to Proxmox Server and add missing flag * fix: Fixes errors display for unique id and name in lxc container * fix: Fixes empty condition of maps * fix: Re-add Device field with disabled value * fix: Improves checkbox condition to match string, labels of fields and disable conditions. * fix: Code compatibility with older versions of Foreman * fix: Add tooltip in form fields * fix: Fixes filtering of vm attributes and config attributes
theforeman · Aug 1, 2024 · a40c106 · a40c106
1 parent 00d7974
commit a40c106
Show file tree

Hide file tree

Showing 15 changed files with 104 additions and 86 deletions.
diff --git a/app/helpers/proxmox_vm_attrs_helper.rb b/app/helpers/proxmox_vm_attrs_helper.rb
@@ -114,11 +114,11 @@ def additional_attrs(vms, param_scope, start_checked)
       start_after_create: vms.start_after_create,
       templated: vms.templated,
     }
-    vms_keys = [:pool, :start_after_create]
+    vms_keys = [:cpu_type, :nameserver, :searchdomain, :hostname]
     extra_attrs = ActiveSupport::HashWithIndifferentAccess.new
     attributes.each do |key, value|
       camel_key = key.to_s.include?('_') ? snake_to_camel(key.to_s).to_sym : key
-      nested_key = vms_keys.include?(key) ? key : "config_attributes[#{key}]"
+      nested_key = vms_keys.include?(key) ? "config_attributes[#{key}]" : key
       value = start_checked if key == :start_after_create
       extra_attrs[camel_key] = { name: "#{param_scope}[#{nested_key}]", value: value }
     end

diff --git a/app/views/compute_resources_vms/form/proxmox/_add_react_component_to_host_form.html.erb b/app/views/compute_resources_vms/form/proxmox/_add_react_component_to_host_form.html.erb
@@ -1,5 +1,4 @@
 <% content_for(:javascripts) do %>
   <%= webpacked_plugins_js_for :foreman_fog_proxmox %>
-  <%= javascript_include_tag 'foreman_fog_proxmox/proxmox_vm', "data-turbolinks-track" => true %>
 <% end %>
 <%= react_component('ProxmoxVmType', { registerComp: true }) unless @host.managed %>
diff --git a/webpack/components/GeneralTabContent.js b/webpack/components/GeneralTabContent.js
@@ -22,7 +22,7 @@ const GeneralTabContent = ({
       required
       type="number"
       value={general?.vmid?.value}
-      disabled={fromProfile}
+      disabled={!newVm || fromProfile}
       onChange={handleChange}
     />
     <InputField
@@ -48,7 +48,7 @@ const GeneralTabContent = ({
         label={__('Start after creation?')}
         type="checkbox"
         value={general?.startAfterCreate?.value}
-        checked={general?.startAfterCreate?.value === '1'}
+        checked={String(general?.startAfterCreate?.value) === '1'}
         onChange={handleChange}
       />
     )}
@@ -58,7 +58,7 @@ const GeneralTabContent = ({
         label={__('Create image?')}
         type="checkbox"
         value={general?.templated?.value}
-        checked={general?.templated?.value === '1'}
+        checked={String(general?.templated?.value) === '1'}
         disabled={untemplatable}
         onChange={handleChange}
       />

diff --git a/webpack/components/ProxmoxComputeSelectors.js b/webpack/components/ProxmoxComputeSelectors.js
@@ -52,6 +52,7 @@ const ProxmoxComputeSelectors = {
   ],
 
   proxmoxVgasMap: [
+    { value: '', labal: '' },
     { value: 'std', label: 'Standard VGA' },
     { value: 'vmware', label: 'Vmware compatible' },
     { value: 'qxl', label: 'SPICE' },

diff --git a/webpack/components/ProxmoxContainer/MountPoint.js b/webpack/components/ProxmoxContainer/MountPoint.js
@@ -45,16 +45,18 @@ const MountPoint = ({ id, data, storagesMap }) => {
       />
       <InputField
         name={mp?.size?.name}
-        label={__('Size')}
+        label={__('Size (GB)')}
         type="number"
         value={mp?.size?.value}
         onChange={handleChange}
       />
-      <input
+      <InputField
+        label={__('Device')}
         name={mp?.device?.name}
-        type="hidden"
+        disabled
         value={mp?.device?.value}
         onChange={handleChange}
+        tooltip={__('Device value is set automatically.')}
       />
       <input
         name={mp?.id?.name}
@@ -68,11 +70,6 @@ const MountPoint = ({ id, data, storagesMap }) => {
         value={mp?.volid?.value}
         onChange={handleChange}
       />
-      <input
-        name={mp?.storageType?.name}
-        type="hidden"
-        value={mp?.storageType?.value}
-      />
     </div>
   );
 };

diff --git a/webpack/components/ProxmoxContainer/ProxmoxContainerOptions.js b/webpack/components/ProxmoxContainer/ProxmoxContainerOptions.js
@@ -56,21 +56,22 @@ const ProxmoxContainerOptions = ({ options, storages, nodeId }) => {
         label={__('Start at boot')}
         type="checkbox"
         value={opts?.onboot?.value}
-        checked={opts?.onboot?.value === '1'}
+        checked={String(opts?.onboot?.value) === '1'}
         onChange={handleChange}
       />
       <InputField
         name={opts?.ostype?.name}
         label={__('OS Type')}
         type="select"
-        options={ProxmoxComputeSelectors.proxmoxOperatingSystemsMap}
+        options={ProxmoxComputeSelectors.proxmoxOstypesMap}
         value={opts?.ostype?.value}
         onChange={handleChange}
       />
       <InputField
         name={opts?.hostname?.name}
         label={__('Hostname')}
         value={opts?.hostname?.value}
+        disabled
         onChange={handleChange}
       />
       <InputField

diff --git a/webpack/components/ProxmoxContainer/ProxmoxContainerStorage.js b/webpack/components/ProxmoxContainer/ProxmoxContainerStorage.js
@@ -124,7 +124,7 @@ const ProxmoxContainerStorage = ({ storage, storages, nodeId, paramScope }) => {
         />
         <InputField
           name={rootfs?.size?.name}
-          label={__('Size')}
+          label={__('Size (GB)')}
           type="number"
           value={rootfs?.size?.value}
           onChange={handleChange}

diff --git a/webpack/components/ProxmoxContainer/components/NetworkInterface.js b/webpack/components/ProxmoxContainer/components/NetworkInterface.js
@@ -13,7 +13,8 @@ const NetworkInterface = ({
   existingInterfaces,
 }) => {
   const [network, setNetwork] = useState(data);
-  const [error, setError] = useState('');
+  const [idError, setIdError] = useState('');
+  const [nameError, setNameError] = useState('');
   useEffect(() => {
     const currentNetData = JSON.stringify(network);
     const parentNetData = JSON.stringify(data);
@@ -48,23 +49,23 @@ const NetworkInterface = ({
             net.data.id.value !== network.id.value
         )
       ) {
-        setError(__('Error: Duplicate ID found.'));
+        setIdError(__('Error: Duplicate ID found.'));
         return;
       }
-      setError('');
+      setIdError('');
     }
     if (updatedKey === 'name') {
-      const idValue = value;
+      const nameValue = value;
       if (
         Object.values(existingInterfaces).some(
           net =>
-            net.data.id.value === idValue &&
-            net.data.id.value !== network.id.value
+            net.data.name.value === nameValue &&
+            net.data.name.value !== network.name.value
         )
       ) {
-        setError(__('Error: Duplicate Name found.'));
+        setNameError(__('Error: Duplicate Name found.'));
       } else {
-        setError('');
+        setNameError('');
       }
     }
   };
@@ -83,7 +84,7 @@ const NetworkInterface = ({
         type="text"
         value={network?.id?.value}
         onChange={handleChange}
-        error={error}
+        error={idError}
       />
       <InputField
         name={network?.name?.name}
@@ -92,7 +93,7 @@ const NetworkInterface = ({
         type="text"
         value={network?.name?.value}
         onChange={handleChange}
-        error={error}
+        error={nameError}
       />
       <InputField
         name={network?.bridge?.name}
@@ -107,7 +108,7 @@ const NetworkInterface = ({
         label={__('DHCP IPv4')}
         type="checkbox"
         value={network?.dhcp?.value}
-        checked={network?.dhcp?.value === '1'}
+        checked={String(network?.dhcp?.value) === '1'}
         onChange={handleChange}
       />
       <InputField
@@ -130,7 +131,7 @@ const NetworkInterface = ({
         label={__('DHCP IPv6')}
         type="checkbox"
         value={network?.dhcp6?.value}
-        checked={network?.dhcp6?.value === '1'}
+        checked={String(network?.dhcp6?.value) === '1'}
         onChange={handleChange}
       />
       <InputField
@@ -167,7 +168,7 @@ const NetworkInterface = ({
         label={__('Firewall')}
         type="checkbox"
         value={network?.firewall?.value}
-        checked={network?.firewall?.value === '1'}
+        checked={String(network?.firewall?.value) === '1'}
         onChange={handleChange}
       />
     </div>

diff --git a/webpack/components/ProxmoxServer/ProxmoxServerHardware.js b/webpack/components/ProxmoxServer/ProxmoxServerHardware.js
@@ -6,19 +6,19 @@ import InputField from '../common/FormInputs';
 import ProxmoxComputeSelectors from '../ProxmoxComputeSelectors';
 import CPUFlagsModal from './components/CPUFlagsModal';
 
-const cpuFlagNames = [
-  'md_clear',
-  'pcid',
-  'spec_ctrl',
-  'ssbd',
-  'ibpb',
-  'virt_ssbd',
-  'amd_ssbd',
-  'amd_no_ssb',
-  'pdpe1gb',
-  'hv_tlbflush',
-  'hv_evmcs',
-  'aes',
+const cpuFlagsHash = [
+  { key: 'md_clear', label: 'md-clear' },
+  { key: 'pcid', label: 'pcid' },
+  { key: 'spectre', label: 'spec-ctrl' },
+  { key: 'ssbd', label: 'ssbd' },
+  { key: 'ibpb', label: 'ibpb' },
+  { key: 'virt_ssbd', label: 'virt-ssbd' },
+  { key: 'amd_ssbd', label: 'amd-ssbd' },
+  { key: 'amd_no_ssb', label: 'amd-no-ssb' },
+  { key: 'pdpe1gb', label: 'pdpe1gb' },
+  { key: 'hv_tlbflush', label: 'hv-tlbflush' },
+  { key: 'hv_evmcs', label: 'hv-evmcs' },
+  { key: 'aes', label: 'aes' },
 ];
 
 const cpuFlagDescriptions = {
@@ -28,7 +28,7 @@ const cpuFlagDescriptions = {
   pcid: __(
     'Meltdown fix cost reduction on Westmere, Sandy-, and IvyBridge Intel CPUs'
   ),
-  spec_ctrl: __('Allows improved Spectre mitigation with Intel CPUs'),
+  spectre: __('Allows improved Spectre mitigation with Intel CPUs'),
   ssbd: __('Protection for "Speculative Store Bypass" for Intel models'),
   ibpb: __('Allows improved Spectre mitigation with AMD CPUs'),
   virt_ssbd: __(
@@ -47,20 +47,20 @@ const cpuFlagDescriptions = {
   hv_evmcs: __(
     'Improve performance for nested virtualization. Only supported on Intel CPUs.'
   ),
-  aes: __('Activate AES instruction set for HW instruction'),
+  aes: __('Activate AES instruction set for HW accelaration.'),
 };
 
 const filterAndAddDescriptions = hardware =>
-  Object.keys(hardware)
-    .filter(key => cpuFlagNames.includes(key))
-    .reduce((acc, key) => {
+  cpuFlagsHash.reduce((acc, { key, label }) => {
+    if (key in hardware) {
       acc[key] = {
         ...hardware[key],
         description: cpuFlagDescriptions[key] || '',
-        label: key,
+        label,
       };
-      return acc;
-    }, {});
+    }
+    return acc;
+  }, {});
 
 const ProxmoxServerHardware = ({ hardware }) => {
   const [hw, setHw] = useState(hardware);
@@ -141,7 +141,7 @@ const ProxmoxServerHardware = ({ hardware }) => {
           label={__('Enable NUMA')}
           type="checkbox"
           value={hw?.numa?.value}
-          checked={hw?.numa?.value === '1'}
+          checked={String(hw?.numa?.value) === '1'}
           onChange={handleChange}
         />
         <div style={{ marginLeft: '5%', display: 'inline-block' }}>

diff --git a/webpack/components/ProxmoxServer/ProxmoxServerNetwork.js b/webpack/components/ProxmoxServer/ProxmoxServerNetwork.js
@@ -63,7 +63,7 @@ const ProxmoxServerNetwork = ({ network, bridges, paramScope }) => {
           name: `${paramScope}[interfaces_attributes][${nextId}][firewall]`,
           value: '0',
         },
-        link_down: {
+        linkDown: {
           name: `${paramScope}[interfaces_attributes][${nextId}][link_down]`,
           value: '0',
         },

diff --git a/webpack/components/ProxmoxServer/ProxmoxServerOptions.js b/webpack/components/ProxmoxServer/ProxmoxServerOptions.js
@@ -38,15 +38,15 @@ const ProxmoxServerOptions = ({ options }) => {
         label={__('Start at boot')}
         type="checkbox"
         value={opts?.onboot?.value}
-        checked={opts?.onboot?.value === '1'}
+        checked={String(opts?.onboot?.value) === '1'}
         onChange={handleChange}
       />
       <InputField
         name={opts?.agent?.name}
         label={__('Qemu Agent')}
         type="checkbox"
         value={opts?.agent?.value}
-        checked={opts?.agent?.value === '1'}
+        checked={String(opts?.agent?.value) === '1'}
         onChange={handleChange}
       />
       <InputField
@@ -55,7 +55,7 @@ const ProxmoxServerOptions = ({ options }) => {
         info={__('Enable/disable KVM hardware virtualization')}
         type="checkbox"
         value={opts?.kvm?.value}
-        checked={opts?.kvm?.value === '1'}
+        checked={String(opts?.kvm?.value) === '1'}
         onChange={handleChange}
       />
       <InputField

diff --git a/webpack/components/ProxmoxServer/components/HardDisk.js b/webpack/components/ProxmoxServer/components/HardDisk.js
@@ -74,15 +74,15 @@ const HardDisk = ({
         onChange={handleChange}
       />
       <input
-        name={hdd?.device?.name}
+        name={hdd?.id?.name}
         type="hidden"
-        value={hdd?.device?.value}
+        value={hdd?.id?.value}
         onChange={handleChange}
       />
       <input
-        name={hdd?.id?.name}
+        name={hdd?.volid?.name}
         type="hidden"
-        value={hdd?.id?.value}
+        value={hdd?.volid?.value}
         onChange={handleChange}
       />
       <InputField
@@ -102,6 +102,14 @@ const HardDisk = ({
         onChange={handleChange}
         error={error}
       />
+      <InputField
+        label={__('Device')}
+        name={hdd?.device?.name}
+        value={hdd?.device?.value}
+        onChange={handleChange}
+        disabled
+        tooltip={__('Device value is set automatically.')}
+      />
       <InputField
         name={hdd?.cache?.name}
         label={__('Cache')}
@@ -112,7 +120,7 @@ const HardDisk = ({
       />
       <InputField
         name={hdd?.size?.name}
-        label={__('Size')}
+        label={__('Size (GB)')}
         type="number"
         value={hdd?.size?.value}
         onChange={handleChange}

diff --git a/webpack/components/ProxmoxServer/components/NetworkInterface.js b/webpack/components/ProxmoxServer/components/NetworkInterface.js
@@ -115,15 +115,15 @@ const NetworkInterface = ({
         label={__('Firewall')}
         type="checkbox"
         value={network?.firewall?.value}
-        checked={network?.firewall?.value === '1'}
+        checked={String(network?.firewall?.value) === '1'}
         onChange={handleChange}
       />
       <InputField
         name={network?.linkDown?.name}
         label={__('Disconnect')}
         type="checkbox"
         value={network?.linkDown?.value}
-        checked={network?.linkDown?.value === '1'}
+        checked={String(network?.linkDown?.value) === '1'}
         onChange={handleChange}
       />
     </div>