Implemented handling of Issues webhook, for "opened", "closed", "edited", "reopened"

Author: begedin

lib/code_corps/adapter/map_transformer.ex

@@ -0,0 +1,86 @@
+defmodule CodeCorps.Adapter.MapTransformer do
+  @moduledoc ~S"""
+  Module used to transform maps for the purposes of various adapters used by the
+  application.
+  """
+
+  @typedoc ~S"""
+  A format representing how a single key should be mapped from a source. The
+  actual format is a 2 element tuple.
+
+  The first element is the destination key in the output map.
+
+  The second element is a list of keys representing the nested path to the key
+  in the source map.
+
+  For example, the tuple:
+
+  `{:target_path, ["nested", "path", "to", "source"]}`
+
+  Means that, from the source map, we need to take the nested value under
+  "nested" => "path" => "to" => "source" and then put it into the output map,
+  as a value for the key ":target_path".
+  """
+  @type key_mapping :: {atom, list[atom]}
+
+  @typedoc """
+
+  """
+  @type mapping :: list(key_mapping)
+
+  @doc ~S"""
+  Takes a source map and a list of tuples representing how the source map
+  should be transformed into a new map, then applies the mapping
+  operation on each field.
+  """
+  @spec transform(map, mapping) :: map
+  def transform(%{} = source_map, mapping) when is_list(mapping) do
+    mapping |> Enum.reduce(%{}, &map_field(&1, &2, source_map))
+  end
+
+  @spec map_field(key_mapping, map, map) :: map
+  defp map_field({target_field, source_path}, %{} = target_map, %{} = source_map) do
+    value = get_in(source_map, source_path)
+    target_map |> Map.put(target_field, value)
+  end
+
+  @doc ~S"""
+  Performs the inverse of `&transform/2`
+  """
+  @spec transform_inverse(map, mapping) :: map
+  def transform_inverse(%{} = map, mapping) when is_list(mapping) do
+    mapping |> Enum.reduce(%{}, &map_field_inverse(&1, &2, map))
+  end
+
+  @spec map_field_inverse(key_mapping, map, map) :: map
+  defp map_field_inverse({source_field, target_path}, target_map, source_map) do
+    value = source_map |> Map.get(source_field)
+    list = target_path |> Enum.reverse
+    result = put_value(list, value, %{})
+    deep_merge(target_map, result)
+  end
+
+  defp put_value(_, value, map) when is_nil(value), do: map
+  defp put_value([head | tail], value, map) do
+    new_value = Map.put(%{}, head, value)
+    put_value(tail, new_value, map)
+  end
+  defp put_value([], new_value, _map), do: new_value
+
+  defp deep_merge(left, right) do
+    Map.merge(left, right, &deep_resolve/3)
+  end
+
+  # Key exists in both maps, and both values are maps as well.
+  # These can be merged recursively.
+  defp deep_resolve(_key, left = %{}, right = %{}) do
+    deep_merge(left, right)
+  end
+
+  # Key exists in both maps, but at least one of the values is
+  # NOT a map. We fall back to standard merge behavior, preferring
+  # the value on the right.
+  defp deep_resolve(_key, _left, right) do
+    right
+  end
+end

lib/code_corps/github/adapters/github_repo.ex

@@ -1,22 +1,16 @@
 defmodule CodeCorps.GitHub.Adapters.GithubRepo do
-  def from_api(%{
-    "id" => github_id,
-    "name" => name,
-    "owner" => %{
-      "id" => github_account_id,
-      "avatar_url" => github_account_avatar_url,
-      "login" => github_account_login,
-      "type" => github_account_type
-    }
-  }) do
-    %{
-      github_id: github_id,
-      name: name,
-      github_account_id: github_account_id,
-      github_account_avatar_url: github_account_avatar_url,
-      github_account_login: github_account_login,
-      github_account_type: github_account_type
-    }
+
+  @mapping [
+    {:github_account_avatar_url, ["owner", "avatar_url"]},
+    {:github_account_id, ["owner", "id"]},
+    {:github_account_login, ["owner", "login"]},
+    {:github_account_type, ["owner", "type"]},
+    {:github_id, ["id"]},
+    {:name, ["name"]}
+  ]
+
+  @spec from_api(map) :: map
+  def from_api(%{} = payload) do
+    payload |> CodeCorps.Adapter.MapTransformer.transform(@mapping)
   end
