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.

Sign up for GitHub

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

Row action name cleanup #14849

Merged
merged 14 commits into from
Oct 23, 2024
Merged

Row action name cleanup #14849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
713500f
Add sdk.automation.find
adrinr Oct 23, 2024
1f01277
Clean code
adrinr Oct 23, 2024
076f211
Remove unnecessary row action update endpoint
adrinr Oct 23, 2024
7781e75
Simplify row action permissions response
adrinr Oct 23, 2024
015854f
Dont persist row action name
adrinr Oct 23, 2024
dab3826
Allow renaming row actions
adrinr Oct 23, 2024
2868c84
Remove duplicated modals
adrinr Oct 23, 2024
022ec12
Clean
adrinr Oct 23, 2024
8f99072
Reuse
adrinr Oct 23, 2024
b4a979e
Fix get names
adrinr Oct 23, 2024
1fef783
Check names properly
adrinr Oct 23, 2024
732d701
Remove row action update tests
adrinr Oct 23, 2024
46a7f15
Fix name checks
adrinr Oct 23, 2024
63daa6b
Remove obsolete test
adrinr Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions packages/builder/src/components/automation/AutomationPanel/AutomationNavItem.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,13 @@
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
export let icon

let confirmDeleteDialog
let updateAutomationDialog
let updateRowActionDialog

$: isRowAction = sdk.automations.isRowAction(automation)

Expand Down Expand Up @@ -92,7 +90,7 @@
name: "Edit",
keyBind: null,
visible: true,
callback: updateRowActionDialog.show,
callback: updateAutomationDialog.show,
},
del,
]
Expand Down Expand Up @@ -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} />
83 changes: 0 additions & 83 deletions packages/builder/src/components/automation/AutomationPanel/UpdateRowActionModal.svelte
View file
Open in desktop

This file was deleted.

57 changes: 15 additions & 42 deletions packages/server/src/api/controllers/rowAction/crud.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import {
CreateRowActionRequest,
Ctx,
RowActionPermissions,
RowActionPermissionsResponse,
RowActionResponse,
RowActionsResponse,
UpdateRowActionRequest,
} from "@budibase/types"
import sdk from "../../../sdk"

Expand All @@ -30,14 +30,15 @@ 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]) => ({
...acc,
[key]: {
id: key,
tableId,
name: action.name,
name: automationNames[action.automationId],
automationId: action.automationId,
allowedSources: flattenAllowedSources(tableId, action.permissions),
},
Expand Down Expand Up @@ -68,26 +69,6 @@ export async function create(
ctx.status = 201
}

export async function update(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This logic has not been moved to the automation screen, so this is not needed anymore

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
Expand All @@ -96,38 +77,36 @@ 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,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This data is not actually required, and it would require fetching the automation names. Instead of adding the extra call, we can just simplify the responses

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

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
Expand All @@ -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
Expand All @@ -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),
}
}
Expand Down
6 changes: 0 additions & 6 deletions packages/server/src/api/routes/rowAction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
123 changes: 0 additions & 123 deletions packages/server/src/api/routes/tests/rowAction.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading
Loading