Eliminated need for github repo as argument in task syncer

Author: begedin

lib/code_corps/github/event/common/repo_finder.ex

@@ -14,13 +14,10 @@ defmodule CodeCorps.GitHub.Event.Common.RepoFinder do
   - `{:error, :unmatched_project}` if record was found, but has no associated
     `ProjectGithubRepo` children
   """
-  @spec find_repo(map) :: {:ok, GithubRepo.t} | {:error, :unmatched_repository} | {:error, :unmatched_project}
+  @spec find_repo(map) :: {:ok, GithubRepo.t} | {:error, :unmatched_repository}
   def 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}
+    case GithubRepo |> Repo.get_by(github_id: github_id) do
+      %GithubRepo{} = github_repo -> {:ok, github_repo}
       nil -> {:error, :unmatched_repository}
     end
   end

lib/code_corps/github/event/issue_comment.ex

@@ -68,7 +68,7 @@ defmodule CodeCorps.GitHub.Event.IssueComment do
     |> Multi.run(:issue, fn %{repo: %GithubRepo{} = github_repo} -> github_repo |> Issues.IssueLinker.create_or_update_issue(issue_payload) end)
     |> Multi.run(:issue_user, fn %{issue: github_issue} -> github_issue |> Issues.UserLinker.find_or_create_user(payload) end)
     |> Multi.run(:comment_user, fn _ -> IssueComment.UserLinker.find_or_create_user(payload) end)
-    |> Multi.run(:tasks, fn %{issue: github_issue, repo: github_repo, issue_user: user} -> github_issue |> TaskSyncer.sync_all(github_repo, user, payload) end)
+    |> Multi.run(:tasks, fn %{issue: github_issue, issue_user: user} -> github_issue |> TaskSyncer.sync_all(user, payload) end)
     |> Multi.run(:comments, fn %{tasks: tasks, comment_user: user} -> CommentSyncer.sync_all(tasks, user, payload) end)
   end
   defp operational_multi(%{"action" => "deleted"} = payload) do

lib/code_corps/github/event/issues.ex

@@ -53,7 +53,7 @@ defmodule CodeCorps.GitHub.Event.Issues do
     |> Multi.run(:repo, fn _ -> RepoFinder.find_repo(payload) end)
     |> Multi.run(:issue, fn %{repo: github_repo} -> link_issue(github_repo, payload) end)
     |> Multi.run(:user, fn %{issue: github_issue} -> UserLinker.find_or_create_user(github_issue, payload) end)
-    |> Multi.run(:tasks, fn %{issue: github_issue, repo: github_repo, user: user} -> TaskSyncer.sync_all(github_issue, github_repo, user, payload) end)
+    |> Multi.run(:tasks, fn %{issue: github_issue, user: user} -> github_issue |> TaskSyncer.sync_all(user, payload) end)
     |> Repo.transaction
     |> marshall_result()
   end

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

@@ -16,12 +16,18 @@ defmodule CodeCorps.GitHub.Event.Issues.TaskSyncer do
                    {:error, {list(Task.t), list(Changeset.t)}}
 
   @doc """
-  When provided a `GithubRepo`, a `User` and a GitHub API payload, for each
-  `Project` associated to that `GithubRepo` via a `ProjectGithubRepo`, it
-  creates or updates a `Task` associated to the specified `User`.
+  When provided a `CodeCorps.GithubIssue`, 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(GithubIssue.t, GithubRepo.t, User.t, map) :: {:ok, list(Task.t)}
-  def sync_all(%GithubIssue{} = github_issue, %GithubRepo{project_github_repos: project_github_repos}, %User{} = user, %{} = payload) do
+  @spec sync_all(GithubIssue.t, User.t, map) :: {:ok, list(Task.t)}
+  def sync_all(%GithubIssue{} = github_issue, %User{} = user, %{} = payload) do
+
+    %GithubIssue{
+      github_repo: %GithubRepo{project_github_repos: project_github_repos}
+    } = github_issue |> Repo.preload(github_repo: :project_github_repos)
+
     project_github_repos
     |> Enum.map(&sync(github_issue, &1, user, payload))
     |> ResultAggregator.aggregate

