Explorar el Código

fix: argument validation on bindings

jamesread hace 6 meses
padre
commit
b4157ed5ba

+ 21 - 0
frontend/js/OutputTerminal.js

@@ -74,4 +74,25 @@ export class OutputTerminal {
   resize (cols, rows) {
     this.terminal.resize(cols, rows)
   }
+
+  /**
+   * Get the terminal buffer content as a string.
+   * This method is intended for use in integration tests to verify output.
+   * @returns {string} The terminal buffer content as a string
+   */
+  getBufferAsString () {
+    if (!this.terminal) {
+      return ''
+    }
+
+    const buffer = this.terminal.buffer.active
+    let text = ''
+    for (let i = 0; i < buffer.length; i++) {
+      const line = buffer.getLine(i)
+      if (line) {
+        text += line.translateToString(true) + '\n'
+      }
+    }
+    return text
+  }
 }

+ 15 - 2
frontend/resources/vue/views/ArgumentForm.vue

@@ -25,7 +25,9 @@
                 </option>
               </select>
               
-              <component v-else :is="getInputComponent(arg)" :id="arg.name" :name="arg.name" :value="getArgumentValue(arg)"
+              <component v-else :is="getInputComponent(arg)" :id="arg.name" :name="arg.name" 
+                :value="(arg.type === 'checkbox' || arg.type === 'confirmation') ? undefined : getArgumentValue(arg)"
+                :checked="(arg.type === 'checkbox' || arg.type === 'confirmation') ? getArgumentValue(arg) : undefined"
                 :list="arg.suggestions ? `${arg.name}-choices` : undefined" 
                 :type="getInputComponent(arg) !== 'select' ? getInputType(arg) : undefined"
                 :rows="arg.type === 'raw_string_multiline' ? 5 : undefined"
@@ -109,7 +111,18 @@ async function setup() {
       confirmationChecked.value = checkedValue
     } else {
       const paramValue = getQueryParamValue(arg.name)
-      argValues.value[arg.name] = paramValue !== null ? paramValue : arg.defaultValue || ''
+      if (arg.type === 'checkbox') {
+        // For checkboxes, handle boolean default values properly
+        if (paramValue !== null) {
+          argValues.value[arg.name] = paramValue === '1' || paramValue === 'true' || paramValue === true
+        } else if (arg.defaultValue !== undefined && arg.defaultValue !== '') {
+          argValues.value[arg.name] = arg.defaultValue === '1' || arg.defaultValue === 'true' || arg.defaultValue === true
+        } else {
+          argValues.value[arg.name] = false
+        }
+      } else {
+        argValues.value[arg.name] = paramValue !== null ? paramValue : arg.defaultValue || ''
+      }
     }
   })
 

+ 15 - 0
integration-tests/lib/elements.js

@@ -137,3 +137,18 @@ export async function getActionButton (webdriver, title) {
 
   return buttons[0]
 }
+
+export async function getTerminalBuffer() {
+  try {
+    const output = await webdriver.executeScript(`
+      if (window.terminal && window.terminal.getBufferAsString) {
+        return window.terminal.getBufferAsString();
+      }
+      return null;
+    `)
+    return output
+  } catch (e) {
+    console.log('[getTerminalBuffer] Error:', e.message)
+    return null
+  }
+}

+ 109 - 62
integration-tests/tests/checkbox/checkbox.mjs

@@ -5,8 +5,93 @@ import {
   getRootAndWait,
   getActionButton,
   takeScreenshotOnFailure,
+  getTerminalBuffer,
 } from '../../lib/elements.js'
 
