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

Row action name cleanup #14849

Merged

Conversation

adrinr
Copy link
Collaborator

@adrinr adrinr commented Oct 23, 2024

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:

  • Remove the persisted row action name, as it should use the automation one. This is only used in the builder, so we don't need to worry for the extra db check.
  • Remove the row action update endpoint, as this is just renaming the actual action
  • Simplify some API responses
  • Remove the update row action frontend modal, as it's doing the same than the automation modal

Addresses

Copy link

qa-wolf bot commented Oct 23, 2024

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

@github-actions github-actions bot added firestorm Data/Infra/Revenue Team size/m labels Oct 23, 2024
@@ -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

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

@adrinr adrinr marked this pull request as ready for review October 23, 2024 12:14
@adrinr adrinr requested a review from a team as a code owner October 23, 2024 12:14
@adrinr adrinr requested review from samwho, mike12345567, aptkingston and a team and removed request for a team October 23, 2024 12:14
@adrinr adrinr marked this pull request as draft October 23, 2024 12:15
@adrinr adrinr changed the title Budi 8770/row action automation names are not updated in automation Row action name cleanup Oct 23, 2024
@adrinr adrinr marked this pull request as ready for review October 23, 2024 12:25
Copy link
Member

@aptkingston aptkingston left a 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 👍

@adrinr adrinr merged commit 64db4db into v3-ui Oct 23, 2024
11 checks passed
@adrinr adrinr deleted the BUDI-8770/row-action-automation-names-are-not-updated-in-automation branch October 23, 2024 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
firestorm Data/Infra/Revenue Team size/m
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants