Push to production

lib/code_corps/github/sync/issue/github_issue/github_issue.ex

@@ -37,13 +37,21 @@ defmodule CodeCorps.GitHub.Sync.Issue.GithubIssue do
       |> GithubIssue.changeset(payload |> Adapters.Issue.to_issue())
       |> Changeset.put_assoc(:github_user, github_user)
       |> Changeset.put_assoc(:github_repo, github_repo)
-      |> Changeset.put_assoc(:github_pull_request, github_pull_request)
+      |> maybe_put_github_pull_request(github_pull_request)
       |> Repo.insert_or_update()
     else
       {:error, error} -> {:error, error}
     end
   end
 
+  @spec maybe_put_github_pull_request(Changeset.t, GithubPullRequest.t | nil) :: Changeset.t
+  defp maybe_put_github_pull_request(%Changeset{} = changeset, %GithubPullRequest{} = github_pull_request) do
+    changeset |> Changeset.put_assoc(:github_pull_request, github_pull_request)
+  end
+  defp maybe_put_github_pull_request(%Changeset{} = changeset, nil) do
+    changeset
+  end
+
   @spec find_or_init(map) :: GithubIssue.t
   defp find_or_init(%{"id" => github_id}) do
     case GithubIssue |> Repo.get_by(github_id: github_id) |> Repo.preload([:github_user, :github_repo, :github_pull_request]) do

lib/code_corps/task/service.ex

@@ -23,10 +23,12 @@ defmodule CodeCorps.Task.Service do
     :user
   ]
 
+  @type result :: {:ok, Task.t} | {:error, Changeset.t} | {:error, :github} | {:error, :task_not_found}
+
   @doc ~S"""
   Performs all actions involved in creating a task on a project
   """
-  @spec create(map) :: {:ok, Task.t} | {:error, Changeset.t} | {:error, :github}
+  @spec create(map) :: result
   def create(%{} = attributes) do
     Multi.new
     |> Multi.insert(:task, %Task{} |> Task.create_changeset(attributes))
@@ -38,22 +40,23 @@ defmodule CodeCorps.Task.Service do
     |> marshall_result()
   end
 
-  @spec update(Task.t, map) :: {:ok, Task.t} | {:error, Changeset.t} | {:error, :github}
+  @spec update(Task.t, map) :: result
   def update(%Task{} = task, %{} = attributes) do
     Multi.new
     |> Multi.update(:task, task |> Task.update_changeset(attributes))
     |> Multi.run(:preload, fn %{task: %Task{} = task} ->
          {:ok, task |> Repo.preload(@preloads)}
        end)
     |> Multi.run(:github, (fn %{preload: %Task{} = task} -> task |> update_on_github() end))
-    |> Repo.transaction
+    |> Repo.transaction()
     |> marshall_result()
   end
 
-  @spec marshall_result(tuple) :: {:ok, Task.t} | {:error, Changeset.t} | {:error, :github}
+  @spec marshall_result(tuple) :: result
   defp marshall_result({:ok, %{github: %Task{} = task}}), do: {:ok, task}
   defp marshall_result({:ok, %{task: %Task{} = task}}), do: {:ok, task}
   defp marshall_result({:error, :task, %Changeset{} = changeset, _steps}), do: {:error, changeset}
+  defp marshall_result({:error, :github, {:error, :task_not_found}, _steps}), do: {:error, :task_not_found}
   defp marshall_result({:error, :github, result, _steps}) do
     Logger.info "An error occurred when creating/updating the task with the GitHub API"
     Logger.info "#{inspect result}"
@@ -70,7 +73,7 @@ defmodule CodeCorps.Task.Service do
          {:ok, %GithubIssue{} = github_issue} <-
            payload
            |> Sync.Issue.GithubIssue.create_or_update_issue(github_repo) do
-      task |> link_with_github_changeset(github_issue) |> Repo.update
+      task |> link_with_github_changeset(github_issue) |> Repo.update()
     else
       {:error, error} -> {:error, error}
     end
@@ -81,16 +84,16 @@ defmodule CodeCorps.Task.Service do
     task |> Changeset.change(%{github_issue: github_issue})
   end
 