test/lib/code_corps/github/event/common/repo_finder_test.exs

@@ -7,28 +7,23 @@ defmodule CodeCorps.GitHub.Event.Common.RepoFinderTest do
 
   @loadable_event_fixtures ~w(issue_comment_created issue_comment_edited issue_comment_deleted issues_closed issues_opened issues_edited issues_reopened)
 
-  describe "find_repo/1" do
-    @loadable_event_fixtures |> Enum.each(fn fixture ->
-      @fixture fixture
+  @loadable_event_fixtures |> Enum.each(fn fixture ->
+    @fixture fixture
 
-      setup do
-        {:ok, %{payload: load_event_fixture(@fixture)}}
-      end
+    setup do
+      {:ok, %{payload: load_event_fixture(@fixture)}}
+    end
 
-      test "loads properly for event fixture #{@fixture}", %{payload: payload} do
-        # if no repo locally, returns error
+    describe "finder_repo for #{@fixture}" do
+      test "returns error if no matched repository", %{payload: payload} do
         assert RepoFinder.find_repo(payload) == {:error, :unmatched_repository}
+      end
 
-        # if repo is found but is not connected to any projects, returns error
+      test "returns repository if matched, preloads project github repos", %{payload: payload} do
         github_repo = insert(:github_repo, github_id: payload["repository"]["id"])
-        assert RepoFinder.find_repo(payload) == {:error, :unmatched_project}
-
-        # returns repo if all is in order
-        %{id: project_github_repo_id} = insert(:project_github_repo, github_repo: github_repo)
-        {:ok, %{id: found_repo_id, project_github_repos: project_github_repos}} = RepoFinder.find_repo(payload)
+        {:ok, %{id: found_repo_id}} = RepoFinder.find_repo(payload)
         assert found_repo_id == github_repo.id
-        assert Enum.map(project_github_repos, &Map.get(&1, :id)) == [project_github_repo_id]
       end
-    end)
-  end
+    end
+  end)
 end

test/lib/code_corps/github/event/issue_comment_test.exs

@@ -27,21 +27,6 @@ defmodule CodeCorps.GitHub.Event.IssueCommentTest do
     describe "handle/1 for IssueComment::#{action}" do
       @payload load_event_fixture("issue_comment_#{action}")
 
-      test "with unmatched both users, passes with no changes made if no matching projects" do
-        %{
-          "issue" => %{"user" => %{"id" => issue_user_github_id}},
-          "comment" => %{"user" => %{"id" => comment_user_github_id}},
-          "repository" => %{"id" => repo_github_id}
-        } = @payload
-
-        insert(:github_repo, github_id: repo_github_id)
-        assert IssueComment.handle(@payload) == {:ok, []}
-        assert Repo.aggregate(Task, :count, :id) == 0
-        assert Repo.aggregate(Comment, :count, :id) == 0
-        refute Repo.get_by(User, github_id: issue_user_github_id)
-        refute Repo.get_by(User, github_id: comment_user_github_id)
-      end
-
       test "with unmatched both users, creates users, creates missing tasks, missing comments, for all projects connected with the github repo" do
         %{
           "issue" => %{
@@ -102,21 +87,6 @@ defmodule CodeCorps.GitHub.Event.IssueCommentTest do
         refute Repo.one(User)
       end
 
-      test "with matched issue user, unmatched comment_user, passes with no changes made if no matching projects" do
-        %{
-          "issue" => %{"user" => %{"id" => issue_user_github_id}},
-          "comment" => %{"user" => %{"id" => comment_user_github_id}},
-          "repository" => %{"id" => repo_github_id}
-        } = @payload
-
-        insert(:user, github_id: issue_user_github_id)
-        insert(:github_repo, github_id: repo_github_id)
-        assert IssueComment.handle(@payload) == {:ok, []}
-        assert Repo.aggregate(Task, :count, :id) == 0
-        assert Repo.aggregate(Comment, :count, :id) == 0
-        refute Repo.get_by(User, github_id: comment_user_github_id)
-      end
-
       test "with matched issue user, unmatched comment user, creates and updates tasks, comments and comment user, for each related project" do
         %{
           "issue" => %{
@@ -191,21 +161,6 @@ defmodule CodeCorps.GitHub.Event.IssueCommentTest do
         assert IssueComment.handle(@payload) == {:error, :repository_not_found}
       end
 
-      test "with unmatched issue user, matched comment_user, passes with no changes made if no matching projects" do
-        %{
-          "issue" => %{"user" => %{"id" => issue_user_github_id}},
-          "comment" => %{"user" => %{"id" => comment_user_github_id}},
-          "repository" => %{"id" => repo_github_id}
-        } = @payload
-
-        insert(:user, github_id: comment_user_github_id)
-        insert(:github_repo, github_id: repo_github_id)
-        assert IssueComment.handle(@payload) == {:ok, []}
-        assert Repo.aggregate(Task, :count, :id) == 0
-        assert Repo.aggregate(Comment, :count, :id) == 0
-        refute Repo.get_by(User, github_id: issue_user_github_id)
-      end
-
       test "with unmatched issue user, matched comment user, creates and updates tasks, comments and issue user, for each related project" do
         %{
           "issue" => %{
@@ -269,21 +224,6 @@ defmodule CodeCorps.GitHub.Event.IssueCommentTest do
         assert IssueComment.handle(@payload) == {:error, :repository_not_found}
       end
 
-      test "with matched issue and comment_user, passes with no changes made if no matching projects" do
-        %{
-          "issue" => %{"user" => %{"id" => issue_user_github_id}},
-          "comment" => %{"user" => %{"id" => comment_user_github_id}},
-          "repository" => %{"id" => repo_github_id}
-        } = @payload
-
-        insert(:user, github_id: comment_user_github_id)
-        insert(:user, github_id: issue_user_github_id)
-        insert(:github_repo, github_id: repo_github_id)
-        assert IssueComment.handle(@payload) == {:ok, []}
-        assert Repo.aggregate(Task, :count, :id) == 0
-        assert Repo.aggregate(Comment, :count, :id) == 0
-      end
-
       test "with matched issue and comment user, creates and updates tasks, comments, for each related project" do
         %{
           "issue" => %{

test/lib/code_corps/github/event/issues/task_syncer_test.exs

@@ -17,12 +17,12 @@ defmodule CodeCorps.GitHub.Event.Issues.TaskSyncerTest do
 
     test "creates missing, updates existing tasks for each project associated with the github repo" do
       user = insert(:user)
-      github_issue = insert(:github_issue)
-      github_repo = insert(:github_repo)
+      %{github_repo: github_repo} = github_issue = insert(:github_issue)
 
       %{"issue" => %{"body" => issue_body}} = @payload
 
-      [%{project: project_1}, _, _] = project_github_repos = insert_list(3, :project_github_repo, github_repo: github_repo)
+      [%{project: project_1}, _, _] = project_github_repos =
+        insert_list(3, :project_github_repo, github_repo: github_repo)
 
       task_1 = insert(:task, project: project_1, github_issue: github_issue, github_repo: github_repo, user: user)
 
@@ -33,11 +33,7 @@ defmodule CodeCorps.GitHub.Event.Issues.TaskSyncerTest do
         insert(:task_list, project: project, inbox: true)
       end)
 
-      github_repo = Repo.preload(github_repo, :project_github_repos)
-
-      {:ok, tasks} =
-        github_issue
-        |> TaskSyncer.sync_all(github_repo, user, @payload)
+      {:ok, tasks} = github_issue |> TaskSyncer.sync_all(user, @payload)
 
       assert Repo.aggregate(Task, :count, :id) == 3
 
@@ -52,20 +48,19 @@ defmodule CodeCorps.GitHub.Event.Issues.TaskSyncerTest do
     end
 
     test "fails on validation errors" do
-      github_issue = insert(:github_issue)
+      %{github_repo: github_repo} = github_issue = insert(:github_issue)
 
       bad_payload = @payload |> put_in(~w(issue title), nil)
 
-      %{project: project, github_repo: github_repo} = insert(:project_github_repo)
+      %{project: project} =
+        insert(:project_github_repo, github_repo: github_repo)
+
       %{user: user} = insert(:task, project: project, github_issue: github_issue, github_repo: github_repo)
 
       insert(:task_list, project: project, inbox: true)
 
-      github_repo = Repo.preload(github_repo, :project_github_repos)
-
       {:error, {tasks, errors}} =
-        github_issue
-        |> TaskSyncer.sync_all(github_repo, user, bad_payload)
+        github_issue |> TaskSyncer.sync_all(user, bad_payload)
 
       assert tasks |> Enum.count == 0
       assert errors |> Enum.count == 1

test/lib/code_corps/github/event/issues/user_linker_test.exs

@@ -26,11 +26,11 @@ defmodule CodeCorps.GitHub.Event.Issues.UserLinkerTest do
       } = @payload
 
       user = insert(:user)
-      github_issue = insert(:github_issue, number: number)
-      repo = insert(:github_repo, github_id: github_repo_id)
+      github_repo = insert(:github_repo, github_id: github_repo_id)
+      github_issue = insert(:github_issue, number: number, github_repo: github_repo)
       # multiple tasks, all with same user is ok
       insert_pair(
-        :task, user: user, github_repo: repo, github_issue: github_issue)
+        :task, user: user, github_repo: github_repo, github_issue: github_issue)
 
       {:ok, %User{} = returned_user} = UserLinker.find_or_create_user(github_issue, @payload)
 
@@ -43,10 +43,10 @@ defmodule CodeCorps.GitHub.Event.Issues.UserLinkerTest do
         "repository" => %{"id" => github_repo_id}
       } = @payload
 
-      github_issue = insert(:github_issue, number: number)
-      repo = insert(:github_repo, github_id: github_repo_id)
+      github_repo = insert(:github_repo, github_id: github_repo_id)
+      github_issue = insert(:github_issue, number: number, github_repo: github_repo)
       # multiple tasks, each with different user is not ok
-      insert_pair(:task, github_repo: repo, github_issue: github_issue)
+      insert_pair(:task, github_repo: github_repo, github_issue: github_issue)
 
       assert {:error, :multiple_users} ==
         UserLinker.find_or_create_user(github_issue, @payload)
@@ -61,17 +61,16 @@ defmodule CodeCorps.GitHub.Event.Issues.UserLinkerTest do
       {:ok, %User{} = returned_user} = UserLinker.find_or_create_user(github_issue, @payload)
 
       assert preinserted_user.id == returned_user.id
+      assert Repo.get_by(User, attributes)
+
 
-      assert Repo.one(User)
     end
 
     test "creates user if none is found by any other method" do
       %{"issue" => %{"number" => number}} = @payload
       github_issue = insert(:github_issue, number: number)
       {:ok, %User{} = returned_user} = UserLinker.find_or_create_user(github_issue, @payload)
 
-      assert Repo.one(User)
-
       created_attributes = UserAdapter.from_github_user(@user_payload)
       created_user = Repo.get_by(User, created_attributes)
       assert created_user.id == returned_user.id
@@ -105,8 +104,6 @@ defmodule CodeCorps.GitHub.Event.Issues.UserLinkerTest do
       github_issue = insert(:github_issue, number: number)
       {:ok, %User{} = returned_user} = UserLinker.find_or_create_user(github_issue, @bot_payload)
 
-      assert Repo.one(User)
-
       created_attributes = UserAdapter.from_github_user(@bot_user_payload)
       created_user = Repo.get_by(User, created_attributes)
       assert created_user.id == returned_user.id