Repository navigation
feat(coupons): add lifecycle webhooks - #6306
avanish-garg wants to merge 5 commits into
Conversation
Welcome, @avanish-garg!Thanks for your first contribution! Before we proceed with the review, please sign the Fiduciary License Agreement: Once signed, this PR will be automatically updated. |
Thanks, @avanish-garg! 🎉Your CLA has been signed and is now on file. We'll proceed with the review shortly. |
groyoh
left a comment
groyoh
left a comment
There was a problem hiding this comment.
Some points may block the merge: the
- The branch is based on a
mainfrom early September and needs a rebase.spec/models/webhook_endpoint_spec.rbwas added onmainsince then and fails with this change (see the inline comment). - A coupon also changes state when it is terminated, through the
terminateCouponGraphQL mutation or the dailyClock::TerminateCouponsJobfor expired coupons (Coupons::TerminateService). The payload exposesterminated_at, yet no webhook is sent for that change, so a consumer that mirrors coupons from these webhooks keeps them active. Sendingcoupon.updatedfromCoupons::TerminateService(both#calland.terminate_all_expired) would cover it. - The new webhooks are not documented in getlago/lago-openapi (
src/webhooks/hasbillable_metric_created.yamlbut nothing for coupons). A matching PR there would let SDK users and the docs see the payload.
| "coupon.created" => Webhooks::Coupons::CreatedService, | ||
| "coupon.updated" => Webhooks::Coupons::UpdatedService, | ||
| "coupon.deleted" => Webhooks::Coupons::DeletedService, |
There was a problem hiding this comment.
coupon.* types are not declared in config/webhook_event_types.yml. WebhookEndpoint#validate_event_types rejects any type missing from that file, so an endpoint that selects its events cannot subscribe to them, and the eventTypes GraphQL query behind the webhook form does not list them. Only endpoints that listen to all events receive them.
On current main, spec/models/webhook_endpoint_spec.rb:163 checks that every key of WEBHOOK_SERVICES is declared in that file. With this branch applied on main, it fails:
Expected ["coupon.created", "coupon.updated", "coupon.deleted"] to return a truthy result for empty?
Entries like these would fix it (after a rebase on main):
coupon_created:
name: coupon.created
description: A new coupon has been created
category: Coupons
deprecated: false
coupon_updated:
name: coupon.updated
description: A coupon has been updated
category: Coupons
deprecated: false
coupon_deleted:
name: coupon.deleted
description: A coupon has been deleted
category: Coupons
deprecated: falseWebhookEventTypeEnum and WebhookEventCategoryEnum are built from this file, so schema.graphql and schema.json also need a bin/rails graphql:schema:dump.
There was a problem hiding this comment.
Fixed in f44fee475ccbc73437bee6b2fa45b7fe6ff31e30 after a rebase on main. The coupon types are declared under a Coupons category, and schema.graphql and schema.json were dumped again.
coupon.created/updated/deleted webhook event names already existed
(added for activity logs), but nothing ever fired them -- sibling
catalog resources Plan/BillableMetric/Feature all fire webhooks on
create/update/destroy, Coupon didn't.
Adds Webhooks::Coupons::{Created,Updated,Deleted}Service mirroring
the existing BillableMetrics webhook services, registers them in
SendWebhookJob::WEBHOOK_SERVICES, and wires the SendWebhookJob calls
into Coupons::{Create,Update,Destroy}Service.
## Context Webhook endpoints can only subscribe to the event types declared in `config/webhook_event_types.yml`. The `coupon.*` webhooks were added to `SendWebhookJob::WEBHOOK_SERVICES` without being declared there, so an endpoint that selects its events could not subscribe to them, and the webhook form did not list them. `spec/models/webhook_endpoint_spec.rb` also requires every handler type to be declared. ## Description `coupon.created`, `coupon.updated` and `coupon.deleted` are declared under a new `Coupons` category. The GraphQL schema was dumped again, since `EventTypeEnum` and `EventCategoryEnum` are built from this file.
## Context A coupon is terminated by the `terminateCoupon` GraphQL mutation, or by `Clock::TerminateCouponsJob` once its `expiration_at` has passed. The coupon webhook payload exposes `status` through `terminated_at`, but no webhook was sent on termination, so a consumer that mirrors coupons from the webhooks kept them active. ## Description `Coupons::TerminateService` now sends `coupon.updated` when it terminates a coupon, both for a single coupon and for the expired ones. Nothing is sent when the coupon was already terminated.
2bfcc07 to
510adf3
Compare
## Context Termination is a distinct step in a coupon's life, like `subscription.terminated` for subscriptions. Sending it as `coupon.updated` made it hard to tell apart from an edit. The expired coupons were also terminated by a loop that repeated the body of `Coupons::TerminateService#call`. ## Description `Coupons::TerminateService` now sends a new `coupon.terminated` webhook, declared in `config/webhook_event_types.yml`, in place of `coupon.updated`. `terminate_all_expired` calls the service for each expired coupon, so both paths terminate and notify the same way.
|
The branch was rebased on |
## Context The coupon webhook specs only checked a few keys of the payload, mostly by type, through the `creates webhook` shared example, which ignores the keys it is not given. A field missing from the payload, renamed or with a wrong value was not caught. ## Description Each spec now builds a percentage coupon with a recurring frequency, a time limit and a plan and a billable metric as targets, so most fields hold a real value, and compares the whole payload with `eq`. The terminated spec also checks `terminated_at`.
2 participants
Context
Coupons send no webhooks, unlike plans, billable metrics and features, which send
*.created,*.updatedand*.deleted. An integration that mirrors the coupon catalog has to poll the API to see a new, changed, terminated or deleted coupon.Description
The coupon services now send
coupon.created,coupon.updatedandcoupon.deleted, with the samecouponpayload as the REST API. The webhook services follow theWebhooks::BillableMetrics::*Serviceclasses.Coupons::TerminateServicealso sends a newcoupon.terminatedwebhook, so a termination can be told apart from an edit (likesubscription.terminated). It is sent both for theterminateCouponmutation and for coupons terminated byClock::TerminateCouponsJobonce theirexpiration_athas passed. Expired coupons now go through the same service call, so both paths behave the same. Nothing is sent when the coupon was already terminated.The four event types are declared in
config/webhook_event_types.ymlunder a newCouponscategory, so a webhook endpoint can subscribe to them. The GraphQL schema was dumped again, sinceEventTypeEnumandEventCategoryEnumare built from this file.Fixes #6307