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

Add GitHub pull request #1074

Merged
merged 2 commits into from
Oct 17, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
defmodule CodeCorps.GitHub.Adapters.GithubAppInstallation do
defmodule CodeCorps.GitHub.Adapters.AppInstallation do
@moduledoc """
Module used to convert GitHub payloads into attributes for a
`GithubAppInstallation`.
Expand Down
39 changes: 39 additions & 0 deletions lib/code_corps/github/adapters/pull_request.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
defmodule CodeCorps.GitHub.Adapters.PullRequest do

@mapping [
{:additions, ["additions"]},
{:body, ["body"]},
{:changed_files, ["changed_files"]},
{:closed_at, ["closed_at"]},
{:comments, ["comments"]},
{:comments_url, ["comments_url"]},
{:commits, ["commits"]},
{:commits_url, ["commits_url"]},
{:deletions, ["deletions"]},
{:diff_url, ["diff_url"]},
{:github_created_at, ["created_at"]},
{:github_id, ["id"]},
{:github_updated_at, ["updated_at"]},
{:html_url, ["html_url"]},
{:issue_url, ["issue_url"]},
{:locked, ["locked"]},
{:merge_commit_sha, ["merge_commit_sha"]},
{:mergeable_state, ["mergeable_state"]},
{:merged, ["merged"]},
{:merged_at, ["merged_at"]},
{:number, ["number"]},
{:patch_url, ["patch_url"]},
{:review_comment_url, ["review_comment_url"]},
{:review_comments, ["review_comments"]},
{:review_comments_url, ["review_comments_url"]},
{:state, ["state"]},
{:statuses_url, ["statuses_url"]},
{:title, ["title"]},
{:url, ["url"]}
]

@spec from_api(map) :: map
def from_api(%{} = payload) do
payload |> CodeCorps.Adapter.MapTransformer.transform(@mapping)
end
end
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
defmodule CodeCorps.GitHub.Adapters.GithubRepo do
defmodule CodeCorps.GitHub.Adapters.Repo do

@mapping [
{:github_account_avatar_url, ["owner", "avatar_url"]},
Expand Down
4 changes: 2 additions & 2 deletions lib/code_corps/github/event/installation/changeset_builder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ defmodule CodeCorps.GitHub.Event.Installation.ChangesetBuilder do
GithubAppInstallation,
User
}
alias CodeCorps.GitHub.Adapters.GithubAppInstallation, as: GithubAppInstallationAdapter
alias CodeCorps.GitHub.Adapters.AppInstallation, as: AppInstallationAdapter
alias Ecto.Changeset

@doc """
Expand All @@ -20,7 +20,7 @@ defmodule CodeCorps.GitHub.Event.Installation.ChangesetBuilder do
%GithubAppInstallation{} = github_app_installation,
%{} = payload) do

attrs = GithubAppInstallationAdapter.from_installation_event(payload)
attrs = AppInstallationAdapter.from_installation_event(payload)

github_app_installation
|> Changeset.change(attrs)
Expand Down
4 changes: 2 additions & 2 deletions lib/code_corps/github/event/installation/repos.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ defmodule CodeCorps.GitHub.Event.Installation.Repos do
Repo
}

alias CodeCorps.GitHub.Adapters.GithubRepo, as: GithubRepoAdapter
alias CodeCorps.GitHub.Adapters.Repo, as: RepoAdapter

alias Ecto.{Changeset, Multi}

Expand Down Expand Up @@ -51,7 +51,7 @@ defmodule CodeCorps.GitHub.Event.Installation.Repos do
# transaction step 2
@spec adapt_api_repo_list(map) :: {:ok, list(map)}
defp adapt_api_repo_list(%{api_response: repositories}) do
adapter_results = repositories |> Enum.map(&GithubRepoAdapter.from_api/1)
adapter_results = repositories |> Enum.map(&RepoAdapter.from_api/1)
{:ok, adapter_results}
end

Expand Down
96 changes: 96 additions & 0 deletions lib/code_corps/github/event/pull_request.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
defmodule CodeCorps.GitHub.Event.PullRequest do
@moduledoc ~S"""
In charge of handling a GitHub Webhook payload for the PullRequest event type

[https://developer.github.com/v3/activity/events/types/#pullrequestevent](https://developer.github.com/v3/activity/events/types/#pullrequestevent)
"""

@behaviour CodeCorps.GitHub.Event.Handler

