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

Allow for dynamic Cockpit Actions and add support for customizable HTTP request actions #1386

Conversation

rafaellehmkuhl
Copy link
Member

@rafaellehmkuhl rafaellehmkuhl commented Oct 3, 2024

Kapture.2024-10-03.at.14.56.15.mp4
image

Import can be tested with the following action files:
http-request-action (Arm with HTTP).json
http-request-action (Disarm with HTTP).json

Fix #568

@rafaellehmkuhl rafaellehmkuhl force-pushed the add-dynamic-http-request-actions branch from e515201 to 88cfb54 Compare October 3, 2024 18:05
@ES-Alexander
Copy link
Contributor

ES-Alexander commented Oct 3, 2024

  • File import/export would be very useful
  • Include a "test now" button in the Actions editor

@rafaellehmkuhl rafaellehmkuhl force-pushed the add-dynamic-http-request-actions branch 6 times, most recently from 04f6210 to 44cde4a Compare October 8, 2024 20:17
@rafaellehmkuhl
Copy link
Member Author

  • File import/export would be very useful
  • Include a "test now" button in the Actions editor

Added both:

image

Can be tested with the following files:

http-request-action (Arm with HTTP).json
http-request-action (Disarm with HTTP).json

@rafaellehmkuhl rafaellehmkuhl marked this pull request as ready for review October 8, 2024 20:20
@rafaellehmkuhl rafaellehmkuhl force-pushed the add-dynamic-http-request-actions branch 2 times, most recently from 7bc8f6a to 284e1ee Compare October 8, 2024 20:42
Copy link
Contributor

Choose a reason for hiding this comment

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

This bottom-divider on the expansion panel could be removed without problem.
If you remove it and set the bottom padding as below, the ExpansionPanel will get centered on the config window:
class="flex-col h-full overflow-y-auto ml-[5px] -mb-[10px] pr-3" :class="interfaceStore.isOnSmallScreen ? 'max-w-[80vw] max-h-[90vh]' : 'max-w-[680px] max-h-[85vh]'" > <ExpansiblePanel no-top-divider no-bottom-divider :is-expanded="!interfaceStore.isOnPhoneScreen">

image

image

Copy link
Contributor

Choose a reason for hiding this comment

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

To keep visual consistency, we should keep the dialog modals looking as close as possible to each other.
This visual consistency will be automated soon with BlueVue, but for now things must be manual.

Check this visual diff:
Before
image
After
image
This is a good reference for the dialog:
image

If you want a shortcut, this is how I implemented the example:

`<v-dialog` v-model="actionDialog.show" max-width="500px">
    <v-card class="rounded-lg" :style="interfaceStore.globalGlassMenuStyles">
      <v-card-title class="text-h6 font-weight-bold py-4 text-center">{{
        editMode ? 'Edit action' : 'Create new action'
      }}</v-card-title>
      <v-card-text class="px-8">
        <v-form class="d-flex flex-column gap-2" @submit.prevent="createActionConfig">
          <v-text-field
            v-model="newActionConfig.name"
            label="Action Name"
            required
            variant="outlined"
            density="compact"
          ></v-text-field>
          <v-select
            v-model="newActionConfig.method"
            :items="availableHttpRequestMethods"
            label="Request Type"
            required
            variant="outlined"
            density="compact"
          ></v-select>
          <v-text-field
            v-model="newActionConfig.url"
            label="URL"
            required
            variant="outlined"
            density="compact"
          ></v-text-field>

          <div class="d-flex align-center justify-space-between">
            <h3 class="text-subtitle-2 font-weight-bold">URL Parameters</h3>
            <v-btn variant="text" class="px-2 py-1" density="compact" @click="openUrlParamDialog">
              <v-icon size="small">mdi-plus</v-icon>
              Add
            </v-btn>
          </div>
          <div v-if="Object.keys(newActionConfig.urlParams).length > 0" class="mb-2">
            <v-chip-group>
              <v-chip
                v-for="(param, index) in Object.entries(newActionConfig.urlParams)"
                :key="`param-${index}`"
                closable
                size="x-small"
                class="ma-1"
                @click:close="removeUrlParam(param[0])"
              >
                {{ param[0] }}: {{ param[1] }}
              </v-chip>
            </v-chip-group>
          </div>

          <div class="d-flex align-center justify-space-between">
            <h3 class="text-subtitle-2 font-weight-bold">Headers</h3>
            <v-btn variant="text" class="px-2 py-1" density="compact" @click="openHeaderDialog">
              <v-icon size="small">mdi-plus</v-icon>
              Add
            </v-btn>
          </div>
          <div v-if="Object.keys(newActionConfig.headers).length > 0" class="mb-2">
            <v-chip-group>
              <v-chip
                v-for="(header, index) in Object.entries(newActionConfig.headers)"
                :key="`header-${index}`"
                closable
                size="x-small"
                class="ma-1"
                @click:close="removeHeader(header[0])"
              >
                {{ header[0] }}: {{ header[1] }}
              </v-chip>
            </v-chip-group>
          </div>

          <div class="d-flex align-center justify-space-between">
            <h3 class="text-subtitle-2 font-weight-bold">JSON Body</h3>
            <v-btn variant="text" class="px-2 py-1" density="compact" @click="openJsonDialog">
              <v-icon size="small">mdi-code-json</v-icon>
              Edit
            </v-btn>
          </div>
        </v-form>
      </v-card-text>
      <v-divider class="mt-2 mx-10" />
      <v-card-actions>
        <div class="flex justify-between items-center pa-2 w-full h-full">
          <v-btn color="white" variant="text" @click="closeActionDialog">Cancel</v-btn>
          <div class="flex gap-x-10">
            <v-btn variant="text" @click="resetNewAction">Reset</v-btn>
            <v-btn color="primary" :disabled="!isFormValid" variant="text" @click="saveActionConfig">
              {{ editMode ? 'Save' : 'Create' }}
            </v-btn>
          </div>
        </div>
      </v-card-actions>
    </v-card>