+async function openCheckboxArgumentForm() {
+  await getRootAndWait()
+  const btn = await getActionButton(webdriver, 'Test checkbox argument')
+  await btn.click()
+
+  await webdriver.wait(
+    new Condition('wait for argument form page', async () => {
+      const url = await webdriver.getCurrentUrl()
+      return url.includes('/actionBinding/') && url.includes('/argumentForm')
+    }),
+    5000
+  )
+}
+
+async function getCheckboxInput() {
+  return await webdriver.findElement(By.id('confirm'))
+}
+
+async function submitCheckboxForm() {
+  const submitButton = await webdriver.findElement(By.css('button[name="start"]'))
+  await submitButton.click()
+}
+
+async function waitForLogsPage() {
+  await webdriver.wait(
+    new Condition('wait for logs page', async () => {
+      const url = await webdriver.getCurrentUrl()
+      return url.includes('/logs/') && !url.endsWith('/logs')
+    }),
+    5000
+  )
+}
+
+async function waitForExecutionComplete() {
+  await webdriver.wait(
+    new Condition('wait for execution status', async () => {
+      const statusElements = await webdriver.findElements(By.id('execution-dialog-status'))
+      return statusElements.length > 0
+    }),
+    5000
+  )
+
+  await webdriver.wait(
+    new Condition('wait for execution to finish', async () => {
+      try {
+        const statusElement = await webdriver.findElement(By.id('execution-dialog-status'))
+        const statusText = await statusElement.getText()
+        return !statusText.includes('Executing')
+      } catch (e) {
+        return false
+      }
+    }),
+    5000
+  )
+
+  // Small delay to allow terminal to write output
+  await webdriver.sleep(500)
+}
+
+async function waitForTerminalOutput(expectedValue) {
+  await webdriver.wait(
+    new Condition(`wait for checkbox value ${expectedValue} in output`, async () => {
+      try {
+        const terminalReady = await webdriver.executeScript(`
+          return !!(window.terminal && window.terminal.getBufferAsString);
+        `)
+        if (!terminalReady) {
+          return false
+        }
+        
+        const output = await getTerminalBuffer()
+        if (!output) {
+          return false
+        }
+        
+        return output.trim().includes(`Checkbox value: ${expectedValue}`)
+      } catch (e) {
+        return false
+      }
+    }),
+    5000
+  )
+}
+
 describe('config: checkbox', function () {
   before(async function () {
     await runner.start('checkbox')
@@ -21,82 +106,44 @@ describe('config: checkbox', function () {
   })
 
   it('Checkbox argument is rendered as a checkbox input', async function () {
-    await getRootAndWait()
-
-    const btn = await getActionButton(webdriver, 'Test checkbox argument')
-
-    await btn.click()
+    await openCheckboxArgumentForm()
 
-    // Wait for navigation to argument form page
-    await webdriver.wait(
-      new Condition('wait for argument form page', async () => {
-        const url = await webdriver.getCurrentUrl()
-        return url.includes('/actionBinding/') && url.includes('/argumentForm')
-      }),
-      8000
-    )
+    const checkboxInput = await getCheckboxInput()
 
-    // Find the checkbox input field
-    const checkboxInput = await webdriver.findElement(By.id('confirm'))
+    expect(await checkboxInput.getTagName()).to.equal('input')
+    expect(await checkboxInput.getAttribute('type')).to.equal('checkbox')
 
-    // Verify it's an input of type checkbox
-    const tagName = await checkboxInput.getTagName()
-    expect(tagName).to.equal('input')
-
-    const inputType = await checkboxInput.getAttribute('type')
-    expect(inputType).to.equal('checkbox')
-
-    // Verify the label is present
     const label = await webdriver.findElement(By.css('label[for="confirm"]'))
     expect(await label.getText()).to.contain('Confirm option')
   })
 
-  it('Checkbox argument can be toggled and submitted', async function () {
-    await getRootAndWait()
+  it('Checkbox argument submits 0 by default when unchecked', async function () {
+    this.timeout(15000)
+    await openCheckboxArgumentForm()
 
-    const btn = await getActionButton(webdriver, 'Test checkbox argument')
+    const checkboxInput = await getCheckboxInput()
+    expect(await checkboxInput.isSelected()).to.be.false
 
-    await btn.click()
-
-    // Wait for navigation to argument form page
-    await webdriver.wait(
-      new Condition('wait for argument form page', async () => {
-        const url = await webdriver.getCurrentUrl()
-        return url.includes('/actionBinding/') && url.includes('/argumentForm')
-      }),
-      8000
-    )
+    await submitCheckboxForm()
+    await waitForLogsPage()
+    await waitForExecutionComplete()
+    await waitForTerminalOutput('0')
+  })
 
-    const checkboxInput = await webdriver.findElement(By.id('confirm'))
+  it('Checkbox argument can be toggled and submitted', async function () {
+    this.timeout(15000)
+    await openCheckboxArgumentForm()
 
-    // Toggle the checkbox
+    const checkboxInput = await getCheckboxInput()
     await checkboxInput.click()
-
-    // Small wait for Vue to process the change
     await webdriver.sleep(100)
 
-    // Verify the checkbox is checked
-    const isChecked = await checkboxInput.isSelected()
-    expect(isChecked).to.be.true
-
-    // Find and click the submit button
-    const submitButton = await webdriver.findElement(
-      By.css('button[name="start"]')
-    )
-    await submitButton.click()
-
-    // Wait for navigation to logs page
-    await webdriver.wait(
-      new Condition('wait for logs page', async () => {
-        const url = await webdriver.getCurrentUrl()
-        return url.includes('/logs/')
-      }),
-      8000
-    )
-
-    // Verify we're on the logs page (action was executed)
-    const url = await webdriver.getCurrentUrl()
-    expect(url).to.include('/logs/')
+    expect(await checkboxInput.isSelected()).to.be.true
+
+    await submitCheckboxForm()
+    await waitForLogsPage()
+    await waitForExecutionComplete()
+    await waitForTerminalOutput('1')
   })
 })
 

+ 1 - 1
integration-tests/tests/checkbox/config.yaml

@@ -13,6 +13,6 @@ actions:
         title: Confirm option
         type: checkbox
         description: "When checked: 1, when unchecked: 0"
-        default: "false"
+        default: false
 
 

+ 92 - 0
service/internal/executor/arguments_test.go

@@ -6,6 +6,7 @@ import (
 
 	config "github.com/OliveTin/OliveTin/internal/config"
 	"github.com/OliveTin/OliveTin/internal/entities"
+	log "github.com/sirupsen/logrus"
 
 	"testing"
 
@@ -22,6 +23,97 @@ func TestSanitizeUnimplemented(t *testing.T) {
 	assert.NotNil(t, err, "Test an argument type that does not exist")
 }
 
+func TestValidateArgumentCheckboxDefaultValues(t *testing.T) {
+	arg := config.ActionArgument{
+		Name: "confirm",
+		Type: "checkbox",
+	}
+	action := config.Action{
+		Title: "Test checkbox default values",
+	}
+
+	// Default checkbox values without choices should accept "1" and "0"
+	err := ValidateArgument(&arg, "1", &action)
+	assert.Nil(t, err, "Expected checkbox value \"1\" to be accepted without choices")
+
+	err = ValidateArgument(&arg, "0", &action)
+	assert.Nil(t, err, "Expected checkbox value \"0\" to be accepted without choices")
+}
+
+func TestMangleCheckboxValueWithChoices(t *testing.T) {
+	log.SetLevel(log.PanicLevel)
+
+	arg := config.ActionArgument{
+		Name: "confirm",
+		Type: "checkbox",
+		Choices: []config.ActionArgumentChoice{
+			{Title: "Enabled", Value: "on"},
+			{Title: "Disabled", Value: "off"},
+		},
+	}
+
+	// When the incoming value matches a choice title, it should be mapped to the choice value
+	out := mangleCheckboxValue(&arg, "Enabled", "Test action")
+	assert.Equal(t, "on", out, "Expected checkbox title to be mangled to its value")
+
+	out = mangleCheckboxValue(&arg, "Disabled", "Test action")
+	assert.Equal(t, "off", out, "Expected checkbox title to be mangled to its value")
+
+	// When there is no matching title, the value should be returned unchanged
+	out = mangleCheckboxValue(&arg, "something-else", "Test action")
+	assert.Equal(t, "something-else", out, "Expected non-matching value to be returned unchanged")
+}
+
+func TestMangleArgumentValueCheckbox(t *testing.T) {
+	log.SetLevel(log.PanicLevel)
+
+	arg := config.ActionArgument{
+		Name: "confirm",
+		Type: "checkbox",
+		Choices: []config.ActionArgumentChoice{
+			{Title: "Yes", Value: "true-value"},
+			{Title: "No", Value: "false-value"},
+		},
+	}
+
+	out := MangleArgumentValue(&arg, "Yes", "Test action")
+	assert.Equal(t, "true-value", out, "Expected MangleArgumentValue to delegate to mangleCheckboxValue for checkbox types")
+
+	out = MangleArgumentValue(&arg, "No", "Test action")
+	assert.Equal(t, "false-value", out)
+
+	// For non-matching values, it should return the original value
+	out = MangleArgumentValue(&arg, "maybe", "Test action")
+	assert.Equal(t, "maybe", out)
+}
+
+func TestValidateArgumentCheckboxWithChoices(t *testing.T) {
+	log.SetLevel(log.PanicLevel)
+
+	arg := config.ActionArgument{
+		Name: "confirm",
+		Type: "checkbox",
+		Choices: []config.ActionArgumentChoice{
+			{Title: "Enabled", Value: "on"},
+			{Title: "Disabled", Value: "off"},
+		},
+	}
+	action := config.Action{
+		Title: "Test checkbox with choices",
+	}
+
+	// Titles should be accepted once mangled to their values
+	err := ValidateArgument(&arg, "Enabled", &action)
+	assert.Nil(t, err, "Expected checkbox title \"Enabled\" to be accepted after mangling to choice value")
+
+	err = ValidateArgument(&arg, "Disabled", &action)
+	assert.Nil(t, err, "Expected checkbox title \"Disabled\" to be accepted after mangling to choice value")
+
+	// Unknown titles should be rejected because they do not match any choice value
+	err = ValidateArgument(&arg, "Maybe", &action)
+	assert.NotNil(t, err, "Expected unknown checkbox title to be rejected against choices")
+}
+
 func TestArgumentValueNullable(t *testing.T) {
 	a1 := config.Action{
 		Title: "Release the hounds",