-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Row action name cleanup #14849
Row action name cleanup #14849
Conversation
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
@@ -68,26 +69,6 @@ export async function create( | |||
ctx.status = 201 | |||
} | |||
|
|||
export async function update( |
There was a problem hiding this comment.
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
const table = await getTable(ctx) | ||
const tableId = table._id! | ||
const { actionId } = ctx.params | ||
|
||
const action = await sdk.rowActions.setTablePermission(tableId, actionId) | ||
ctx.body = { | ||
tableId, |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Always happy to see simplification 👍
Description
On the initial design of row actions, the crud of these (specially editions and deletions) were done in the data section. With the latest designs, these are managed in the automation section, so there is quite a lot of code that can be simplified to achieve this.
Main changes:
Addresses