Disallow firewall-rule subform submission when target / host filter value is missing (#2550)

disallow subform submission when value is missing

Author: charliepark

app/forms/firewall-rules-common.tsx

@@ -211,7 +211,7 @@ const TargetAndHostFilterSubform = ({
       )}
       <MiniTable.ClearAndAddButtons
         addButtonCopy={`Add ${sectionType === 'host' ? 'host filter' : 'target'}`}
-        disableClear={!value}
+        disabled={!value}
         onClear={() => subform.reset()}
         onSubmit={submitSubform}
       />
@@ -452,7 +452,7 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) =
         </div>
         <MiniTable.ClearAndAddButtons
           addButtonCopy="Add port filter"
-          disableClear={!portValue}
+          disabled={!portValue}
           onClear={() => portRangeForm.reset()}
           onSubmit={submitPortRange}
         />

app/forms/network-interface-edit.tsx

@@ -128,7 +128,7 @@ export function EditNetworkInterfaceForm({
         </div>
         <MiniTable.ClearAndAddButtons
           addButtonCopy="Add Transit IP"
-          disableClear={!transitIpValue}
+          disabled={!transitIpValue}
           onClear={() => transitIpsForm.reset()}
           onSubmit={submitTransitIp}
         />

app/ui/lib/MiniTable.tsx

@@ -48,7 +48,7 @@ export const RemoveCell = ({ onClick, label }: { onClick: () => void; label: str
 
 type ClearAndAddButtonsProps = {
   addButtonCopy: string
-  disableClear: boolean
+  disabled: boolean
   onClear: () => void
   onSubmit: () => void
 }
@@ -59,15 +59,15 @@ type ClearAndAddButtonsProps = {
  */
 export const ClearAndAddButtons = ({
   addButtonCopy,
-  disableClear,
+  disabled,
   onClear,
   onSubmit,
 }: ClearAndAddButtonsProps) => (
   <div className="flex justify-end gap-2.5">
-    <Button variant="ghost" size="sm" disabled={disableClear} onClick={onClear}>
+    <Button variant="ghost" size="sm" onClick={onClear} disabled={disabled}>
       Clear
     </Button>
-    <Button size="sm" onClick={onSubmit}>
+    <Button size="sm" onClick={onSubmit} disabled={disabled}>
       {addButtonCopy}
     </Button>
   </div>

test/e2e/firewall-rules.e2e.ts

@@ -160,6 +160,9 @@ test('firewall rule form targets table', async ({ page }) => {
 
   const addButton = page.getByRole('button', { name: 'Add target' })
 
+  // addButton should be disabled until a value is added
+  await expect(addButton).toBeDisabled()
+
   // add targets with overlapping names and types to test delete
 
   await targetVpcNameField.fill('abc')
@@ -181,7 +184,10 @@ test('firewall rule form targets table', async ({ page }) => {
   // add a subnet by selecting from a dropdown; make sure 'mock-subnet' is present in the dropdown,
   // even though VPC:'mock-subnet' has already been added
   await selectOption(page, 'Target type', 'VPC subnet')
+  // addButton should be disabled again, as type has changed but no value has been entered
+  await expect(addButton).toBeDisabled()
   await selectOption(page, subnetNameField, 'mock-subnet')
+  await expect(addButton).toBeEnabled()
   await addButton.click()
   await expectRowVisible(targets, { Type: 'subnet', Value: 'mock-subnet' })