TAN-5687 Webhooks #12365
TAN-5687 Webhooks #12365
Conversation
|
…nlab into TAN-5687-webhooks
| const methods = useForm({ | ||
| mode: 'onBlur', | ||
| resolver: yupResolver(schema), | ||
| defaultValues: { |
There was a problem hiding this comment.
No need to add empty default values, this can be deleted.
|
|
||
| return ( | ||
| <Box w="100%" m="24px auto" pr="24px"> | ||
| {!success ? ( |
There was a problem hiding this comment.
Instead of having a separate state for success, you can use the form state to determine if it has been submitted via methods.formState.isSubmitSuccessful
| project_id?: string | null; | ||
| } | ||
|
|
||
| export const WEBHOOK_EVENTS = [ |
There was a problem hiding this comment.
This is good but I think putting those directly into the dropdown might be confusing as survey submission or proposal is also an idea. Maybe we need better framing?
There was a problem hiding this comment.
It's important that these strings exactly match what is sent to the receiving end of the webhook, so they should be literal. But I should indeed link to the documentation (for which I forgot to ask your review, did just now), will add.
There was a problem hiding this comment.
I wasn't suggesting to change what is sent to the API, just that keeping these as they are in the dropdown itself could be confusing and that maybe we should use human language there like "Input (or post) created" or something similar.
| @@ -0,0 +1,153 @@ | |||
| # Changes Made to Implementation Plan | |||
There was a problem hiding this comment.
Was it intended that everything in this folder was committed? Maybe it was part of the working process and we should leave it out? Or is all of the information in here valuable enough to remain in the codebase?
There was a problem hiding this comment.
I don't know, it's a discussion we should have, I think. I think it's useful information to understand the feature, but we should align on how and where to incorporate such information. For now I'll keep it in for the sake of our discussion (next week)
|
|
||
| def perform | ||
| # Delete successful deliveries older than 30 days | ||
| deleted_success = Webhooks::Delivery.succeeded |
There was a problem hiding this comment.
Curious why we don't keep deliveries for more than 30 days?
There was a problem hiding this comment.
Just because it potentially bloats the database. I think we'll start without and not define the rake task yet, but suspect we might want to start doing it at some point.
Changelog
Added
Changed