From a3bee2820d26c8ff281d32786ca168927b93de8e Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Mon, 3 Jul 2017 16:12:27 +0200 Subject: [PATCH] Implement handling of "InstallationRepositories" "added" and "removed" events --- lib/code_corps/github/event.ex | 36 ++++ .../github/{events => event}/installation.ex | 62 +++--- .../github/event/installation_repositories.ex | 129 ++++++++++++ .../github/{events => event}/issue_comment.ex | 2 +- .../github/{events => event}/issues.ex | 2 +- .../events/installation_repositories.ex | 17 -- lib/code_corps/github/webhook/handler.ex | 8 +- ...70630140136_create_project_github_repo.exs | 14 ++ priv/repo/structure.sql | 76 ++++++- .../installation_repositories_added.json | 64 ++++++ .../installation_repositories_removed.json | 5 + .../event/installation_repositories_test.exs | 185 ++++++++++++++++++ .../{events => event}/installation_test.exs | 4 +- .../{events => event}/issue_comment_test.exs | 4 +- .../github/{events => event}/issues_test.exs | 4 +- test/lib/code_corps/github/event_test.exs | 37 ++++ .../events/installation_repositories_test.exs | 19 -- .../github/webhook/handler_test.exs | 41 +++- test/models/github_repo_test.exs | 14 ++ test/support/factories.ex | 11 ++ web/models/github_app_installation.ex | 1 - web/models/github_event.ex | 2 + web/models/project_github_repo.ex | 16 ++ 23 files changed, 662 insertions(+), 91 deletions(-) create mode 100644 lib/code_corps/github/event.ex rename lib/code_corps/github/{events => event}/installation.ex (62%) create mode 100644 lib/code_corps/github/event/installation_repositories.ex rename lib/code_corps/github/{events => event}/issue_comment.ex (88%) rename lib/code_corps/github/{events => event}/issues.ex (89%) delete mode 100644 lib/code_corps/github/events/installation_repositories.ex create mode 100644 priv/repo/migrations/20170630140136_create_project_github_repo.exs create mode 100644 test/fixtures/github/events/installation_repositories_added.json create mode 100644 test/lib/code_corps/github/event/installation_repositories_test.exs rename test/lib/code_corps/github/{events => event}/installation_test.exs (97%) rename test/lib/code_corps/github/{events => event}/issue_comment_test.exs (80%) rename test/lib/code_corps/github/{events => event}/issues_test.exs (82%) create mode 100644 test/lib/code_corps/github/event_test.exs delete mode 100644 test/lib/code_corps/github/events/installation_repositories_test.exs create mode 100644 test/models/github_repo_test.exs create mode 100644 web/models/project_github_repo.ex diff --git a/lib/code_corps/github/event.ex b/lib/code_corps/github/event.ex new file mode 100644 index 000000000..e5ff792d6 --- /dev/null +++ b/lib/code_corps/github/event.ex @@ -0,0 +1,36 @@ +defmodule CodeCorps.GitHub.Event do + @moduledoc ~S""" + In charge of marking `GithubEvent` records as "processing", "processed" or + "errored", based on the outcome of processing a webhook event payload. + """ + + alias CodeCorps.{GithubEvent, Repo} + alias Ecto.Changeset + + @type error :: atom | Changeset.t + @type processing_result :: {:ok, any} | {:error, error} + + @doc ~S""" + Sets record status to "processing", marking it as being processed at this + moment. Our webhook handling should skip processing payloads for events which + are already being processed. + """ + @spec start_processing(GithubEvent.t) :: {:ok, GithubEvent.t} + def start_processing(%GithubEvent{} = event) do + event |> Changeset.change(%{status: "processing"}) |> Repo.update() + end + + @doc ~S""" + Sets record status to "processed" or "errored" based on the first element of + first argument, which is the result tuple. The result tuple should always be + either `{:ok, data}` if the the processing of the event payload went as + expected, or `{:error, reason}` if something went wrong. + """ + @spec stop_processing(processing_result, GithubEvent.t) :: {:ok, GithubEvent.t} + def stop_processing({:ok, _data}, %GithubEvent{} = event) do + event |> Changeset.change(%{status: "processed"}) |> Repo.update + end + def stop_processing({:error, _reason}, %GithubEvent{} = event) do + event |> Changeset.change(%{status: "errored"}) |> Repo.update + end +end diff --git a/lib/code_corps/github/events/installation.ex b/lib/code_corps/github/event/installation.ex similarity index 62% rename from lib/code_corps/github/events/installation.ex rename to lib/code_corps/github/event/installation.ex index 052193b7d..c136ec490 100644 --- a/lib/code_corps/github/events/installation.ex +++ b/lib/code_corps/github/event/installation.ex @@ -1,4 +1,4 @@ -defmodule CodeCorps.GitHub.Events.Installation do +defmodule CodeCorps.GitHub.Event.Installation do @moduledoc """ In charge of dealing with "Installation" GitHub Webhook events """ @@ -8,6 +8,7 @@ defmodule CodeCorps.GitHub.Events.Installation do GithubAppInstallation, GithubEvent, GithubRepo, + GitHub.Event, Repo, User } @@ -25,14 +26,15 @@ defmodule CodeCorps.GitHub.Events.Installation do - marked the passed in event as "processed" or "errored" """ @spec handle(GithubEvent.t, map) :: {:ok, GithubEvent.t} - def handle(%GithubEvent{action: "created"} = event, payload) do + def handle(%GithubEvent{action: action} = event, payload) do event - |> start_event_processing() - |> do_handle(payload) - |> stop_event_processing(event) + |> Event.start_processing() + |> do_handle(action, payload) + |> Event.stop_processing(event) end - defp do_handle({:ok, %GithubEvent{}}, %{"installation" => installation_attrs, "sender" => sender_attrs}) do + @spec do_handle({:ok, GithubEvent.t}, String.t, map) :: {:ok, GithubAppInstallation.t} | {:error, :api_error} + defp do_handle({:ok, %GithubEvent{}}, "created", %{"installation" => installation_attrs, "sender" => sender_attrs}) do case {sender_attrs |> find_user, installation_attrs |> find_installation} do {nil, nil} -> create_unmatched_user_installation(installation_attrs) {%User{} = user, nil} -> create_installation_initiated_on_github(user, installation_attrs) @@ -40,6 +42,7 @@ defmodule CodeCorps.GitHub.Events.Installation do end end + @spec create_installation_initiated_on_github(User.t, map) :: {:ok, GithubAppInstallation.t} defp create_installation_initiated_on_github(%User{} = user, %{"id" => github_id}) do %GithubAppInstallation{} |> Changeset.change(%{github_id: github_id, installed: true, state: "initiated_on_github"}) @@ -47,6 +50,7 @@ defmodule CodeCorps.GitHub.Events.Installation do |> Repo.insert() end + @spec create_matched_installation(GithubAppInstallation.t) :: {:ok, GithubAppInstallation.t} defp create_matched_installation(%GithubAppInstallation{} = installation) do installation |> start_installation_processing() @@ -54,6 +58,14 @@ defmodule CodeCorps.GitHub.Events.Installation do |> stop_installation_processing() end + @spec create_unmatched_user_installation(map) :: {:ok, GithubAppInstallation.t} + defp create_unmatched_user_installation(%{"id" => github_id}) do + %GithubAppInstallation{} + |> Changeset.change(%{github_id: github_id, installed: true, state: "unmatched_user"}) + |> Repo.insert() + end + + @spec create_repository(GithubAppInstallation.t, map) :: {:ok, GithubRepo.t} defp create_repository(%GithubAppInstallation{} = installation, repo_attributes) do %GithubRepo{} |> Changeset.change(repo_attributes |> GithubRepoAdapter.from_api) @@ -61,46 +73,32 @@ defmodule CodeCorps.GitHub.Events.Installation do |> Repo.insert() end - defp create_unmatched_user_installation(%{"id" => github_id}) do - %GithubAppInstallation{} - |> Changeset.change(%{github_id: github_id, installed: true, state: "unmatched_user"}) - |> Repo.insert() - end - + @spec find_installation(map) :: GithubAppInstallation.t | nil defp find_installation(%{"id" => github_id}), do: GithubAppInstallation |> Repo.get_by(github_id: github_id) + @spec find_user(map) :: User.t | nil defp find_user(%{"id" => github_id}), do: User |> Repo.get_by(github_id: github_id) + @spec process_repos({:ok, GithubAppInstallation.t}) :: {:ok, GithubAppInstallation.t} | {:error, :api_error} defp process_repos({:ok, %GithubAppInstallation{} = installation}) do - # TODO: Consider moving into transaction - {:ok, repositories} = - installation - |> GitHub.Installation.repositories() - |> (fn {:ok, repositories} -> repositories end).() - |> Enum.map(&create_repository(installation, &1)) - |> Enum.map(fn {:ok, repository} -> repository end) - |> (fn repositories -> {:ok, repositories} end).() - - {:ok, installation |> Map.put(:github_repos, repositories)} - end + with {:ok, repo_payloads} <- installation |> GitHub.Installation.repositories() do + repositories = + repo_payloads + |> Enum.map(&create_repository(installation, &1)) + |> Enum.map(fn {:ok, repository} -> repository end) - defp start_event_processing(%GithubEvent{} = event) do - event |> Changeset.change(%{status: "processing"}) |> Repo.update() - end - - defp stop_event_processing({:ok, %GithubAppInstallation{}}, %GithubEvent{} = event) do - event |> Changeset.change(%{status: "processed"}) |> Repo.update - end - defp stop_event_processing(_, %GithubEvent{} = event) do - event |> Changeset.change(%{status: "errored"}) |> Repo.update + {:ok, installation |> Map.put(:github_repos, repositories)} + end end + @spec start_installation_processing({:ok, GithubAppInstallation.t}) :: {:ok, GithubAppInstallation.t} defp start_installation_processing(%GithubAppInstallation{} = installation) do installation |> Changeset.change(%{installed: true, state: "processing"}) |> Repo.update() end + @spec stop_installation_processing({:ok, GithubAppInstallation.t}) :: {:ok, GithubAppInstallation.t} defp stop_installation_processing({:ok, %GithubAppInstallation{} = installation}) do installation |> Changeset.change(%{state: "processed"}) diff --git a/lib/code_corps/github/event/installation_repositories.ex b/lib/code_corps/github/event/installation_repositories.ex new file mode 100644 index 000000000..9589a1955 --- /dev/null +++ b/lib/code_corps/github/event/installation_repositories.ex @@ -0,0 +1,129 @@ +defmodule CodeCorps.GitHub.Event.InstallationRepositories do + @moduledoc """ + In charge of dealing with "InstallationRepositories" GitHub Webhook events + """ + + alias CodeCorps.{ + GithubAppInstallation, + GithubEvent, + GithubRepo, + GitHub.Event, + Repo + } + + alias Ecto.Changeset + + @doc """ + Handles an "InstallationRepositories" GitHub Webhook event. The event could be + of subtype "added" or "removed" and is handled differently based on that. + + `InstallationRepositories::added` will + - find `GithubAppInstallation` + - marks event as errored if not found + - marks event as errored if payload does not have keys it needs, meaning + there's something wrong with our code and we need to updated + - find or create `GithubRepo` records affected + + `InstallationRepositories::removed` will + - find `GithubAppInstallation` + - marks event as errored if not found + - marks event as errored if payload does not have keys it needs, meaning + there's something wrong with our code and we need to updated + - find `GithubRepo` records affected and delete them + - if there is no `GithubRepo` to delete, it just skips it + - `ProjectGithubRepo` are deleted automatically, since they are set to + `on_delete: :delete_all` + """ + @spec handle(GithubEvent.t, map) :: {:ok, GithubEvent.t} + def handle(%GithubEvent{action: action} = event, payload) do + event + |> Event.start_processing() + |> do_handle(action, payload) + |> Event.stop_processing(event) + end + + @typep outcome :: {:ok, [GithubRepo.t]} | + {:error, :no_installation} | + {:error, :unexpected_action_or_payload} | + {:error, :unexpected_installation_payload} | + {:error, :unexpected_repo_payload} + + @spec do_handle({:ok, GithubEvent.t}, String.t, map) :: outcome + defp do_handle({:ok, %GithubEvent{}}, "added", %{"installation" => installation_attrs, "repositories_added" => repositories_attr_list}) do + case installation_attrs |> find_installation() do + nil -> {:error, :no_installation} + :unexpected_installation_payload -> {:error, :unexpected_installation_payload} + %GithubAppInstallation{} = installation -> + case repositories_attr_list |> Enum.all?(&valid?/1) do + true -> find_or_create_all(installation, repositories_attr_list) + false -> {:error, :unexpected_repo_payload} + end + end + end + defp do_handle({:ok, %GithubEvent{}}, "removed", %{"installation" => installation_attrs, "repositories_removed" => repositories_attr_list}) do + case installation_attrs |> find_installation() do + nil -> {:error, :no_installation} + :unexpected_installation_payload -> {:error, :unexpected_installation_payload} + %GithubAppInstallation{} = installation -> + case repositories_attr_list |> Enum.all?(&valid?/1) do + true -> delete_all(installation, repositories_attr_list) + false -> {:error, :unexpected_repo_payload} + end + end + end + defp do_handle({:ok, %GithubEvent{}}, _action, _payload), do: {:error, :unexpected_action_or_payload} + + @spec find_installation(any) :: GithubAppInstallation.t | nil | :unexpected_installation_payload + defp find_installation(%{"id" => github_id}), do: GithubAppInstallation |> Repo.get_by(github_id: github_id) + defp find_installation(_payload), do: :unexpected_installation_payload + + # should return true if the payload is a map and has the expected keys + @spec valid?(any) :: boolean + defp valid?(%{"id" => _, "name" => _}), do: true + defp valid?(_), do: false + + @spec find_or_create_all(GithubAppInstallation.t, list(map)) :: {:ok, list(GithubRepo.t)} + defp find_or_create_all(%GithubAppInstallation{} = installation, repositories_attr_list) when is_list(repositories_attr_list) do + repositories_attr_list + |> Enum.map(&find_or_create(installation, &1)) + |> aggregate() + end + + @spec find_or_create(GithubAppInstallation.t, map) :: {:ok, GithubRepo.t} + defp find_or_create(%GithubAppInstallation{} = installation, %{"id" => github_id, "name" => name} = attrs) do + case find_repo(installation, attrs) do + nil -> + %GithubRepo{} + |> Changeset.change(%{github_id: github_id, name: name}) + |> Changeset.put_assoc(:github_app_installation, installation) + |> Repo.insert() + %GithubRepo{} = github_repo -> + {:ok, github_repo} + end + end + + @spec delete_all(GithubAppInstallation.t, list(map)) :: {:ok, [GithubRepo.t]} + defp delete_all(%GithubAppInstallation{} = installation, repositories_attr_list) when is_list(repositories_attr_list) do + repositories_attr_list + |> Enum.map(&find_repo(installation, &1)) + |> Enum.reject(&is_nil/1) + |> Enum.map(&Repo.delete/1) + |> aggregate() + end + + @spec find_repo(GithubAppInstallation.t, map) :: GithubRepo.t | nil + defp find_repo(%GithubAppInstallation{id: installation_id}, %{"id" => github_id}) do + GithubRepo + |> Repo.get_by(github_app_installation_id: installation_id, github_id: github_id) + end + + # [{:ok, repo_1}, {:ok, repo_2}, {:ok, repo_3}] -> {:ok, [repo_1, repo_2, repo_3]} + @spec aggregate(list({:ok, GithubRepo.t})) :: {:ok, list(GithubRepo.t)} + defp aggregate(results) do + repositories = + results + |> Enum.map(fn {:ok, %GithubRepo{} = github_repo} -> github_repo end) + + {:ok, repositories} + end +end diff --git a/lib/code_corps/github/events/issue_comment.ex b/lib/code_corps/github/event/issue_comment.ex similarity index 88% rename from lib/code_corps/github/events/issue_comment.ex rename to lib/code_corps/github/event/issue_comment.ex index 63e494865..905769ff8 100644 --- a/lib/code_corps/github/events/issue_comment.ex +++ b/lib/code_corps/github/event/issue_comment.ex @@ -1,4 +1,4 @@ -defmodule CodeCorps.GitHub.Events.IssueComment do +defmodule CodeCorps.GitHub.Event.IssueComment do @moduledoc """ In charge of dealing with "IssueComment" GitHub Webhook events """ diff --git a/lib/code_corps/github/events/issues.ex b/lib/code_corps/github/event/issues.ex similarity index 89% rename from lib/code_corps/github/events/issues.ex rename to lib/code_corps/github/event/issues.ex index 9abac3f54..6d590a559 100644 --- a/lib/code_corps/github/events/issues.ex +++ b/lib/code_corps/github/event/issues.ex @@ -1,4 +1,4 @@ -defmodule CodeCorps.GitHub.Events.Issues do +defmodule CodeCorps.GitHub.Event.Issues do @moduledoc """ In charge of dealing with "Issues" GitHub Webhook events """ diff --git a/lib/code_corps/github/events/installation_repositories.ex b/lib/code_corps/github/events/installation_repositories.ex deleted file mode 100644 index 47722a8d1..000000000 --- a/lib/code_corps/github/events/installation_repositories.ex +++ /dev/null @@ -1,17 +0,0 @@ -defmodule CodeCorps.GitHub.Events.InstallationRepositories do - @moduledoc """ - In charge of dealing with "InstallationRepositories" GitHub Webhook events - """ - - alias CodeCorps.GithubEvent - - @doc """ - Handles an "InstallationRepositories" GitHub Webhook event - - The general idea is - - marked the passed in event as "processing" - - do the work - - marked the passed in event as "processed" or "errored" - """ - def handle(%GithubEvent{}, %{}), do: :not_fully_implemented -end diff --git a/lib/code_corps/github/webhook/handler.ex b/lib/code_corps/github/webhook/handler.ex index 7462e4143..506194927 100644 --- a/lib/code_corps/github/webhook/handler.ex +++ b/lib/code_corps/github/webhook/handler.ex @@ -5,10 +5,10 @@ defmodule CodeCorps.GitHub.Webhook.Handler do alias CodeCorps.{ GithubEvent, - GitHub.Events.Installation, - GitHub.Events.InstallationRepositories, - GitHub.Events.IssueComment, - GitHub.Events.Issues, + GitHub.Event.Installation, + GitHub.Event.InstallationRepositories, + GitHub.Event.IssueComment, + GitHub.Event.Issues, GitHub.Webhook.EventSupport, Repo } diff --git a/priv/repo/migrations/20170630140136_create_project_github_repo.exs b/priv/repo/migrations/20170630140136_create_project_github_repo.exs new file mode 100644 index 000000000..94f76704f --- /dev/null +++ b/priv/repo/migrations/20170630140136_create_project_github_repo.exs @@ -0,0 +1,14 @@ +defmodule CodeCorps.Repo.Migrations.CreateProjectGithubRepo do + use Ecto.Migration + + def change do + create table(:project_github_repos) do + add :project_id, references(:projects, on_delete: :delete_all) + add :github_repo_id, references(:github_repos, on_delete: :delete_all) + + timestamps() + end + + create unique_index(:project_github_repos, [:project_id, :github_repo_id]) + end +end diff --git a/priv/repo/structure.sql b/priv/repo/structure.sql index c7727787e..3ff96d81e 100644 --- a/priv/repo/structure.sql +++ b/priv/repo/structure.sql @@ -2,8 +2,8 @@ -- PostgreSQL database dump -- --- Dumped from database version 9.5.4 --- Dumped by pg_dump version 9.5.4 +-- Dumped from database version 9.5.1 +-- Dumped by pg_dump version 9.5.1 SET statement_timeout = 0; SET lock_timeout = 0; @@ -395,6 +395,38 @@ CREATE SEQUENCE project_categories_id_seq ALTER SEQUENCE project_categories_id_seq OWNED BY project_categories.id; +-- +-- Name: project_github_repos; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE project_github_repos ( + id integer NOT NULL, + project_id integer, + github_repo_id integer, + inserted_at timestamp without time zone NOT NULL, + updated_at timestamp without time zone NOT NULL +); + + +-- +-- Name: project_github_repos_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE project_github_repos_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: project_github_repos_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE project_github_repos_id_seq OWNED BY project_github_repos.id; + + -- -- Name: project_skills; Type: TABLE; Schema: public; Owner: - -- @@ -1530,6 +1562,13 @@ ALTER TABLE ONLY previews ALTER COLUMN id SET DEFAULT nextval('previews_id_seq': ALTER TABLE ONLY project_categories ALTER COLUMN id SET DEFAULT nextval('project_categories_id_seq'::regclass); +-- +-- Name: id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY project_github_repos ALTER COLUMN id SET DEFAULT nextval('project_github_repos_id_seq'::regclass); + + -- -- Name: id; Type: DEFAULT; Schema: public; Owner: - -- @@ -1799,6 +1838,14 @@ ALTER TABLE ONLY project_categories ADD CONSTRAINT project_categories_pkey PRIMARY KEY (id); +-- +-- Name: project_github_repos_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY project_github_repos + ADD CONSTRAINT project_github_repos_pkey PRIMARY KEY (id); + + -- -- Name: project_skills_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -2094,6 +2141,13 @@ CREATE UNIQUE INDEX organizations_lower_slug_index ON organizations USING btree CREATE UNIQUE INDEX project_categories_project_id_category_id_index ON project_categories USING btree (project_id, category_id); +-- +-- Name: project_github_repos_project_id_github_repo_id_index; Type: INDEX; Schema: public; Owner: - +-- + +CREATE UNIQUE INDEX project_github_repos_project_id_github_repo_id_index ON project_github_repos USING btree (project_id, github_repo_id); + + -- -- Name: project_skills_project_id_index; Type: INDEX; Schema: public; Owner: - -- @@ -2476,6 +2530,22 @@ ALTER TABLE ONLY project_categories ADD CONSTRAINT project_categories_project_id_fkey FOREIGN KEY (project_id) REFERENCES projects(id); +-- +-- Name: project_github_repos_github_repo_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY project_github_repos + ADD CONSTRAINT project_github_repos_github_repo_id_fkey FOREIGN KEY (github_repo_id) REFERENCES github_repos(id) ON DELETE CASCADE; + + +-- +-- Name: project_github_repos_project_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY project_github_repos + ADD CONSTRAINT project_github_repos_project_id_fkey FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + + -- -- Name: project_skills_project_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -2808,5 +2878,5 @@ ALTER TABLE ONLY user_tasks -- PostgreSQL database dump complete -- -INSERT INTO "schema_migrations" (version) VALUES (20160723215749), (20160804000000), (20160804001111), (20160805132301), (20160805203929), (20160808143454), (20160809214736), (20160810124357), (20160815125009), (20160815143002), (20160816020347), (20160816034021), (20160817220118), (20160818000944), (20160818132546), (20160820113856), (20160820164905), (20160822002438), (20160822004056), (20160822011624), (20160822020401), (20160822044612), (20160830081224), (20160830224802), (20160911233738), (20160912002705), (20160912145957), (20160918003206), (20160928232404), (20161003185918), (20161019090945), (20161019110737), (20161020144622), (20161021131026), (20161031001615), (20161121005339), (20161121014050), (20161121043941), (20161121045709), (20161122015942), (20161123081114), (20161123150943), (20161124085742), (20161125200620), (20161126045705), (20161127054559), (20161205024856), (20161207112519), (20161209192504), (20161212005641), (20161214005935), (20161215052051), (20161216051447), (20161218005913), (20161219160401), (20161219163909), (20161220141753), (20161221085759), (20161226213600), (20161231063614), (20170102130055), (20170102181053), (20170104113708), (20170104212623), (20170104235423), (20170106013143), (20170115035159), (20170115230549), (20170121014100), (20170131234029), (20170201014901), (20170201025454), (20170201035458), (20170201183258), (20170220032224), (20170224233516), (20170226050552), (20170228085250), (20170308214128), (20170308220713), (20170308222552), (20170313130611), (20170318032449), (20170318082740), (20170324194827), (20170424215355), (20170501225441), (20170526095401), (20170602000208), (20170622205732), (20170626231059), (20170628092119), (20170628213609), (20170629183404); +INSERT INTO "schema_migrations" (version) VALUES (20160723215749), (20160804000000), (20160804001111), (20160805132301), (20160805203929), (20160808143454), (20160809214736), (20160810124357), (20160815125009), (20160815143002), (20160816020347), (20160816034021), (20160817220118), (20160818000944), (20160818132546), (20160820113856), (20160820164905), (20160822002438), (20160822004056), (20160822011624), (20160822020401), (20160822044612), (20160830081224), (20160830224802), (20160911233738), (20160912002705), (20160912145957), (20160918003206), (20160928232404), (20161003185918), (20161019090945), (20161019110737), (20161020144622), (20161021131026), (20161031001615), (20161121005339), (20161121014050), (20161121043941), (20161121045709), (20161122015942), (20161123081114), (20161123150943), (20161124085742), (20161125200620), (20161126045705), (20161127054559), (20161205024856), (20161207112519), (20161209192504), (20161212005641), (20161214005935), (20161215052051), (20161216051447), (20161218005913), (20161219160401), (20161219163909), (20161220141753), (20161221085759), (20161226213600), (20161231063614), (20170102130055), (20170102181053), (20170104113708), (20170104212623), (20170104235423), (20170106013143), (20170115035159), (20170115230549), (20170121014100), (20170131234029), (20170201014901), (20170201025454), (20170201035458), (20170201183258), (20170220032224), (20170224233516), (20170226050552), (20170228085250), (20170308214128), (20170308220713), (20170308222552), (20170313130611), (20170318032449), (20170318082740), (20170324194827), (20170424215355), (20170501225441), (20170526095401), (20170602000208), (20170622205732), (20170626231059), (20170628092119), (20170628213609), (20170629183404), (20170630140136); diff --git a/test/fixtures/github/events/installation_repositories_added.json b/test/fixtures/github/events/installation_repositories_added.json new file mode 100644 index 000000000..6040236ce --- /dev/null +++ b/test/fixtures/github/events/installation_repositories_added.json @@ -0,0 +1,64 @@ +{ + "action": "added", + "installation": { + "id": 2, + "account": { + "login": "octocat", + "id": 1, + "avatar_url": "https://github.com/images/error/octocat_happy.gif", + "gravatar_id": "", + "url": "https://api.github.com/users/octocat", + "html_url": "https://github.com/octocat", + "followers_url": "https://api.github.com/users/octocat/followers", + "following_url": "https://api.github.com/users/octocat/following{/other_user}", + "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}", + "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/octocat/subscriptions", + "organizations_url": "https://api.github.com/users/octocat/orgs", + "repos_url": "https://api.github.com/users/octocat/repos", + "events_url": "https://api.github.com/users/octocat/events{/privacy}", + "received_events_url": "https://api.github.com/users/octocat/received_events", + "type": "User", + "site_admin": false + }, + "repository_selection": "selected", + "access_tokens_url": "https://api.github.com/installations/2/access_tokens", + "repositories_url": "https://api.github.com/installation/repositories", + "html_url": "https://github.com/settings/installations/2" + }, + "repository_selection": "selected", + "repositories_added": [ + { + "id": 1296269, + "name": "Hello-World", + "full_name": "octocat/Hello-World" + }, + { + "id": 1296270, + "name": "Hello-Code-Corps", + "full_name": "octocat/Hello-Code-Corps" + } + ], + "repositories_removed": [ + + ], + "sender": { + "login": "octocat", + "id": 1, + "avatar_url": "https://github.com/images/error/octocat_happy.gif", + "gravatar_id": "", + "url": "https://api.github.com/users/octocat", + "html_url": "https://github.com/octocat", + "followers_url": "https://api.github.com/users/octocat/followers", + "following_url": "https://api.github.com/users/octocat/following{/other_user}", + "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}", + "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/octocat/subscriptions", + "organizations_url": "https://api.github.com/users/octocat/orgs", + "repos_url": "https://api.github.com/users/octocat/repos", + "events_url": "https://api.github.com/users/octocat/events{/privacy}", + "received_events_url": "https://api.github.com/users/octocat/received_events", + "type": "User", + "site_admin": false + } +} diff --git a/test/fixtures/github/events/installation_repositories_removed.json b/test/fixtures/github/events/installation_repositories_removed.json index 96f7adfcc..00d3f1bd3 100644 --- a/test/fixtures/github/events/installation_repositories_removed.json +++ b/test/fixtures/github/events/installation_repositories_removed.json @@ -35,6 +35,11 @@ "id": 1296269, "name": "Hello-World", "full_name": "octocat/Hello-World" + }, + { + "id": 1296270, + "name": "Hello-Code-Corps", + "full_name": "octocat/Hello-Code-Corps" } ], "sender": { diff --git a/test/lib/code_corps/github/event/installation_repositories_test.exs b/test/lib/code_corps/github/event/installation_repositories_test.exs new file mode 100644 index 000000000..e448e6c25 --- /dev/null +++ b/test/lib/code_corps/github/event/installation_repositories_test.exs @@ -0,0 +1,185 @@ +defmodule CodeCorps.GitHub.Event.InstallationRepositoriesTest do + @moduledoc false + + use CodeCorps.DbAccessCase + use CodeCorps.GitHubCase + + import CodeCorps.Factories + import CodeCorps.TestHelpers.GitHub + + alias CodeCorps.{ + GithubEvent, + GithubRepo, + GitHub.Event.InstallationRepositories, + ProjectGithubRepo, + Repo + } + + describe "handle/2" do + test "marks event as errored if invalid action" do + payload = %{} + event = insert(:github_event, action: "foo", type: "installation_repositories") + assert InstallationRepositories.handle(event, payload) + + updated_event = Repo.one(GithubEvent) + assert updated_event.status == "errored" + end + + test "marks event as errored if invalid payload" do + payload = %{} + event = insert(:github_event, action: "added", type: "installation_repositories") + assert InstallationRepositories.handle(event, payload) + + updated_event = Repo.one(GithubEvent) + assert updated_event.status == "errored" + end + end + + describe "handle/2 for InstallationRepositories::added" do + @payload load_event_fixture("installation_repositories_added") + + test "creates repos" do + %{ + "installation" => %{"id" => installation_github_id}, + "repositories_added" => [repo_1_payload, repo_2_payload] + } = @payload + + %{id: installation_id} = insert(:github_app_installation, github_id: installation_github_id) + + event = insert(:github_event, action: "added", type: "installation_repositories") + assert InstallationRepositories.handle(event, @payload) + + github_repo_1 = Repo.get_by(GithubRepo, github_id: repo_1_payload["id"]) + assert github_repo_1 + assert github_repo_1.name == repo_1_payload["name"] + assert github_repo_1.github_app_installation_id == installation_id + + github_repo_2 = Repo.get_by(GithubRepo, github_id: repo_2_payload["id"]) + assert github_repo_2 + assert github_repo_2.name == repo_2_payload["name"] + assert github_repo_2.github_app_installation_id == installation_id + + updated_event = Repo.one(GithubEvent) + assert updated_event.status == "processed" + end + + test "skips creating existing repos" do + %{ + "installation" => %{"id" => installation_github_id}, + "repositories_added" => [repo_1_payload, repo_2_payload] + } = @payload + + installation = insert(:github_app_installation, github_id: installation_github_id) + preinserted_repo = insert(:github_repo, github_app_installation: installation, github_id: repo_1_payload["id"]) + + event = insert(:github_event, action: "added", type: "installation_repositories") + assert InstallationRepositories.handle(event, @payload) + + github_repo_1 = Repo.get_by(GithubRepo, github_id: repo_1_payload["id"]) + assert github_repo_1.id == preinserted_repo.id + + github_repo_2 = Repo.get_by(GithubRepo, github_id: repo_2_payload["id"]) + assert github_repo_2 + assert github_repo_2.name == repo_2_payload["name"] + assert github_repo_2.github_app_installation_id == installation.id + + assert Repo.aggregate(GithubRepo, :count, :id) == 2 + + updated_event = Repo.one(GithubEvent) + assert updated_event.status == "processed" + end + + test "marks event as errored if invalid instalation payload" do + event = insert(:github_event, action: "added", type: "installation_repositories") + assert InstallationRepositories.handle(event, @payload |> Map.put("installation", "foo")) + + updated_event = Repo.one(GithubEvent) + assert updated_event.status == "errored" + end + + test "marks event as errored if invalid repo payload" do + event = insert(:github_event, action: "added", type: "installation_repositories") + assert InstallationRepositories.handle(event, @payload |> Map.put("repositories_added", ["foo"])) + + updated_event = Repo.one(GithubEvent) + assert updated_event.status == "errored" + end + + test "marks event as errored if no installation" do + event = insert(:github_event, action: "added", type: "installation_repositories") + assert InstallationRepositories.handle(event, @payload) + + updated_event = Repo.one(GithubEvent) + assert updated_event.status == "errored" + end + end + + describe "handle/2 for InstallationRepositories::removed" do + @payload load_event_fixture("installation_repositories_removed") + + test "deletes github repos and associated project github repos" do + %{ + "installation" => %{"id" => installation_github_id}, + "repositories_removed" => [repo_1_payload, repo_2_payload] + } = @payload + + %{project: project} = installation = insert(:github_app_installation, github_id: installation_github_id) + github_repo_1 = insert(:github_repo, github_app_installation: installation, github_id: repo_1_payload["id"]) + insert(:project_github_repo, project: project, github_repo: github_repo_1) + insert(:github_repo, github_app_installation: installation, github_id: repo_2_payload["id"]) + + event = insert(:github_event, action: "removed", type: "installation_repositories") + assert InstallationRepositories.handle(event, @payload) + + assert Repo.aggregate(GithubRepo, :count, :id) == 0 + assert Repo.aggregate(ProjectGithubRepo, :count, :id) == 0 + + updated_event = Repo.one(GithubEvent) + assert updated_event.status == "processed" + end + + test "skips deleting if nothing to delete" do + %{ + "installation" => %{"id" => installation_github_id}, + "repositories_removed" => [repo_1_payload, _repo_2_payload] + } = @payload + + %{project: project} = installation = insert(:github_app_installation, github_id: installation_github_id) + github_repo_1 = insert(:github_repo, github_app_installation: installation, github_id: repo_1_payload["id"]) + insert(:project_github_repo, project: project, github_repo: github_repo_1) + + event = insert(:github_event, action: "removed", type: "installation_repositories") + assert InstallationRepositories.handle(event, @payload) + + assert Repo.aggregate(GithubRepo, :count, :id) == 0 + assert Repo.aggregate(ProjectGithubRepo, :count, :id) == 0 + + updated_event = Repo.one(GithubEvent) + assert updated_event.status == "processed" + end + + test "marks event as errored if invalid instalation payload" do + event = insert(:github_event, action: "removed", type: "installation_repositories") + assert InstallationRepositories.handle(event, @payload |> Map.put("installation", "foo")) + + updated_event = Repo.one(GithubEvent) + assert updated_event.status == "errored" + end + + test "marks event as errored if invalid repo payload" do + event = insert(:github_event, action: "removed", type: "installation_repositories") + assert InstallationRepositories.handle(event, @payload |> Map.put("repositories_removed", ["foo"])) + + updated_event = Repo.one(GithubEvent) + assert updated_event.status == "errored" + end + + test "marks event as errored if no installation" do + event = insert(:github_event, action: "added", type: "installation_repositories") + assert InstallationRepositories.handle(event, @payload) + + updated_event = Repo.one(GithubEvent) + assert updated_event.status == "errored" + end + end +end diff --git a/test/lib/code_corps/github/events/installation_test.exs b/test/lib/code_corps/github/event/installation_test.exs similarity index 97% rename from test/lib/code_corps/github/events/installation_test.exs rename to test/lib/code_corps/github/event/installation_test.exs index 1f2d72624..4ccf92771 100644 --- a/test/lib/code_corps/github/events/installation_test.exs +++ b/test/lib/code_corps/github/event/installation_test.exs @@ -1,4 +1,4 @@ -defmodule CodeCorps.GitHub.Events.InstallationTest do +defmodule CodeCorps.GitHub.Event.InstallationTest do @moduledoc false use CodeCorps.DbAccessCase @@ -11,7 +11,7 @@ defmodule CodeCorps.GitHub.Events.InstallationTest do GithubAppInstallation, GithubEvent, GithubRepo, - GitHub.Events.Installation, + GitHub.Event.Installation, Repo } diff --git a/test/lib/code_corps/github/events/issue_comment_test.exs b/test/lib/code_corps/github/event/issue_comment_test.exs similarity index 80% rename from test/lib/code_corps/github/events/issue_comment_test.exs rename to test/lib/code_corps/github/event/issue_comment_test.exs index 52afe062f..cf6d0384d 100644 --- a/test/lib/code_corps/github/events/issue_comment_test.exs +++ b/test/lib/code_corps/github/event/issue_comment_test.exs @@ -1,4 +1,4 @@ -defmodule CodeCorps.GitHub.Events.IssueCommentTest do +defmodule CodeCorps.GitHub.Event.IssueCommentTest do @moduledoc false use ExUnit.Case, aysnc: true @@ -7,7 +7,7 @@ defmodule CodeCorps.GitHub.Events.IssueCommentTest do alias CodeCorps.{ GithubEvent, - GitHub.Events.IssueComment + GitHub.Event.IssueComment } describe "handle/2" do diff --git a/test/lib/code_corps/github/events/issues_test.exs b/test/lib/code_corps/github/event/issues_test.exs similarity index 82% rename from test/lib/code_corps/github/events/issues_test.exs rename to test/lib/code_corps/github/event/issues_test.exs index 9e1b330d9..5e6d157ff 100644 --- a/test/lib/code_corps/github/events/issues_test.exs +++ b/test/lib/code_corps/github/event/issues_test.exs @@ -1,4 +1,4 @@ -defmodule CodeCorps.GitHub.Events.IssuesTest do +defmodule CodeCorps.GitHub.Event.IssuesTest do @moduledoc false use ExUnit.Case, aysnc: true @@ -7,7 +7,7 @@ defmodule CodeCorps.GitHub.Events.IssuesTest do alias CodeCorps.{ GithubEvent, - GitHub.Events.Issues + GitHub.Event.Issues } describe "handle/2" do diff --git a/test/lib/code_corps/github/event_test.exs b/test/lib/code_corps/github/event_test.exs new file mode 100644 index 000000000..689cb630b --- /dev/null +++ b/test/lib/code_corps/github/event_test.exs @@ -0,0 +1,37 @@ +defmodule CodeCorps.GitHub.EventTest do + @moduledoc false + + use CodeCorps.DbAccessCase + + import CodeCorps.Factories + + alias CodeCorps.{ + GithubEvent, + GitHub.Event + } + + describe "start_processing/1" do + test "sets event status to processing" do + event = insert(:github_event, status: "unprocessed") + {:ok, %GithubEvent{} = updated_event} = Event.start_processing(event) + assert updated_event.status == "processing" + end + end + + @ok_tuple {:ok, "foo"} + @error_tuple {:error, "bar"} + + describe "stop_processing/2" do + test "sets event status to 'processed' if ok tuple is the first argument" do + event = insert(:github_event, status: "processing") + {:ok, %GithubEvent{} = updated_event} = Event.stop_processing(@ok_tuple, event) + assert updated_event.status == "processed" + end + + test "sets event status to 'errored' if error tuple is the first argument" do + event = insert(:github_event, status: "processing") + {:ok, %GithubEvent{} = updated_event} = Event.stop_processing(@error_tuple, event) + assert updated_event.status == "errored" + end + end +end diff --git a/test/lib/code_corps/github/events/installation_repositories_test.exs b/test/lib/code_corps/github/events/installation_repositories_test.exs deleted file mode 100644 index e262c06cc..000000000 --- a/test/lib/code_corps/github/events/installation_repositories_test.exs +++ /dev/null @@ -1,19 +0,0 @@ -defmodule CodeCorps.GitHub.Events.InstallationRepositoriesTest do - @moduledoc false - - use ExUnit.Case, aysnc: true - - import CodeCorps.TestHelpers.GitHub - - alias CodeCorps.{ - GithubEvent, - GitHub.Events.InstallationRepositories - } - - describe "handle/2" do - test "is not implemented" do - payload = load_event_fixture("installation_repositories_removed") - assert InstallationRepositories.handle(%GithubEvent{}, payload) == :not_fully_implemented - end - end -end diff --git a/test/lib/code_corps/github/webhook/handler_test.exs b/test/lib/code_corps/github/webhook/handler_test.exs index fba9b2577..f84a6739f 100644 --- a/test/lib/code_corps/github/webhook/handler_test.exs +++ b/test/lib/code_corps/github/webhook/handler_test.exs @@ -4,6 +4,7 @@ defmodule CodeCorps.GitHub.Webhook.HandlerTest do use CodeCorps.DbAccessCase import CodeCorps.TestHelpers.GitHub + import CodeCorps.Factories alias CodeCorps.{ GithubEvent, @@ -12,7 +13,7 @@ defmodule CodeCorps.GitHub.Webhook.HandlerTest do } describe "handle" do - test "handles issues event" do + test "issues 'opened' event is supported but not implemented" do payload = load_event_fixture("issues_opened") assert Handler.handle("issues", "abc-123", payload) == :not_fully_implemented @@ -26,7 +27,7 @@ defmodule CodeCorps.GitHub.Webhook.HandlerTest do assert event.type == "issues" end - test "handles issue_comment event" do + test "issue_comment 'created' event is supported, but not implemented" do payload = load_event_fixture("issue_comment_created") assert Handler.handle("issue_comment", "abc-123", payload) == :not_fully_implemented @@ -40,21 +41,47 @@ defmodule CodeCorps.GitHub.Webhook.HandlerTest do assert event.type == "issue_comment" end - test "handles installation_repositories event" do - payload = load_event_fixture("installation_repositories_removed") + test "handles installation_repositories 'added' event" do + %{ + "installation" => %{ + "id" => installation_id + } + } = payload = load_event_fixture("installation_repositories_added") - assert Handler.handle("installation_repositories", "abc-123", payload) == :not_fully_implemented + insert(:github_app_installation, github_id: installation_id) + + assert Handler.handle("installation_repositories", "abc-123", payload) + + event = Repo.one(GithubEvent) + + assert event.action == "added" + assert event.github_delivery_id == "abc-123" + assert event.status == "processed" + assert event.source == "not implemented" + assert event.type == "installation_repositories" + end + + test "handles installation_repositories 'removed' event" do + %{ + "installation" => %{ + "id" => installation_id + } + } = payload = load_event_fixture("installation_repositories_removed") + + insert(:github_app_installation, github_id: installation_id) + + assert Handler.handle("installation_repositories", "abc-123", payload) event = Repo.one(GithubEvent) assert event.action == "removed" assert event.github_delivery_id == "abc-123" - assert event.status == "unprocessed" + assert event.status == "processed" assert event.source == "not implemented" assert event.type == "installation_repositories" end - test "handles installation event" do + test "handles installation 'created' event" do payload = load_event_fixture("installation_created") assert Handler.handle("installation", "abc-123", payload) diff --git a/test/models/github_repo_test.exs b/test/models/github_repo_test.exs new file mode 100644 index 000000000..53a12f9df --- /dev/null +++ b/test/models/github_repo_test.exs @@ -0,0 +1,14 @@ +defmodule CodeCorps.GithubRepoTest do + use CodeCorps.ModelCase + + alias CodeCorps.GithubRepo + + test "deletes associated ProjectGithubRepo records when deleting GithubRepo" do + github_repo = insert(:github_repo) + insert_pair(:project_github_repo, github_repo: github_repo) + + github_repo |> Repo.delete + + assert Repo.aggregate(GithubRepo, :count, :id) == 0 + end +end diff --git a/test/support/factories.ex b/test/support/factories.ex index 1f98720da..580861a7c 100644 --- a/test/support/factories.ex +++ b/test/support/factories.ex @@ -40,6 +40,10 @@ defmodule CodeCorps.Factories do %CodeCorps.GithubEvent{} end + def github_repo_factory do + %CodeCorps.GithubRepo{} + end + def organization_factory do %CodeCorps.Organization{ name: sequence(:username, &"Organization #{&1}"), @@ -103,6 +107,13 @@ defmodule CodeCorps.Factories do } end + def project_github_repo_factory do + %CodeCorps.ProjectGithubRepo{ + project: build(:project), + github_repo: build(:github_repo) + } + end + def role_factory do %CodeCorps.Role{ name: sequence(:name, &"Role #{&1}"), diff --git a/web/models/github_app_installation.ex b/web/models/github_app_installation.ex index c64b05ad4..b094e15e2 100644 --- a/web/models/github_app_installation.ex +++ b/web/models/github_app_installation.ex @@ -52,7 +52,6 @@ defmodule CodeCorps.GithubAppInstallation do current_state |> CodeCorps.Transition.GithubAppInstallationState.next(changed_state) case next_state do - nil -> changeset {:ok, next_state} -> cast(changeset, %{state: next_state}, [:state]) {:error, reason} -> add_error(changeset, :state, reason) end diff --git a/web/models/github_event.ex b/web/models/github_event.ex index f6132926e..31f4db2d5 100644 --- a/web/models/github_event.ex +++ b/web/models/github_event.ex @@ -1,6 +1,8 @@ defmodule CodeCorps.GithubEvent do use CodeCorps.Web, :model + @type t :: %__MODULE__{} + schema "github_events" do field :action, :string field :github_delivery_id, :string diff --git a/web/models/project_github_repo.ex b/web/models/project_github_repo.ex new file mode 100644 index 000000000..e9c566070 --- /dev/null +++ b/web/models/project_github_repo.ex @@ -0,0 +1,16 @@ +defmodule CodeCorps.ProjectGithubRepo do + @moduledoc """ + Represents a link between a Project and a GithubRepo. + """ + + use CodeCorps.Web, :model + + @type t :: %__MODULE__{} + + schema "project_github_repos" do + belongs_to :project, CodeCorps.Project + belongs_to :github_repo, CodeCorps.GithubRepo + + timestamps() + end +end