-  def from_api(_), do: {:error, :invalid_repo_payload}
 end

lib/code_corps/github/adapters/task.ex

@@ -0,0 +1,18 @@
+defmodule CodeCorps.GitHub.Adapters.Task do
+  @moduledoc """
+  Used to adapt a GitHub issue payload into attributes for creating or updating
+  a `CodeCorps.Task`.
+  """
+
+  @mapping [
+    {:github_id, ["id"]},
+    {:title, ["title"]},
+    {:markdown, ["body"]},
+    {:status, ["state"]}
+  ]
+
+  @spec from_issue(map) :: map
+  def from_issue(%{} = payload) do
+    payload |> CodeCorps.Adapter.MapTransformer.transform(@mapping)
+  end
+end

lib/code_corps/github/adapters/user.ex

@@ -0,0 +1,17 @@
+defmodule CodeCorps.GitHub.Adapters.User do
+  @moduledoc """
+  Used to adapt a GitHub issue payload into attributes for creating or updating
+  a `CodeCorps.Task`.
+  """
+
+  @mapping [
+    {:github_username, ["login"]},
+    {:github_id, ["id"]},
+    {:github_avatar_url, ["avatar_url"]}
+  ]
+
+  @spec from_github_user(map) :: map
+  def from_github_user(%{} = payload) do
+    payload |> CodeCorps.Adapter.MapTransformer.transform(@mapping)
+  end
+end

lib/code_corps/github/event/installation/repos.ex

@@ -71,19 +71,12 @@ defmodule CodeCorps.GitHub.Event.Installation.Repos do
   end
 
   # transaction step 2
-  @spec adapt_api_repo_list(%{api_response: any}) :: list(map) | {:error, :invalid_repo_payload}
+  @spec adapt_api_repo_list(%{api_response: any}) :: {:ok, list(map)}
   defp adapt_api_repo_list(%{api_response: repositories}) do
     adapter_results = repositories |> Enum.map(&GithubRepoAdapter.from_api/1)
-    case adapter_results |>  Enum.all?(&valid?/1) do
-      true -> {:ok, adapter_results}
-      false -> {:error, :invalid_repo_payload}
-    end
+    {:ok, adapter_results}
   end
 