alias CodeCorps.{
GitHub.Event.Common.RepoFinder,
GitHub.Event.PullRequest.PullRequestLinker,
GitHub.Event.PullRequest.TaskSyncer,
GitHub.Event.PullRequest.UserLinker,
GitHub.Event.PullRequest.Validator,
Repo,
Task
}
alias Ecto.Multi

@type outcome :: {:ok, list(Task.t)} |
{:error, :not_fully_implemented} |
{:error, :unexpected_action} |
{:error, :unexpected_payload} |
{:error, :repository_not_found} |
{:error, :validation_error_on_inserting_user} |
{:error, :multiple_github_users_matched_same_cc_user} |
{:error, :validation_error_on_syncing_tasks} |
{:error, :unexpected_transaction_outcome}

@doc ~S"""
Handles the "PullRequest" GitHub webhook

The process is as follows
- validate the payload is structured as expected
- validate the action is properly supported
- match payload with affected `CodeCorps.GithubRepo` record using
`CodeCorps.GitHub.Event.Common.RepoFinder`
- match with a `CodeCorps.User` using
`CodeCorps.GitHub.Event.PullRequest.UserLinker`
- for each `CodeCorps.ProjectGithubRepo` belonging to matched repo
- match and update, or create a `CodeCorps.Task` on the associated
`CodeCorps.Project`

If the process runs all the way through, the function will return an `:ok`
tuple with a list of affected (created or updated) tasks.

If it fails, it will instead return an `:error` tuple, where the second
element is the atom indicating a reason.
"""
@spec handle(map) :: outcome
def handle(payload) do
Multi.new
|> Multi.run(:payload, fn _ -> payload |> validate_payload() end)
|> Multi.run(:action, fn _ -> payload |> validate_action() end)
|> Multi.run(:repo, fn _ -> RepoFinder.find_repo(payload) end)
|> Multi.run(:pull_request, fn %{repo: github_repo} -> link_pull_request(github_repo, payload) end)
|> Multi.run(:user, fn %{pull_request: github_pull_request} -> UserLinker.find_or_create_user(github_pull_request, payload) end)
|> Multi.run(:tasks, fn %{pull_request: github_pull_request, user: user} -> github_pull_request |> TaskSyncer.sync_all(user, payload) end)
|> Repo.transaction
|> marshall_result()
end

@spec link_pull_request(GithubRepo.t, map) :: {:ok, GithubIssue.t} | {:error, Ecto.Changeset.t}
defp link_pull_request(github_repo, %{"pull_request" => attrs}) do
PullRequestLinker.create_or_update_pull_request(github_repo, attrs)
end

@spec marshall_result(tuple) :: tuple
defp marshall_result({:ok, %{tasks: tasks}}), do: {:ok, tasks}
defp marshall_result({:error, :payload, :invalid, _steps}), do: {:error, :unexpected_payload}
defp marshall_result({:error, :action, :not_fully_implemented, _steps}), do: {:error, :not_fully_implemented}
defp marshall_result({:error, :action, :unexpected_action, _steps}), do: {:error, :unexpected_action}
defp marshall_result({:error, :repo, :unmatched_project, _steps}), do: {:ok, []}
defp marshall_result({:error, :repo, :unmatched_repository, _steps}), do: {:error, :repository_not_found}
defp marshall_result({:error, :user, %Ecto.Changeset{}, _steps}), do: {:error, :validation_error_on_inserting_user}
defp marshall_result({:error, :user, :multiple_users, _steps}), do: {:error, :multiple_github_users_matched_same_cc_user}
defp marshall_result({:error, :tasks, {_tasks, _errors}, _steps}), do: {:error, :validation_error_on_syncing_tasks}
defp marshall_result({:error, _errored_step, _error_response, _steps}), do: {:error, :unexpected_transaction_outcome}

@implemented_actions ~w(opened closed edited reopened)
@unimplemented_actions ~w(assigned unassigned review_requested review_request_removed labeled unlabeled)

@spec validate_action(map) :: {:ok, :implemented} | {:error, :not_fully_implemented | :unexpected_action}
defp validate_action(%{"action" => action}) when action in @implemented_actions, do: {:ok, :implemented}
defp validate_action(%{"action" => action}) when action in @unimplemented_actions, do: {:error, :not_fully_implemented}
defp validate_action(_payload), do: {:error, :unexpected_action}