-  @spec update_on_github(Task.t) :: {:ok, Task.t} | {:error, Changeset.t} | {:error, GitHub.api_error_struct}
+  @spec update_on_github(Task.t) :: {:ok, Task.t} | {:error, Changeset.t} | {:error, GitHub.api_error_struct} | {:error, :task_not_found}
   defp update_on_github(%Task{github_repo_id: nil, github_issue_id: nil} = task), do: {:ok, task}
   defp update_on_github(%Task{github_repo_id: _, github_issue_id: nil} = task), do: task |> create_on_github()
   defp update_on_github(%Task{github_repo: github_repo} = task) do
     with {:ok, payload} <- GitHub.API.Issue.update(task),
-         {:ok, %GithubIssue{}} <-
-           payload
-           |> Sync.Issue.GithubIssue.create_or_update_issue(github_repo) do
-      {:ok, Task |> Repo.get(task.id)}
+         {:ok, %GithubIssue{}} <- payload |> Sync.Issue.GithubIssue.create_or_update_issue(github_repo),
+         %Task{} = task <- Repo.get(Task, task.id) do
+      {:ok, task}
     else
+      nil -> {:error, :task_not_found}
       {:error, error} -> {:error, error}
     end
   end

lib/code_corps_web/controllers/github_event_controller.ex

@@ -1,9 +1,11 @@
 defmodule CodeCorpsWeb.GithubEventController do
   @moduledoc false
   use CodeCorpsWeb, :controller