-  @spec valid?(any) :: boolean
-  defp valid?({:error, _}), do: false
-  defp valid?(_), do: true
-
   # transaction step 3
   @spec delete_repos(%{processing_installation: GithubAppInstallation.t, repo_attrs_list: list(map)}) :: aggregated_result
   defp delete_repos(%{
@@ -141,18 +134,28 @@ defmodule CodeCorps.GitHub.Event.Installation.Repos do
   @spec create(GithubAppInstallation.t, map) :: {:ok, GithubRepo.t}
   defp create(%GithubAppInstallation{} = installation, %{} = repo_attributes) do
     %GithubRepo{}
-    |> Changeset.change(repo_attributes)
+    |> changeset(repo_attributes)
     |> Changeset.put_assoc(:github_app_installation, installation)
     |> Repo.insert()
   end
 
   @spec update(GithubRepo.t, map) :: {:ok, GithubRepo.t}
   defp update(%GithubRepo{} = github_repo, %{} = repo_attributes) do
     github_repo
-    |> Changeset.change(repo_attributes)
+    |> changeset(repo_attributes)
     |> Repo.update()
   end
 
+  @spec changeset(GithubRepo.t, map) :: Changeset.t
+  defp changeset(%GithubRepo{} = github_repo, %{} = repo_attributes) do
+    github_repo
+    |> Changeset.change(repo_attributes)
+    |> Changeset.validate_required([
+      :github_id, :name, :github_account_id,
+      :github_account_avatar_url, :github_account_login, :github_account_type
+    ])
+  end
+
   # transaction step 5
   @spec mark_processed(%{processing_installation: GithubAppInstallation.t}) :: {:ok, GithubAppInstallation.t}
   defp mark_processed(%{processing_installation: %GithubAppInstallation{} = installation}) do

lib/code_corps/github/event/issues.ex

@@ -1,17 +1,132 @@
 defmodule CodeCorps.GitHub.Event.Issues do
-  @moduledoc """
+  @moduledoc ~S"""
   In charge of dealing with "Issues" GitHub Webhook events
+
+  https://developer.github.com/v3/activity/events/types/#issuesevent
   """
 
-  alias CodeCorps.GithubEvent
+  alias CodeCorps.{
+    GithubEvent,
+    GithubRepo,
+    GitHub.Event.Issues.ChangesetBuilder,
+    GitHub.Event.Issues.Validator,
+    GitHub.Event.Issues.UserLinker,
+    ProjectGithubRepo,
+    Repo,
+    Task,
+    User
+  }
+  alias Ecto.{Changeset, Multi}
+
+  @typep outcome :: {:ok, list(Task.t)} |
+                    {:error, :not_fully_implemented} |
+                    {:error, :unexpected_payload} |
+                    {:error, :unexpected_action} |
+                    {:error, :unmatched_repository}
+
+  @implemented_actions ~w(opened closed edited reopened)
+  @unimplemented_actions ~w(assigned unassigned milestoned demilestoned labeled unlabeled)
+
+  @doc ~S"""
+  Handles the "Issues" GitHub webhook
 
-  @doc """
-  Handles an "Issues" GitHub Webhook event
+  The process is as follows
+  - validate the payload is structured as expected
+  - try and find the appropriate `GithubRepo` record.
+  - for each `ProjectGithubRepo` belonging to that `Project`
+    - find or initialize a new `Task`
+    - try and find a `User`, associate `Task` with user
+    - commit the change as an insert or update action
 
-  The general idea is
-  - marked the passed in event as "processing"
-  - do the work
-  - marked the passed in event as "processed" or "errored"
+  Depending on the success of the process, the function will return one of
+  - `{:ok, list_of_tasks}`
+  - `{:error, :not_fully_implemented}` - while we're aware of this action, we have not implemented support for it yet
+  - `{:error, :unexpected_payload}` - the payload was not as expected
+  - `{:error, :unexpected_action}` - the action was not of type we are aware of
+  - `{:error, :unmatched_repository}` - the repository for this issue was not found
+
+  Note that it is also possible to have a matched GithubRepo, but with that
+  record not having any ProjectGithubRepo children. The outcome of that case
+  should NOT be an errored event, since it simply means that the GithubRepo
+  was not linked to a Project by the Project owner. This is allowed and
+  relatively common.
   """
-  def handle(%GithubEvent{}, %{}), do: {:error, :not_fully_implemented}
+  @spec handle(GithubEvent.t, map) :: outcome
+  def handle(%GithubEvent{action: action}, payload) when action in @implemented_actions do
+    case payload |> Validator.valid? do
+      true -> do_handle(payload)
+      false -> {:error, :unexpected_payload}
+    end
+  end
+  def handle(%GithubEvent{action: action}, _payload) when action in @unimplemented_actions do
+    {:error, :not_fully_implemented}
+  end
+  def handle(%GithubEvent{action: _action}, _payload), do: {:error, :unexpected_action}
+
+  @spec do_handle(map) :: {:ok, list(Task.t)} | {:error, :unmatched_repository}
+  defp do_handle(%{} = payload) do
+    multi =
+      Multi.new
+      |> Multi.run(:repo, fn _ -> find_repo(payload) end)
+      |> Multi.run(:user, fn _ -> UserLinker.find_or_create_user(payload) end)
+      |> Multi.run(:tasks, &sync_all(&1, payload))
+
+    case Repo.transaction(multi) do
+      {:ok, %{tasks: tasks}} -> {:ok, tasks}
+      {:error, :repo, :unmatched_project, _steps} -> {:ok, []}
+      {:error, _errored_step, error_response, _steps} -> {:error, error_response}
+    end
+  end
+
+  @spec find_repo(map) :: {:ok, GithubRepo.t} | {:error, :unmatched_repository} | {:error, :unmatched_project}
+  defp find_repo(%{"repository" => %{"id" => github_id}}) do
+    case GithubRepo |> Repo.get_by(github_id: github_id) |> Repo.preload(:project_github_repos) do
+      # a GithubRepo with at least some ProjectGithubRepo children
+      %GithubRepo{project_github_repos: [_ | _]} = github_repo -> {:ok, github_repo}
+      # a GithubRepo with no ProjectGithubRepo children
+      %GithubRepo{project_github_repos: []} -> {:error, :unmatched_project}
+      nil -> {:error, :unmatched_repository}
+    end
+  end
+
+  @spec sync_all(map, map) :: {:ok, list(Task.t)}
+  defp sync_all(
+    %{
+      repo: %GithubRepo{project_github_repos: project_github_repos},
+      user: %User{} = user
+    },
+    %{} = payload) do
+
+    project_github_repos
+    |> Enum.map(&sync(&1, user, payload))
+    |> aggregate()
+  end
+
+  @spec sync(ProjectGithubRepo.t, User.t, map) :: {:ok, ProjectGithubRepo.t} | {:error, Changeset.t}
+  defp sync(%ProjectGithubRepo{} = project_github_repo, %User{} = user, %{} = payload) do
+    project_github_repo
+    |> find_or_init_task(payload)
+    |> ChangesetBuilder.build_changeset(payload, project_github_repo, user)
+    |> commit()
+  end
+
+  @spec find_or_init_task(ProjectGithubRepo.t, map) :: Task.t
+  defp find_or_init_task(%ProjectGithubRepo{project_id: project_id}, %{"issue" => %{"id" => github_id}}) do
+    case Task |> Repo.get_by(github_id: github_id, project_id: project_id) do
+      nil -> %Task{}
+      %Task{} = task -> task
+    end
+  end
+
+  @spec commit(Changeset.t) :: {:ok, Task.t} | {:error, Changeset.t}
+  defp commit(%Changeset{data: %Task{id: nil}} = changeset), do: changeset |> Repo.insert
+  defp commit(%Changeset{} = changeset), do: changeset |> Repo.update
+
+  @spec aggregate(list({:ok, Task.t})) :: {:ok, list(Task.t)}
+  defp aggregate(results) do
+    results
+    |> Enum.map(&Tuple.to_list/1)
+    |> Enum.map(&List.last/1)
+    |> (fn tasks -> {:ok, tasks} end).()
+  end
 end

lib/code_corps/github/event/issues/changeset_builder.ex

@@ -0,0 +1,37 @@
+defmodule CodeCorps.GitHub.Event.Issues.ChangesetBuilder do
+  @moduledoc ~S"""
+  In charge of building a `Changeset` to update a `Task` with, when handling an
+  Issues webhook.
+  """
+
+  alias CodeCorps.{
+    GitHub.Event.Issues.StateMapper,
+    Services.MarkdownRendererService,
+    ProjectGithubRepo,
+    Task,
+    User
+  }
+  alias CodeCorps.GitHub.Adapters.Task, as: TaskAdapter
+  alias Ecto.Changeset
+
+  @doc ~S"""
+  Constructs a changeset for syncing a task when processing an Issues webhook
+  """
+  @spec build_changeset(Task.t, map, ProjectGithubRepo.t, User.t) :: Changeset.t
+  def build_changeset(
+    %Task{} = task,
+    %{"issue" => issue_attrs} = payload,
+    %ProjectGithubRepo{project_id: project_id},
+    %User{id: user_id}) do
+
+    task
+    |> Changeset.change(issue_attrs |> TaskAdapter.from_issue())
+    |> Changeset.put_change(:state, payload |> StateMapper.get_state())
+    |> MarkdownRendererService.render_markdown_to_html(:markdown, :body)
+    |> Changeset.put_change(:project_id, project_id)
+    |> Changeset.put_change(:user_id, user_id)
+    |> Changeset.validate_required([:project_id, :user_id, :markdown, :body, :title])
+    |> Changeset.assoc_constraint(:project)
+    |> Changeset.assoc_constraint(:user)
+  end
+end