@spec validate_payload(map) :: {:ok, :valid} | {:error, :invalid}
defp validate_payload(%{} = payload) do
case payload |> Validator.valid? do
true -> {:ok, :valid}
false -> {:error, :invalid}
end
end
end
83 changes: 83 additions & 0 deletions lib/code_corps/github/event/pull_request/changeset_builder.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
defmodule CodeCorps.GitHub.Event.PullRequest.ChangesetBuilder do
@moduledoc ~S"""
In charge of building a `Changeset` to update a `Task` with, when handling an
PullRequest webhook.
"""

alias CodeCorps.{
GithubPullRequest,
ProjectGithubRepo,
Repo,
Services.MarkdownRendererService,
Task,
TaskList,
User,
Validators.TimeValidator
}
alias CodeCorps.GitHub.Adapters.Task, as: TaskAdapter
alias Ecto.Changeset

@doc ~S"""
Constructs a changeset for syncing a `Task` when processing a PullRequest
webhook.

The changeset can be used to create or update a `Task`
"""
@spec build_changeset(Task.t, map, GithubPullRequest.t, ProjectGithubRepo.t, User.t) :: Changeset.t
def build_changeset(
%Task{id: task_id} = task,
%{"pull_request" => pull_request_attrs},
%GithubPullRequest{} = github_pull_request,
%ProjectGithubRepo{} = project_github_repo,
%User{} = user) do

case is_nil(task_id) do
true -> create_changeset(task, pull_request_attrs, github_pull_request, project_github_repo, user)
false -> update_changeset(task, pull_request_attrs)
end
end

@create_attrs ~w(created_at markdown modified_at status title)a
@spec create_changeset(Task.t, map, GithubPullRequest.t, ProjectGithubRepo.t, User.t) :: Changeset.t
defp create_changeset(
%Task{} = task,
%{} = pull_request_attrs,
%GithubPullRequest{id: github_pull_request_id},
%ProjectGithubRepo{project_id: project_id, github_repo_id: github_repo_id},
%User{id: user_id}) do

%TaskList{id: task_list_id} =
TaskList |> Repo.get_by(project_id: project_id, inbox: true)

task
|> Changeset.cast(TaskAdapter.from_api(pull_request_attrs), @create_attrs)
|> MarkdownRendererService.render_markdown_to_html(:markdown, :body)
|> Changeset.put_change(:created_from, "github")
|> Changeset.put_change(:modified_from, "github")
|> Changeset.put_change(:github_pull_request_id, github_pull_request_id)
|> Changeset.put_change(:github_repo_id, github_repo_id)
|> Changeset.put_change(:project_id, project_id)
|> Changeset.put_change(:task_list_id, task_list_id)
|> Changeset.put_change(:user_id, user_id)
|> Changeset.validate_required([:project_id, :task_list_id, :title, :user_id])
|> Changeset.assoc_constraint(:github_pull_request)
|> Changeset.assoc_constraint(:github_repo)
|> Changeset.assoc_constraint(:project)
|> Changeset.assoc_constraint(:task_list)
|> Changeset.assoc_constraint(:user)
end

@update_attrs ~w(markdown modified_at status title)a
@spec update_changeset(Task.t, map) :: Changeset.t
defp update_changeset(%Task{} = task, %{} = pull_request_attrs) do
task
|> Changeset.cast(TaskAdapter.from_api(pull_request_attrs), @update_attrs)
|> MarkdownRendererService.render_markdown_to_html(:markdown, :body)
|> Changeset.put_change(:modified_from, "github")
|> TimeValidator.validate_time_after(:modified_at)
|> Changeset.validate_required([:project_id, :title, :user_id])
|> Changeset.assoc_constraint(:github_repo)
|> Changeset.assoc_constraint(:project)
|> Changeset.assoc_constraint(:user)
end
end
55 changes: 55 additions & 0 deletions lib/code_corps/github/event/pull_request/pull_request_linker.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
defmodule CodeCorps.GitHub.Event.PullRequest.PullRequestLinker do
@moduledoc ~S"""
In charge of finding a pull request to link with a `GithubPullRequest` record
when processing the PullRequest webhook.

The only entry point is `create_or_update_pull_request/1`.
"""

alias CodeCorps.{
GithubPullRequest,
GithubRepo,
Repo
}

alias CodeCorps.GitHub.Adapters.PullRequest, as: PullRequestAdapter

@typep linking_result :: {:ok, GithubPullRequest.t} |
{:error, Ecto.Changeset.t}

