Merge pull request #1079 from code-corps/1078-fix-order-from-github

Set order on tasks from issues/PRs coming from GitHub

Author: begedin

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

@@ -65,6 +65,7 @@ defmodule CodeCorps.GitHub.Event.Issues.ChangesetBuilder do
     |> Changeset.assoc_constraint(:project)
     |> Changeset.assoc_constraint(:task_list)
     |> Changeset.assoc_constraint(:user)
+    |> Task.order_task()
   end
 
   @update_attrs ~w(markdown modified_at status title)a
@@ -79,5 +80,6 @@ defmodule CodeCorps.GitHub.Event.Issues.ChangesetBuilder do
     |> Changeset.assoc_constraint(:github_repo)
     |> Changeset.assoc_constraint(:project)
     |> Changeset.assoc_constraint(:user)
+    |> Task.order_task()
   end
 end

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

@@ -65,6 +65,7 @@ defmodule CodeCorps.GitHub.Event.PullRequest.ChangesetBuilder do
     |> Changeset.assoc_constraint(:project)
     |> Changeset.assoc_constraint(:task_list)
     |> Changeset.assoc_constraint(:user)
+    |> Task.order_task()
   end
 
   @update_attrs ~w(markdown modified_at status title)a
@@ -79,5 +80,6 @@ defmodule CodeCorps.GitHub.Event.PullRequest.ChangesetBuilder do
     |> Changeset.assoc_constraint(:github_repo)
     |> Changeset.assoc_constraint(:project)
     |> Changeset.assoc_constraint(:user)
+    |> Task.order_task()
   end
 end

lib/code_corps/model/task.ex

@@ -44,9 +44,14 @@ defmodule CodeCorps.Task do
     |> cast(params, [:title, :markdown, :task_list_id, :position])
     |> validate_required([:title, :task_list_id])
     |> assoc_constraint(:task_list)
+    |> order_task()
+    |> MarkdownRendererService.render_markdown_to_html(:markdown, :body)
+  end
+
+  def order_task(changeset) do
+    changeset
     |> apply_position()
     |> set_order(:position, :order, :task_list_id)
-    |> MarkdownRendererService.render_markdown_to_html(:markdown, :body)
   end
 
   @spec create_changeset(struct, map) :: Ecto.Changeset.t

lib/code_corps/task/service.ex

@@ -22,6 +22,12 @@ defmodule CodeCorps.Task.Service do
   end
 
   @spec update(Task.t, map) :: {:ok, Task.t} | {:error, Changeset.t} | {:error, :github}
+  def update(%Task{github_issue_id: nil} = task, %{} = attributes) do
+    Multi.new
+    |> Multi.update(:task, task |> Task.update_changeset(attributes))
+    |> Repo.transaction
+    |> marshall_result()
+  end
   def update(%Task{} = task, %{} = attributes) do
     Multi.new
     |> Multi.update(:task, task |> Task.update_changeset(attributes))
@@ -32,6 +38,7 @@ defmodule CodeCorps.Task.Service do
 
   @spec marshall_result(tuple) :: {:ok, Task.t} | {:error, Changeset.t} | {:error, :github}
   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, result, _steps}) do
     Logger.info "An error occurred when creating/updating the task with the GitHub API"
@@ -50,7 +57,10 @@ defmodule CodeCorps.Task.Service do
   @preloads [:github_issue, [github_repo: :github_app_installation], :user, [project: :organization]]
 
   @spec create_on_github(Task.t) :: {:ok, Task.t} :: {:error, GitHub.api_error_struct}
-  defp create_on_github(%Task{github_repo_id: nil} = task), do: {:ok, task}
+  defp create_on_github(%Task{github_repo_id: nil} = task) do
+    # Don't create: no GitHub repo was selected
+    {:ok, task}
+  end
   defp create_on_github(%Task{github_repo: _} = task) do
     with %Task{github_repo: github_repo} = task <- task |> Repo.preload(@preloads),
          {:ok, payload} <- GitHub.Issue.create(task),

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

@@ -52,6 +52,7 @@ defmodule CodeCorps.GitHub.Event.Issues.ChangesetBuilderTest do
       assert get_change(changeset, :user_id) == user.id
 
       assert changeset.valid?
+      assert changeset.changes[:position]
     end
 
     test "validates that modified_at has not already happened" do

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

@@ -65,6 +65,7 @@ defmodule CodeCorps.GitHub.Event.IssuesTest do
         assert task.title == title
         assert task.github_issue.number == number
         assert task.status == "open"
+        assert task.order
       end)
     end
 
@@ -113,6 +114,7 @@ defmodule CodeCorps.GitHub.Event.IssuesTest do
         assert task.title == title
         assert task.github_issue.number == number
         assert task.status == "open"
+        assert task.order
       end)
 
       assert existing_task_id in (tasks |> Enum.map(&Map.get(&1, :id)))