+  import Ecto.Query, only: [from: 2]
 
   alias CodeCorps.{
     GithubEvent,
+    GithubRepo,
     GitHub.Webhook.Handler,
     GitHub.Webhook.EventSupport,
     Helpers.Query,
@@ -43,14 +45,29 @@ defmodule CodeCorpsWeb.GithubEventController do
 
   @spec create(Conn.t, map) :: Conn.t
   def create(%Conn{} = conn, %{} = payload) do
-    type = conn |> get_event_type
-    delivery_id = conn |> get_delivery_id()
+    type = conn |> event_type()
+    delivery_id = conn |> delivery_id()
     action = payload |> Map.get("action", "")
-    event_support = type |> EventSupport.status(action)
-    event_support |> process_event(type, delivery_id, payload)
+    event_support =
+      if should_process?(payload) do
+        process_status = type |> EventSupport.status(action)
+        process_status |> process_event(type, delivery_id, payload)
+        process_status
+      else
+        :ignored
+      end
     conn |> respond_to_webhook(event_support)
   end
 
+  @spec should_process?(map) :: boolean
+  defp should_process?(%{"repository" => %{"id" => repository_id}}) do
+    query = from repo in GithubRepo,
+      where: repo.github_id == ^repository_id,
+      where: not(is_nil(repo.project_id))
+    Repo.one(query) != nil
+  end
+  defp should_process?(_), do: true
+
   @spec update(Conn.t, map) :: Conn.t
   def update(%Conn{} = conn, %{"id" => id} = params) do
     with %GithubEvent{} = github_event <- GithubEvent |> Repo.get(id),
@@ -63,13 +80,13 @@ defmodule CodeCorpsWeb.GithubEventController do
     end
   end
 
-  @spec get_event_type(Conn.t) :: String.t
-  defp get_event_type(%Conn{} = conn) do
+  @spec event_type(Conn.t) :: String.t
+  defp event_type(%Conn{} = conn) do
     conn |> get_req_header("x-github-event") |> List.first
   end
 
-  @spec get_delivery_id(Conn.t) :: String.t
-  defp get_delivery_id(%Conn{} = conn) do
+  @spec delivery_id(Conn.t) :: String.t
+  defp delivery_id(%Conn{} = conn) do
     conn |> get_req_header("x-github-delivery") |> List.first
   end
 

test/lib/code_corps/skills/skills_test.exs

@@ -1,4 +1,4 @@
-defmodule CodeCorps.AccountsTest do
+defmodule CodeCorps.SkillsTest do
   @moduledoc false
 
   use CodeCorps.DbAccessCase

test/lib/code_corps/task/service_test.exs

@@ -5,14 +5,16 @@ defmodule CodeCorps.Task.ServiceTest do
 
   import CodeCorps.GitHub.TestHelpers
 
-  alias CodeCorps.Task
+  alias CodeCorps.{GithubIssue, Repo, Task}
 
   @base_attrs %{
     "title" => "Test task",
     "markdown" => "A test task",
     "status" => "open"
   }
 
+  @issue_payload load_endpoint_fixture("issue")
+
   defp valid_attrs() do
     project = insert(:project)
     task_list = insert(:task_list, project: project, inbox: true)
@@ -106,7 +108,7 @@ defmodule CodeCorps.Task.ServiceTest do
       assert updated_task.id == task.id
       assert updated_task.title == @update_attrs["title"]
       assert updated_task.markdown == @update_attrs["markdown"]
-      assert updated_task.body != task.body
+      refute updated_task.body == task.body
       refute task.github_issue_id
       refute task.github_repo_id
 
@@ -159,13 +161,40 @@ defmodule CodeCorps.Task.ServiceTest do
       assert updated_task.id == task.id
       assert updated_task.title == @update_attrs["title"]
       assert updated_task.markdown == @update_attrs["markdown"]
-      assert updated_task.body != task.body
+      refute updated_task.body == task.body
       assert updated_task.github_issue_id
       assert updated_task.github_repo_id
 
       assert_received({:patch, "https://api.github.com/repos/foo/bar/issues/5", _body, _headers, _options})
     end
 
+    test "propagates changes to github if task is synced to github pull request" do
+      %{
+        "id" => issue_github_id,
+        "number" => number
+      } = @issue_payload
+
+      github_repo = insert(:github_repo, github_account_login: "octocat", name: "Hello-World")
+      github_pull_request = insert(:github_pull_request)
+      github_issue = insert(:github_issue, github_id: issue_github_id, number: number, github_pull_request: github_pull_request, github_repo: github_repo)
+      task = insert(:task, github_repo: github_repo, github_issue: github_issue)
+
+      {:ok, updated_task} = task |> Task.Service.update(@update_attrs)
+
+      assert_received({:patch, "https://api.github.com/repos/octocat/Hello-World/issues/1347", _body, _headers, _options})
+
+      assert updated_task.id == task.id
+      assert updated_task.title == @update_attrs["title"]
+      assert updated_task.markdown == @update_attrs["markdown"]
+      refute updated_task.body == task.body
+      assert updated_task.github_issue_id == github_issue.id
+      assert updated_task.github_repo_id == github_repo.id
+
+      updated_github_issue = Repo.one(GithubIssue)
+
+      assert updated_github_issue.github_pull_request_id == github_pull_request.id
+    end
+
     test "reports {:error, :github}, makes no changes at all if there is a github api error" do
       github_repo =
         :github_repo

test/lib/code_corps_web/controllers/github_event_controller_test.exs

@@ -101,11 +101,22 @@ defmodule CodeCorpsWeb.GithubEventControllerTest do
     test "responds with 200 for an unsupported event", %{conn: conn} do
       path = conn |> github_events_path(:create)
       payload = load_event_fixture("pull_request_synchronize")
+      insert(:github_repo, github_id: payload["repository"]["id"])
       assert conn |> for_event("pull_request", "foo") |> post(path, payload) |> response(200)
 
       assert Repo.get_by(GithubEvent, github_delivery_id: "foo")
     end
 
+    @tag :github_webhook
+    test "responds with 202 for a supported event but no project_id", %{conn: conn} do
+      path = conn |> github_events_path(:create)
+      payload = load_event_fixture("pull_request_synchronize")
+      insert(:github_repo, github_id: payload["repository"]["id"], project: nil)
+      assert conn |> for_event("pull_request", "foo") |> post(path, payload) |> response(202)
+
+      refute Repo.get_by(GithubEvent, github_delivery_id: "foo")
+    end
+
     @tag :github_webhook
     test "responds with 202 for an unknown event", %{conn: conn} do
       path = conn |> github_events_path(:create)