@doc ~S"""
Finds or creates a `GithubPullRequest` using the data in a GitHub PullRequest
payload.

The process is as follows:

- Search for the pull request in our database with the payload data.
- If we return a single `GithubPullRequest`, then the `GithubPullRequest`
should be updated.
- If there are no matching `GithubPullRequest` records, then a
`GithubPullRequest`should be created.
"""
@spec create_or_update_pull_request(GithubRepo.t, map) :: linking_result
def create_or_update_pull_request(%GithubRepo{} = github_repo, %{"id" => github_pull_request_id} = attrs) do
params = PullRequestAdapter.from_api(attrs)

case Repo.get_by(GithubPullRequest, github_id: github_pull_request_id) do
nil -> create_pull_request(github_repo, params)
%GithubPullRequest{} = pull_request -> update_pull_request(pull_request, params)
end
end

defp create_pull_request(%GithubRepo{id: github_repo_id}, params) do
params = Map.put(params, :github_repo_id, github_repo_id)

%GithubPullRequest{}
|> GithubPullRequest.create_changeset(params)
|> Repo.insert
end

defp update_pull_request(%GithubPullRequest{} = github_pull_request, params) do
github_pull_request
|> GithubPullRequest.update_changeset(params)
|> Repo.update
end
end
65 changes: 65 additions & 0 deletions lib/code_corps/github/event/pull_request/task_syncer.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
defmodule CodeCorps.GitHub.Event.PullRequest.TaskSyncer do
alias CodeCorps.{
GithubPullRequest,
GithubRepo,
GitHub.Event.Common.ResultAggregator,
GitHub.Event.PullRequest.ChangesetBuilder,
ProjectGithubRepo,
Task,
User,
Repo
}

alias Ecto.Changeset

@type outcome :: {:ok, list(Task.t)} |
{:error, {list(Task.t), list(Changeset.t)}}

@doc """
When provided a `CodeCorps.GithubPullRequest`, a `CodeCorps.User` and a
GitHub API payload, for each `CodeCorps.Project` associated to that
`CodeCorps.GithubRepo` via a `CodeCorps.ProjectGithubRepo`, it
creates or updates a `CodeCorps.Task`.
"""
@spec sync_all(GithubPullRequest.t, User.t, map) :: {:ok, list(Task.t)}
def sync_all(%GithubPullRequest{} = github_pull_request, %User{} = user, %{} = payload) do

%GithubPullRequest{
github_repo: %GithubRepo{project_github_repos: project_github_repos}
} = github_pull_request |> Repo.preload(github_repo: :project_github_repos)

project_github_repos
|> Enum.map(&sync(github_pull_request, &1, user, payload))
|> ResultAggregator.aggregate
end

@spec sync(GithubPullRequest.t, ProjectGithubRepo.t, User.t, map) :: {:ok, ProjectGithubRepo.t} | {:error, Changeset.t}
defp sync(%GithubPullRequest{} = github_pull_request, %ProjectGithubRepo{} = project_github_repo, %User{} = user, %{} = payload) do
project_github_repo
|> find_or_init_task(github_pull_request)
|> ChangesetBuilder.build_changeset(payload, github_pull_request, project_github_repo, user)
|> commit()
end

@spec find_or_init_task(ProjectGithubRepo.t, GithubPullRequest.t) :: Task.t
defp find_or_init_task(
%ProjectGithubRepo{project_id: project_id, github_repo_id: github_repo_id},
%GithubPullRequest{id: github_pull_request_id}
) do
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't deal with the key part we need here - the parsing of the PR body to inferr which tasks are to be linked.

Really, it's the core logic of that that we need to work out, I think.

  • parse body for closing keywords, extract github ids
  • if they exist, find github issues with those ids, associate with tasks belonging to those issues, dissasociate form tasks belonging to other github issues
  • if none are found, disassociate from associated tasks which do belong to github issues

The special case is when we get a pr created which does not close any issues. In that case, my suggestion is we

  • create a codecorps task, which is linked to the pr, but not to a github iusse
  • we leave that task associated from now on, regardless of new associations being added/removed

My philosophy is, that one was created on CC and can potentially hold information that will be lost, if we disassociate. Maybe at some point in the future, we allow "converting the PR to a github issue", which would associate that task with a github issue as well. At that point, it can behave like a normal task, but until that point, we shouldn't assume and try to avoid any automatons with it. If the PR has been linked to an issue since, let the user close the original task we've created if they want to.


query_params = [
github_pull_request_id: github_pull_request_id,
github_repo_id: github_repo_id,
project_id: project_id
]

case Task |> Repo.get_by(query_params) 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
end
Loading