@@ -182,6 +184,7 @@ defmodule CodeCorps.GitHub.Event.IssuesTest do
         assert task.title == title
         assert task.github_issue.number == number
         assert task.status == "closed"
+        assert task.order
       end)
     end
 
@@ -230,6 +233,7 @@ defmodule CodeCorps.GitHub.Event.IssuesTest do
         assert task.title == title
         assert task.github_issue.number == number
         assert task.status == "closed"
+        assert task.order
       end)
 
       assert existing_task_id in (tasks |> Enum.map(&Map.get(&1, :id)))
@@ -299,6 +303,7 @@ defmodule CodeCorps.GitHub.Event.IssuesTest do
         assert task.title == title
         assert task.github_issue.number == number
         assert task.status == "open"
+        assert task.order
       end)
     end
 
@@ -347,6 +352,7 @@ defmodule CodeCorps.GitHub.Event.IssuesTest do
         assert task.title == title
         assert task.github_issue.number == number
         assert task.status == "open"
+        assert task.order
       end)
 
       assert existing_task_id in (tasks |> Enum.map(&Map.get(&1, :id)))
@@ -416,6 +422,7 @@ defmodule CodeCorps.GitHub.Event.IssuesTest do
         assert task.title == title
         assert task.github_issue.number == number
         assert task.status == "open"
+        assert task.order
       end)
     end
 
@@ -464,6 +471,7 @@ defmodule CodeCorps.GitHub.Event.IssuesTest do
         assert task.title == title
         assert task.github_issue.number == number
         assert task.status == "open"
+        assert task.order
       end)
 
       assert existing_task_id in (tasks |> Enum.map(&Map.get(&1, :id)))

test/lib/code_corps/github/event/pull_request/changeset_builder_test.exs

@@ -52,6 +52,7 @@ defmodule CodeCorps.GitHub.Event.PullRequest.ChangesetBuilderTest do
       assert get_change(changeset, :user_id) == user.id
 
       assert changeset.valid?
+      assert changeset.changes[:position]
     end
 
     test "validates that modified_at has not already happened" do

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

@@ -113,6 +113,7 @@ defmodule CodeCorps.GitHub.Event.PullRequestTest do
         assert task.title == title
         assert task.github_pull_request.number == number
         assert task.status == "open"
+        assert task.order
       end)
 
       assert existing_task_id in (tasks |> Enum.map(&Map.get(&1, :id)))
@@ -182,6 +183,7 @@ defmodule CodeCorps.GitHub.Event.PullRequestTest do
         assert task.title == title
         assert task.github_pull_request.number == number
         assert task.status == "closed"
+        assert task.order
       end)
     end
 
@@ -230,6 +232,7 @@ defmodule CodeCorps.GitHub.Event.PullRequestTest do
         assert task.title == title
         assert task.github_pull_request.number == number
         assert task.status == "closed"
+        assert task.order
       end)
 
       assert existing_task_id in (tasks |> Enum.map(&Map.get(&1, :id)))
@@ -299,6 +302,7 @@ defmodule CodeCorps.GitHub.Event.PullRequestTest do
         assert task.title == title
         assert task.github_pull_request.number == number
         assert task.status == "open"
+        assert task.order
       end)
     end
 
@@ -347,6 +351,7 @@ defmodule CodeCorps.GitHub.Event.PullRequestTest do
         assert task.title == title
         assert task.github_pull_request.number == number
         assert task.status == "open"
+        assert task.order
       end)
 
       assert existing_task_id in (tasks |> Enum.map(&Map.get(&1, :id)))
@@ -416,6 +421,7 @@ defmodule CodeCorps.GitHub.Event.PullRequestTest do
         assert task.title == title
         assert task.github_pull_request.number == number
         assert task.status == "open"
+        assert task.order
       end)
     end
 
@@ -464,6 +470,7 @@ defmodule CodeCorps.GitHub.Event.PullRequestTest do
         assert task.title == title
         assert task.github_pull_request.number == number
         assert task.status == "open"
+        assert task.order
       end)
 
       assert existing_task_id in (tasks |> Enum.map(&Map.get(&1, :id)))

test/lib/code_corps/task/service_test.exs

@@ -137,6 +137,24 @@ defmodule CodeCorps.Task.ServiceTest do
       assert_received({:patch, "https://api.github.com/repos/foo/bar/issues/5", _headers, _body, _options})
     end
 
+    test "does not propagate changes to github if task is synced to github pull request" do
+      github_repo =
+        :github_repo
+        |> insert(github_account_login: "foo", name: "bar")
+
+      github_pull_request = insert(:github_pull_request, number: 5)
+      task = insert(:task, github_repo: github_repo, github_pull_request: github_pull_request)
+
+      {:ok, updated_task} = task |> Task.Service.update(@update_attrs)
+
+      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
+      assert updated_task.github_pull_request_id
+      assert updated_task.github_repo_id
+    end
+
     test "reports {:error, :github}, makes no changes at all if there is a github api error" do
       github_repo =
         :github_repo