Merge pull request #14849 from Budibase/BUDI-8770/row-action-automation-names-are-not-updated-in-automation

Row action name cleanup
This commit is contained in:
Adria Navarro 2024-10-23 15:06:00 +02:00 committed by GitHub
commit 64db4db3b3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 67 additions and 367 deletions

View File

@ -9,7 +9,6 @@
import { sdk } from "@budibase/shared-core"
import ConfirmDialog from "components/common/ConfirmDialog.svelte"
import UpdateAutomationModal from "components/automation/AutomationPanel/UpdateAutomationModal.svelte"
import UpdateRowActionModal from "components/automation/AutomationPanel/UpdateRowActionModal.svelte"
import NavItem from "components/common/NavItem.svelte"
export let automation
@ -17,7 +16,6 @@
let confirmDeleteDialog
let updateAutomationDialog
let updateRowActionDialog
$: isRowAction = sdk.automations.isRowAction(automation)
@ -92,7 +90,7 @@
name: "Edit",
keyBind: null,
visible: true,
callback: updateRowActionDialog.show,
callback: updateAutomationDialog.show,
},
del,
]
@ -135,8 +133,4 @@
This action cannot be undone.
</ConfirmDialog>
{#if isRowAction}
<UpdateRowActionModal {automation} bind:this={updateRowActionDialog} />
{:else}
<UpdateAutomationModal {automation} bind:this={updateAutomationDialog} />
{/if}
<UpdateAutomationModal {automation} bind:this={updateAutomationDialog} />

View File

@ -1,83 +0,0 @@
<script>
import { rowActions } from "stores/builder"
import {
notifications,
Icon,
Input,
ModalContent,
Modal,
} from "@budibase/bbui"
export let automation
export let onCancel = undefined
let name
let error = ""
let modal
export const show = () => {
name = automation?.displayName
modal.show()
}
export const hide = () => {
modal.hide()
}
async function saveAutomation() {
try {
await rowActions.rename(
automation.definition.trigger.inputs.tableId,
automation.definition.trigger.inputs.rowActionId,
name
)
notifications.success(`Row action updated successfully`)
hide()
} catch (error) {
notifications.error("Error saving row action")
}
}
function checkValid(evt) {
name = evt.target.value
if (!name) {
error = "Name is required"
return
}
error = ""
}
</script>
<Modal bind:this={modal} on:hide={onCancel}>
<ModalContent
title="Edit Row Action"
confirmText="Save"
size="L"
onConfirm={saveAutomation}
disabled={error}
>
<Input bind:value={name} label="Name" on:input={checkValid} {error} />
<a
slot="footer"
target="_blank"
href="https://docs.budibase.com/docs/automation-steps"
>
<Icon name="InfoOutline" />
<span>Learn about automations</span>
</a>
</ModalContent>
</Modal>
<style>
a {
color: var(--ink);
font-size: 14px;
vertical-align: middle;
display: flex;
align-items: center;
text-decoration: none;
}
a span {
text-decoration: underline;
margin-left: var(--spectrum-alias-item-padding-s);
}
</style>

View File

@ -2,9 +2,9 @@ import {
CreateRowActionRequest,
Ctx,
RowActionPermissions,
RowActionPermissionsResponse,
RowActionResponse,
RowActionsResponse,
UpdateRowActionRequest,
} from "@budibase/types"
import sdk from "../../../sdk"
@ -30,6 +30,7 @@ export async function find(ctx: Ctx<void, RowActionsResponse>) {
}
const { actions } = rowActions
const automationNames = await sdk.rowActions.getNames(rowActions)
const result: RowActionsResponse = {
actions: Object.entries(actions).reduce<Record<string, RowActionResponse>>(
(acc, [key, action]) => ({
@ -37,7 +38,7 @@ export async function find(ctx: Ctx<void, RowActionsResponse>) {
[key]: {
id: key,
tableId,
name: action.name,
name: automationNames[action.automationId],
automationId: action.automationId,
allowedSources: flattenAllowedSources(tableId, action.permissions),
},
@ -68,26 +69,6 @@ export async function create(
ctx.status = 201
}
export async function update(
ctx: Ctx<UpdateRowActionRequest, RowActionResponse>
) {
const table = await getTable(ctx)
const tableId = table._id!
const { actionId } = ctx.params
const action = await sdk.rowActions.update(tableId, actionId, {
name: ctx.request.body.name,
})
ctx.body = {
tableId,
id: action.id,
name: action.name,
automationId: action.automationId,
allowedSources: flattenAllowedSources(tableId, action.permissions),
}
}
export async function remove(ctx: Ctx<void, void>) {
const table = await getTable(ctx)
const { actionId } = ctx.params
@ -96,22 +77,22 @@ export async function remove(ctx: Ctx<void, void>) {
ctx.status = 204
}
export async function setTablePermission(ctx: Ctx<void, RowActionResponse>) {
export async function setTablePermission(
ctx: Ctx<void, RowActionPermissionsResponse>
) {
const table = await getTable(ctx)
const tableId = table._id!
const { actionId } = ctx.params
const action = await sdk.rowActions.setTablePermission(tableId, actionId)
ctx.body = {
tableId,
id: action.id,
name: action.name,
automationId: action.automationId,
allowedSources: flattenAllowedSources(tableId, action.permissions),
}
}
export async function unsetTablePermission(ctx: Ctx<void, RowActionResponse>) {
export async function unsetTablePermission(
ctx: Ctx<void, RowActionPermissionsResponse>
) {
const table = await getTable(ctx)
const tableId = table._id!
const { actionId } = ctx.params
@ -119,15 +100,13 @@ export async function unsetTablePermission(ctx: Ctx<void, RowActionResponse>) {
const action = await sdk.rowActions.unsetTablePermission(tableId, actionId)
ctx.body = {
tableId,
id: action.id,
name: action.name,
automationId: action.automationId,
allowedSources: flattenAllowedSources(tableId, action.permissions),
}
}
export async function setViewPermission(ctx: Ctx<void, RowActionResponse>) {
export async function setViewPermission(
ctx: Ctx<void, RowActionPermissionsResponse>
) {
const table = await getTable(ctx)
const tableId = table._id!
const { actionId, viewId } = ctx.params
@ -138,15 +117,13 @@ export async function setViewPermission(ctx: Ctx<void, RowActionResponse>) {
viewId
)
ctx.body = {
tableId,
id: action.id,
name: action.name,
automationId: action.automationId,
allowedSources: flattenAllowedSources(tableId, action.permissions),
}
}
export async function unsetViewPermission(ctx: Ctx<void, RowActionResponse>) {
export async function unsetViewPermission(
ctx: Ctx<void, RowActionPermissionsResponse>
) {
const table = await getTable(ctx)
const tableId = table._id!
const { actionId, viewId } = ctx.params
@ -158,10 +135,6 @@ export async function unsetViewPermission(ctx: Ctx<void, RowActionResponse>) {
)
ctx.body = {
tableId,
id: action.id,
name: action.name,
automationId: action.automationId,
allowedSources: flattenAllowedSources(tableId, action.permissions),
}
}

View File

@ -40,12 +40,6 @@ router
rowActionValidator(),
rowActionController.create
)
.put(
"/api/tables/:tableId/actions/:actionId",
authorized(BUILDER),
rowActionValidator(),
rowActionController.update
)
.delete(
"/api/tables/:tableId/actions/:actionId",
authorized(BUILDER),

View File

@ -328,129 +328,6 @@ describe("/rowsActions", () => {
})
})
describe("update", () => {
unauthorisedTests((expectations, testConfig) =>
config.api.rowAction.update(
tableId,
generator.guid(),
createRowActionRequest(),
expectations,
testConfig
)
)
it("can update existing actions", async () => {
for (const rowAction of createRowActionRequests(3)) {
await createRowAction(tableId, rowAction)
}
const persisted = await config.api.rowAction.find(tableId)
const [actionId, actionData] = _.sample(
Object.entries(persisted.actions)
)!
const updatedName = generator.string()
const res = await config.api.rowAction.update(tableId, actionId, {
name: updatedName,
})
expect(res).toEqual({
id: actionId,
tableId,
name: updatedName,
automationId: actionData.automationId,
allowedSources: [tableId],
})
expect(await config.api.rowAction.find(tableId)).toEqual(
expect.objectContaining({
actions: expect.objectContaining({
[actionId]: {
name: updatedName,
id: actionData.id,
tableId: actionData.tableId,
automationId: actionData.automationId,
allowedSources: [tableId],
},
}),
})
)
})
it("trims row action names", async () => {
const rowAction = await createRowAction(tableId, createRowActionRequest())
const res = await config.api.rowAction.update(tableId, rowAction.id, {
name: " action name ",
})
expect(res).toEqual(expect.objectContaining({ name: "action name" }))
expect(await config.api.rowAction.find(tableId)).toEqual(
expect.objectContaining({
actions: expect.objectContaining({
[rowAction.id]: expect.objectContaining({
name: "action name",
}),
}),
})
)
})
it("throws Bad Request when trying to update by a non-existing id", async () => {
await createRowAction(tableId, createRowActionRequest())
await config.api.rowAction.update(
tableId,
generator.guid(),
createRowActionRequest(),
{ status: 400 }
)
})
it("throws Bad Request when trying to update by a via another table id", async () => {
const otherTable = await config.api.table.save(
setup.structures.basicTable()
)
await createRowAction(otherTable._id!, createRowActionRequest())
const action = await createRowAction(tableId, createRowActionRequest())
await config.api.rowAction.update(
otherTable._id!,
action.id,
createRowActionRequest(),
{ status: 400 }
)
})
it("can not use existing row action names (for the same table)", async () => {
const action1 = await createRowAction(tableId, createRowActionRequest())
const action2 = await createRowAction(tableId, createRowActionRequest())
await config.api.rowAction.update(
tableId,
action1.id,
{ name: action2.name },
{
status: 409,
body: {
message: "A row action with the same name already exists.",
},
}
)
})
it("does not throw with name conflicts for the same row action", async () => {
const action1 = await createRowAction(tableId, createRowActionRequest())
await config.api.rowAction.update(tableId, action1.id, {
name: action1.name,
})
})
})
describe("delete", () => {
unauthorisedTests((expectations, testConfig) =>
config.api.rowAction.delete(

View File

@ -1,4 +1,3 @@
import { sdk } from "@budibase/shared-core"
import {
Automation,
RequiredKeys,
@ -99,6 +98,12 @@ export async function get(automationId: string) {
return trimUnexpectedObjectFields(result)
}
export async function find(ids: string[]) {
const db = getDb()
const result = await db.getMultiple<PersistedAutomation>(ids)
return result.map(trimUnexpectedObjectFields)
}
export async function create(automation: Automation) {
automation = trimUnexpectedObjectFields(automation)
const db = getDb()
@ -289,13 +294,6 @@ function guardInvalidUpdatesAndThrow(
}
})
}
if (
sdk.automations.isRowAction(automation) &&
automation.name !== oldAutomation.name
) {
throw new Error("Row actions cannot be renamed")
}
}
function trimUnexpectedObjectFields<T extends Automation>(automation: T): T {

View File

@ -1,5 +1,5 @@
import { sample } from "lodash/fp"
import { Automation, AutomationTriggerStepId } from "@budibase/types"
import { Automation } from "@budibase/types"
import { generator } from "@budibase/backend-core/tests"
import TestConfiguration from "../../../../tests/utilities/TestConfiguration"
import automationSdk from "../"
@ -26,25 +26,6 @@ describe("automation sdk", () => {
})
})
it("cannot rename row action automations", async () => {
await config.doInContext(config.getAppId(), async () => {
const automation = structures.newAutomation({
trigger: {
...structures.automationTrigger(),
stepId: AutomationTriggerStepId.ROW_ACTION,
},
})
const response = await automationSdk.create(automation)
const newName = generator.guid()
const update = { ...response, name: newName }
await expect(automationSdk.update(update)).rejects.toThrow(
"Row actions cannot be renamed"
)
})
})
it.each([
["trigger", (a: Automation) => a.definition.trigger],
["step", (a: Automation) => a.definition.steps[0]],

View File

@ -2,7 +2,6 @@ import {
Automation,
AutomationActionStepId,
AutomationBuilderData,
TableRowActions,
} from "@budibase/types"
import { sdk as coreSdk } from "@budibase/shared-core"
import sdk from "../../../sdk"
@ -26,15 +25,6 @@ export async function getBuilderData(
return tableNameCache[tableId]
}
const rowActionNameCache: Record<string, TableRowActions | undefined> = {}
async function getRowActionName(tableId: string, rowActionId: string) {
if (!rowActionNameCache[tableId]) {
rowActionNameCache[tableId] = await sdk.rowActions.getAll(tableId)
}
return rowActionNameCache[tableId]?.actions[rowActionId]?.name
}
const result: Record<string, AutomationBuilderData> = {}
for (const automation of automations) {
const isRowAction = coreSdk.automations.isRowAction(automation)
@ -49,12 +39,7 @@ export async function getBuilderData(
}
const tableName = await getTableName(tableId)
const rowActionName = await getRowActionName(tableId, rowActionId)
if (!rowActionName) {
throw new Error(`Row action not found: ${rowActionId}`)
}
const rowActionName = automation.name
result[automation._id!] = {
displayName: rowActionName,
triggerInfo: {

View File

@ -13,16 +13,19 @@ import { definitions as TRIGGER_DEFINITIONS } from "../../automations/triggerInf
import * as triggers from "../../automations/triggers"
import sdk from ".."
function ensureUniqueAndThrow(
async function ensureUniqueAndThrow(
doc: TableRowActions,
name: string,
existingRowActionId?: string
) {
const names = await getNames(doc)
name = name.toLowerCase().trim()
if (
Object.entries(doc.actions).find(
([id, a]) =>
a.name.toLowerCase() === name.toLowerCase() &&
id !== existingRowActionId
Object.entries(names).find(
([automationId, automationName]) =>
automationName.toLowerCase().trim() === name &&
automationId !== existingRowActionId
)
) {
throw new HTTPError("A row action with the same name already exists.", 409)
@ -34,18 +37,12 @@ export async function create(tableId: string, rowAction: { name: string }) {
const db = context.getAppDB()
const rowActionsId = generateRowActionsID(tableId)
let doc: TableRowActions
try {
doc = await db.get<TableRowActions>(rowActionsId)
} catch (e: any) {
if (e.status !== 404) {
throw e
}
let doc = await db.tryGet<TableRowActions>(rowActionsId)
if (!doc) {
doc = { _id: rowActionsId, actions: {} }
}
ensureUniqueAndThrow(doc, action.name)
await ensureUniqueAndThrow(doc, action.name)
const appId = context.getAppId()
if (!appId) {
@ -74,7 +71,6 @@ export async function create(tableId: string, rowAction: { name: string }) {
})
doc.actions[newRowActionId] = {
name: action.name,
automationId: automation._id!,
permissions: {
table: { runAllowed: true },
@ -85,6 +81,7 @@ export async function create(tableId: string, rowAction: { name: string }) {
return {
id: newRowActionId,
name: automation.name,
...doc.actions[newRowActionId],
}
}
@ -159,20 +156,6 @@ async function updateDoc(
}
}
export async function update(
tableId: string,
rowActionId: string,
rowActionData: { name: string }
) {
const newName = rowActionData.name.trim()
return await updateDoc(tableId, rowActionId, actionsDoc => {
ensureUniqueAndThrow(actionsDoc, newName, rowActionId)
actionsDoc.actions[rowActionId].name = newName
return actionsDoc
})
}
async function guardView(tableId: string, viewId: string) {
let view
if (docIds.isViewId(viewId)) {
@ -248,13 +231,8 @@ export async function run(
throw new HTTPError("Table not found", 404)
}
const rowActions = await getAll(tableId)
const rowAction = rowActions?.actions[rowActionId]
if (!rowAction) {
throw new HTTPError("Row action not found", 404)
}
const automation = await sdk.automations.get(rowAction.automationId)
const { automationId } = await get(tableId, rowActionId)
const automation = await sdk.automations.get(automationId)
const row = await sdk.rows.find(tableId, rowId)
await triggers.externalTrigger(
@ -272,3 +250,17 @@ export async function run(
{ getResponses: true }
)
}
export async function getNames({ actions }: TableRowActions) {
const automations = await sdk.automations.find(
Object.values(actions).map(({ automationId }) => automationId)
)
const automationNames = automations.reduce<Record<string, string>>(
(names, a) => {
names[a._id] = a.name
return names
},
{}
)
return automationNames
}

View File

@ -1,5 +1,6 @@
import {
CreateRowActionRequest,
RowActionPermissionsResponse,
RowActionResponse,
RowActionsResponse,
RowActionTriggerRequest,
@ -40,23 +41,6 @@ export class RowActionAPI extends TestAPI {
)
}
update = async (
tableId: string,
rowActionId: string,
rowAction: CreateRowActionRequest,
expectations?: Expectations,
config?: { publicUser?: boolean }
) => {
return await this._put<RowActionResponse>(
`/api/tables/${tableId}/actions/${rowActionId}`,
{
body: rowAction,
expectations,
...config,
}
)
}
delete = async (
tableId: string,
rowActionId: string,
@ -78,7 +62,7 @@ export class RowActionAPI extends TestAPI {
expectations?: Expectations,
config?: { publicUser?: boolean }
) => {
return await this._post<RowActionResponse>(
return await this._post<RowActionPermissionsResponse>(
`/api/tables/${tableId}/actions/${rowActionId}/permissions`,
{
expectations: {
@ -96,7 +80,7 @@ export class RowActionAPI extends TestAPI {
expectations?: Expectations,
config?: { publicUser?: boolean }
) => {
return await this._delete<RowActionResponse>(
return await this._delete<RowActionPermissionsResponse>(
`/api/tables/${tableId}/actions/${rowActionId}/permissions`,
{
expectations: {
@ -115,7 +99,7 @@ export class RowActionAPI extends TestAPI {
expectations?: Expectations,
config?: { publicUser?: boolean }
) => {
return await this._post<RowActionResponse>(
return await this._post<RowActionPermissionsResponse>(
`/api/tables/${tableId}/actions/${rowActionId}/permissions/${viewId}`,
{
expectations: {
@ -134,7 +118,7 @@ export class RowActionAPI extends TestAPI {
expectations?: Expectations,
config?: { publicUser?: boolean }
) => {
return await this._delete<RowActionResponse>(
return await this._delete<RowActionPermissionsResponse>(
`/api/tables/${tableId}/actions/${rowActionId}/permissions/${viewId}`,
{
expectations: {

View File

@ -1,14 +1,17 @@
interface RowActionData {
name: string
}
interface RowActionPermissionsData {
allowedSources: string[] | undefined
}
export interface CreateRowActionRequest extends RowActionData {}
export interface UpdateRowActionRequest extends RowActionData {}
export interface RowActionResponse extends RowActionData {
export interface RowActionResponse
extends RowActionData,
RowActionPermissionsData {
id: string
tableId: string
automationId: string
allowedSources: string[] | undefined
}
export interface RowActionsResponse {
@ -18,3 +21,6 @@ export interface RowActionsResponse {
export interface RowActionTriggerRequest {
rowId: string
}
export interface RowActionPermissionsResponse
extends RowActionPermissionsData {}

View File

@ -6,7 +6,6 @@ export interface TableRowActions extends Document {
}
export interface RowActionData {
name: string
automationId: string
permissions: RowActionPermissions
}