rafiki
rafiki copied to clipboard
refactor: made webhook event type an enum in GraphQL schema
Changes proposed in this pull request
- Added
WebhookEventTypescalar in GraphQL schema - Added parser to restrict values to be only the allowed types
- Added tests for the scalar methods
Context
Made WebhookEvent type a scalar instead of a string in GraphQL schema.
Webhook event types format (e.g. incoming_payment.created) are accepted. Given a string with the webhook event type, the mapper returns its corresponding WebhookEventType enum member, or throws an error if it doesn't exist.
Fixes #2786
Checklist
- [ ] Related issues linked using
fixes #number - [ ] Tests added/updated
- [ ] Make sure that all checks pass
- [ ] Bruno collection updated (if necessary)
- [ ] Documentation issue created with
user-docslabel (if necessary) - [ ] OpenAPI specs updated (if necessary)
Deploy Preview for brilliant-pasca-3e80ec canceled.
| Name | Link |
|---|---|
| Latest commit | 77535df84153d8b2f749738382b38019f7cc2fdb |
| Latest deploy log | https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/6710e3e240240300083496db |
@BlairCurrey would be good to get another perspective as well:
what do you think about the issue? I like having a proper enum to be used in the graphql query, but knowing that GQL unfortunately only supports upper snake case values like @oana-lolea mentioned, could it be confusing if people try to use the generated enum value when handing the webhook events in the webhook service, since it won't match up?
could it be confusing if people try to use the generated enum value when handing the webhook events in the webhook service, since it won't match up?
I think it would be problematic. We couldn't use them for identifying which type of webhook was received (ie receviedWebhookEvent.type === WebhookEventType.INCOMING_PAYMENT_CREATED). The client would need its own definition or map for the event types. And documentation-wise it's close but not fully correct which could be confusing.
What about a custom scalar?
const WebhookEventType = new GraphQLScalarType({
name: 'WebhookEventType',
description: 'Webhook event type format (e.g., incoming_payment.created)',
serialize(value) {
return value;
},
parseValue(value) {
const allowedTypes = [
'incoming_payment.created',
'outgoing_payment.completed',
// Add other valid event types
];
if (!allowedTypes.includes(value)) {
throw new Error('Invalid WebhookEventType');
}
return value;
}
});
I guess this might not be clear on the auto-generated documentation side though (it will just show WebhookEventType). But in terms of behavior of the GQL api it seems like what we want.
From the meeting:
Let's try to update the WebhookEventFilter to take in an array of WebhookEvent scalar values. If it breaks the frontend webhooks page out of the box, we can just close the issue with a plan to standardize all of our enums (like #3012) in the future.
Otherwise, we can keep the scalar.
@mkurapov The scalar cannot be used for the filter because it can only be used within the GraphQL schema. We could however use its parser to validate the filter string values.
@oana-lolea ah, I see. Thanks for taking a look into this. Since we can't use it as part of the filter (which would be useful for automatically validating user/client input), maybe we can just close this issue as not planned, and standardize all our enums separately in the future