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

Allow for non-existent event types in Webhooks::Outgoing::Endpoint#event_types #983

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

jagthedrummer
Copy link
Contributor

@jagthedrummer jagthedrummer commented Dec 16, 2024

Fixes: #654

The Set Up

Let's say you had created a Widget model, and had added widget.* events to config/models/webhooks/outgoing/event_types.yml:

scaffolding/absolutely_abstract/creative_concept:
  # This automatically generates `created`, `updated`, and `deleted` webhook events.
  - crud
scaffolding/completely_concrete/tangible_thing:
  - crud
invitation:
  - crud
membership:
  - crud
widget:
  - crud

And then you (or one of your users) sets up an Outgoing Webhook using one of the widget events. Effectively this:

  endpoint = Webhooks::Outgoing::Endpoint.create!(
    url: "https://example.com/webhook",
    name: "test",
    team: @team,
    event_type_ids: ["widget.created", "membership.created"]
  )

Then at a later time you decide that the Widget model was a bad idea, and so you remove it from event_types.yml.

Old Behavior

Previously, if you then called event_types on that endpoint we'd raise an ActiveHash::RecordNotFound error:

> endpoint.event_types
bullet_train-outgoing_webhooks (1.11.0)
  app/models/concerns/webhooks/outgoing/endpoint_support.rb:37:
  in `block in event_types':
  Couldn't find Webhooks::Outgoing::EventType with ID=widget.created
  (ActiveHash::RecordNotFound)
        from bullet_train-outgoing_webhooks (1.11.0) app/models/concerns/webhooks/outgoing/endpoint_support.rb:37:in `map'
        from bullet_train-outgoing_webhooks (1.11.0) app/models/concerns/webhooks/outgoing/endpoint_support.rb:37:in `event_types'

New Behavior

Now if you call event_types on that endpoint we'll return an ActiveRecord::Relation of Webhooks::Outgoing::EventType objects for the still-good event_type_ids for the endpoint. Since widget.created is no longer a valid type, it won't show up in the event_types array and we won't raise an error.

> endpoint.event_types
=> [
      #<Webhooks::Outgoing::EventType:0x000000014dddc068 
        @attributes={:id=>"membership.created"}>
   ]

Possible breaking change

Previously event_types would return an Array of Webhooks::Outgoing::EventType objects.

Now it returns an ActiveRecord::Relation of Webhooks::Outgoing::EventType objects.

@jagthedrummer jagthedrummer merged commit eb8ef54 into main Dec 18, 2024
34 checks passed
@jagthedrummer jagthedrummer deleted the jeremy/non-existent-event-types branch December 18, 2024 15:05
Copy link

@RichStone RichStone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one adding tests here, I now see the most probable reason for the raise here 👀

event_type_ids: ["fake-thing.create"]
)
assert @endpoint.persisted?
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, maybe this was one of the reasons why the .find() was there, namely to raise on creation, since there is no other validation in place for that. It did make sense that we raised before here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for non-existent event types in Webhooks::Outgoing::Endpoint#event_types
2 participants