</v-dialog> `

Copy link
Contributor

Choose a reason for hiding this comment

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

On the edit JSON body template, there is a modal over another modal, and both are over a configuration floating screen; So, the elements piling up is kinda uncomfortable to the user.

For this case in specific, the solution can be widening the top config modal, so it will stay over the others. Check the result with the element already with the other modifications suggested:

image

image

Shortcut code:

<!-- JSON Body Dialog -->
  <v-dialog v-model="bodyDialog.show" max-width="520px">
    <v-card class="rounded-lg p-3" :style="interfaceStore.globalGlassMenuStyles">
      <v-card-title class="text-h6 font-weight-bold pa-2 text-center">Edit JSON body template</v-card-title>
      <v-card-text class="px-6">
        <v-form class="d-flex flex-column gap-2" @submit.prevent="saveJsonBody">
          <v-textarea
            v-model="bodyDialog.bodyText"
            label="JSON Body Template"
            hint="Use {{ anyInputName }} for dynamic values"
            persistent-hint
            :error-messages="bodyDialog.error"
            rows="12"
            variant="outlined"
            density="compact"
            @update:model-value="validateJsonTemplateForDialog"
          ></v-textarea>
        </v-form>
      </v-card-text>
      <v-divider class="ma-2" />
      <v-card-actions class="pa-2 -mb-1">
        <div class="flex w-full justify-between">
          <v-btn color="white" variant="text" @click="closeJsonDialog">Cancel</v-btn>
          <v-btn color="white" :disabled="!bodyDialog.isValid" @click="saveJsonBody">Save</v-btn>
        </div>
      </v-card-actions>
    </v-card>
  </v-dialog>

Copy link
Contributor

@ArturoManzoli ArturoManzoli Oct 9, 2024

Choose a reason for hiding this comment

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

For the 'Add header', the solution would be hiding the modal under it (opacity would do it), since it doesn't have enough content to grow vertically.

Also there are some modifications on the content to match the visual identity:

image

image
Code shortcut:

<!-- Header Dialog -->
  <v-dialog v-model="headerDialog.show" max-width="400px">
    <v-card class="rounded-lg p-3" :style="interfaceStore.globalGlassMenuStyles">
      <v-card-title class="text-h6 font-weight-bold pb-4 text-center">Add header</v-card-title>
      <v-card-text class="pa-4">
        <v-form class="d-flex flex-column gap-2" @submit.prevent="addHeader">
          <v-text-field
            v-model="headerDialog.key"
            label="Header Key"
            required
            variant="outlined"
            :error-messages="headerDialog.error"
            density="compact"
          ></v-text-field>
          <v-text-field
            v-model="headerDialog.value"
            label="Header Value"
            variant="outlined"
            density="compact"
          ></v-text-field>
        </v-form>
      </v-card-text>
      <v-divider class="ma-2" />
      <v-card-actions class="pa-2 -mb-1">
        <div class="flex w-full justify-between">
          <v-btn color="white" variant="text" size="small" @click="closeHeaderDialog">Cancel</v-btn>
          <v-btn color="white" size="small" @click="addHeader">Save</v-btn>
        </div>
      </v-card-actions>
    </v-card>
  </v-dialog>

Copy link
Contributor

Choose a reason for hiding this comment

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

For the 'Url parameters' The solution will have to be the same as before:

Check some mods needed:

image

image

Shortcut code:

<!-- URL Parameter Dialog -->
  <v-dialog v-model="urlParamDialog.show" max-width="400px">
    <v-card class="rounded-lg px-6 py-2" :style="interfaceStore.globalGlassMenuStyles">
      <v-card-title class="text-h6 text-center font-weight-bold pb-6">Add URL parameter</v-card-title>
      <v-card-text class="pa-2">
        <v-form class="d-flex flex-column gap-2" @submit.prevent="addUrlParameter">
          <v-text-field
            v-model="urlParamDialog.key"
            label="Parameter Key"
            required
            variant="outlined"
            density="compact"
          ></v-text-field>
          <v-select
            v-model="urlParamDialog.valueType"
            :items="paramValueOptions"
            label="Parameter Value"
            required
            variant="outlined"
            density="compact"
          ></v-select>
          <v-text-field
            v-if="urlParamDialog.valueType === 'hardcoded'"
            v-model="urlParamDialog.hardcodedValue"
            label="Hardcoded Value"
            required
            variant="outlined"
            density="compact"
          ></v-text-field>
        </v-form>
      </v-card-text>
      <v-divider class="ma-2" />
      <v-card-actions class="pa-2 -mb-1">
        <div class="flex w-full justify-between">
          <v-btn color="white" variant="text" size="small" @click="closeUrlParamDialog">Cancel</v-btn>
          <v-btn color="white" size="small" @click="saveUrlParameter()"> Save </v-btn>
        </div>
      </v-card-actions>
    </v-card>
  </v-dialog>

@ArturoManzoli
Copy link
Contributor

We indeed were using 'Title Case' on the titles for the windows, modals and expansible panels, but after some feedback we decided to keep everything on regular capitalization:

Check the Alerts config page:
image

image

Comment on lines 26 to 39
export const getCockpitActionParametersInfo = (id: string): CockpitActionParameter | undefined => {
return cockpitActionParametersInfo[id]
}

export const getAllCockpitActionParametersInfo = (): Record<string, CockpitActionParameter> => {
return cockpitActionParametersInfo
}

export const getCockpitActionParameterInfo = (id: string): CockpitActionParameter | undefined => {
return cockpitActionParametersInfo[id]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't these two functions (getCockpitActionParametersInfo and getCockpitActionParameterInfo) duplicated?

Copy link
Member Author

Choose a reason for hiding this comment

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

They definitely are 😆

Comment on lines 113 to 157
export const getHttpRequestActionCallback = (id: string): HttpRequestActionCallback => {
const action = getHttpRequestActionConfig(id)
if (!action) {
throw new Error(`Action with id ${id} not found.`)
}

let parsedBody = action.body
const parsedUrlParams = action.urlParams

const cockpitInputsInBody = action.body.match(/{{\s*([^{}\s]+)\s*}}/g)
if (cockpitInputsInBody) {
for (const input of cockpitInputsInBody) {
const parsedInput = input.replace('{{', '').replace('}}', '').trim()
const inputData = getCockpitActionParameterData(parsedInput)
if (inputData) {
parsedBody = parsedBody.replace(input, inputData.toString())
}
}
}

const cockpitInputsInUrlParams = Object.entries(action.urlParams).filter(
([, value]) => typeof value === 'string' && value.startsWith('{{') && value.endsWith('}}')
)
if (cockpitInputsInUrlParams) {
for (const [key, value] of cockpitInputsInUrlParams) {
const parsedInput = value.replace('{{', '').replace('}}', '').trim()
const inputData = getCockpitActionParameterData(parsedInput)
if (inputData) {
parsedUrlParams[key] = inputData.toString()
}
}
}

const url = new URL(action.url)

url.search = new URLSearchParams(parsedUrlParams).toString()

return () => {
fetch(url, {
method: action.method,
headers: action.headers,
body: action.method === HttpRequestMethod.GET ? undefined : parsedBody,
})
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The parameters for the body and url variables are registered when the callback is created, but if they change by the time its executed, the new values won't be updated.

Maybe moving them inside the return, like:

export const getHttpRequestActionCallback = (id: string): HttpRequestActionCallback => {
  const action = getHttpRequestActionConfig(id)
  if (!action) {
    throw new Error(`Action with id ${id} not found.`)
  }

  return () => {
    let parsedBody = action.body
    const parsedUrlParams = { ...action.urlParams }

 [...]

    fetch(url, {
      method: action.method,
      headers: action.headers,
      body: action.method === HttpRequestMethod.GET ? undefined : parsedBody,
    })
  }
}

@ArturoManzoli
Copy link
Contributor

ArturoManzoli commented Oct 9, 2024

Missing some tooltips for the action buttons and some margin on the table header:

image

Margins:
image

@ArturoManzoli
Copy link
Contributor

I was able to load some random .json files into the Actions module and they broke cockpit (obviously.. haha)
What are your thoughts on adding a header to the files exported from the "Cockpit actions configuration" table? Would it be necessary or just overkill for the current needs?

image

@rafaellehmkuhl
Copy link
Member Author

I was able to load some random .json files into the Actions module and they broke cockpit (obviously.. haha) What are your thoughts on adding a header to the files exported from the "Cockpit actions configuration" table? Would it be necessary or just overkill for the current needs?

I think we can add a simple validator to check at least if the json structure is the same.

@ArturoManzoli
Copy link
Contributor

Export icons can be centered inside its container using:

<v-btn
    variant="outlined"
    class="rounded-full mx-1 pl-[3px] pt-[1px]"
    icon="mdi-export"
    size="x-small"
    @click="exportAction(item.id)"
/>

image

Copy link
Contributor

@ArturoManzoli ArturoManzoli left a comment

Choose a reason for hiding this comment

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

Architecture and implementation are awesome!

Just some minor changes as commented earlier.

@rafaellehmkuhl rafaellehmkuhl force-pushed the add-dynamic-http-request-actions branch from 284e1ee to a6fa73d Compare October 14, 2024 14:25
@rafaellehmkuhl
Copy link
Member Author

@ArturoManzoli all UI changes were performed. I've also added file validation for the "Import action" button and changed the data-lake structures to "CockpitActionVariable" (former "CockpitActionParameter").

image

@ArturoManzoli ArturoManzoli merged commit c2354d5 into bluerobotics:master Oct 15, 2024
10 checks passed
@rafaellehmkuhl rafaellehmkuhl deleted the add-dynamic-http-request-actions branch October 15, 2024 11:01
@ES-Alexander ES-Alexander added the docs-needed Change needs to be documented label Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-needed Change needs to be documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow configuration of generic HTTP request to be made from Cockpit Actions
3 participants