-
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
Prevent edits and deletions for row action automations #14218
Prevent edits and deletions for row action automations #14218
Conversation
const result = [] | ||
if (!isRowAction) { | ||
result.push( | ||
...[ |
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.
How come this spread is needed? Could you not ditch the array entirely and pass the objects in bare?
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.
Yes, that would be the same that 3 separate .push(object)
.
if ( | ||
automation.definition.trigger.stepId === AutomationTriggerStepId.ROW_ACTION | ||
) { | ||
ctx.throw("Row actions cannot be deleted", 403) |
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.
nit: https://www.rfc-editor.org/rfc/rfc9110#status.403 suggests the 403 status is to do with authorisation, I suspect 400 is slightly more accurate here because you're attempting to do something that's invalid regardless of who you are.
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.
I changed this 100 times and I did quite a lot of read. I agree that is not a 403, but, at the same time, it's not a 400. The request is not "bad" in terms of format.
I will change it again to be a 400, as 403 might have more implications and anyways this is something that should never be called
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.
Changed to 422, as it seems a good use case for it
Description
Prevent deleting and editing row actions via the frontend (and via the API)
Addresses
Screenshots