Cleaned up unsupported and ignored event handling #1185
Conversation
| @@ -16,15 +16,13 @@ defmodule CodeCorps.GitHub.Event.Installation do | |||
| alias Ecto.{Changeset, Multi} | |||
|
|
|||
| @type outcome :: {:ok, GithubAppInstallation.t} | | |||
| {:error, :not_yet_implemented} | | |||
| {:error, :unexpected_action} | | |||
There was a problem hiding this comment.
:not_yet_implemented and :unexpected_action are no longer reachable with the other changes, so I've removed them for the benefit of simplifying. They would technically still be reachable through a unit test, but not through any sort of integration test.
| @spec handle_supported(String.t, String.t, map) :: {:ok, GithubEvent.t} | ||
| def handle_supported(type, id, %{} = payload) do | ||
| with {:ok, %GithubEvent{} = event} <- type |> build_params(id, "unprocessed", payload) |> create_event() do | ||
| payload |> apply_handler(type) |> Event.stop_processing(event) | ||
| end | ||
| end |
There was a problem hiding this comment.
Removed what's basically an if from this module and simply flattened it out from the POV of the controller. Keeps things simpler, allows us to test more directly, and peels off an abstraction layer.
| end | ||
|
|
||
| defdelegate process(type, id, payload), to: Handler, as: :handle | ||
| end |
There was a problem hiding this comment.
It's basically just a delegate, so no real need for it.
| :supported -> | ||
| Processor.process_async(event_type, conn |> get_delivery_id, event_payload) | ||
| Processor.process(fn -> Handler.handle_supported(type, delivery_id, payload) end) |
There was a problem hiding this comment.
This is the CodeCorps.Processor now, calling CodeCorps.GitHub.Webhook.Handler directly
| {"pull_request", "review_request_removed"}, | ||
| {"pull_request", "labeled"}, | ||
| {"pull_request", "unlabeled"}, | ||
| {"pull_request", "synchronized"}, |
There was a problem hiding this comment.
This is actually called synchronize and not synchronized.
joshsmith
force-pushed
the
fix-partially-supported-github-errored-events
branch
from
ea875b5
to
2860e50
Compare
joshsmith
force-pushed
the
fix-partially-supported-github-errored-events
branch
2 times, most recently
from
e4e8997
to
4dad13e
Compare
|
There were actually a number of things amiss here, including most importantly that the |
joshsmith
force-pushed
the
fix-partially-supported-github-errored-events
branch
from
4dad13e
to
65f5143
Compare
This PR deals with us storing partially unsupported (we support the type, but not the action) events as errored.
Instead, they are now stored as "unsupported".
Additionally, this PR adds
{"pull_request", "syncronized"}to the list of unsupported events.While all this works and removes some noise from our event log, I should not that functionally, these partially supported events are no different than events we outright ignore. Once and if we add full support, we will not be digging through our event log to restore them, se we should either
The first gives us the benefit of tracking down events we may have missed potentially. The second gives us les work and less things to store, so I'm not sure of our approach (mainly due to not being sure how much we would end up storing).