From ba0346b99ca567d0da349787b75823b8ef89e27a Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Wed, 15 Feb 2017 18:18:19 +0100 Subject: [PATCH] Add PATCH endpoint for user tasks --- .../controllers/user_task_controller_test.exs | 28 ++++++++++ test/models/user_task_test.exs | 21 ++++++++ test/policies/user_task_policy_test.exs | 52 ++++++++++++++++++- web/controllers/user_task_controller.ex | 7 ++- web/models/abilities.ex | 1 + web/models/user_task.ex | 22 ++++++-- web/policies/user_task_policy.ex | 9 ++++ web/router.ex | 2 +- 8 files changed, 135 insertions(+), 7 deletions(-) diff --git a/test/controllers/user_task_controller_test.exs b/test/controllers/user_task_controller_test.exs index 5f6789732..5042c8d1a 100644 --- a/test/controllers/user_task_controller_test.exs +++ b/test/controllers/user_task_controller_test.exs @@ -3,6 +3,8 @@ defmodule CodeCorps.UserTaskControllerTest do use CodeCorps.ApiCase, resource_name: :user_task + alias CodeCorps.{Repo, UserTask} + describe "index" do test "lists all entries on index", %{conn: conn} do [user_task_1, user_task_2] = insert_pair(:user_task) @@ -75,6 +77,32 @@ defmodule CodeCorps.UserTaskControllerTest do end end + describe "update" do + @tag :authenticated + test "updates chosen resource", %{conn: conn, current_user: current_user} do + task = insert(:task, user: current_user) + user_task = insert(:user_task, task: task) + new_user = insert(:user) + + assert conn |> request_update(user_task, %{user_id: new_user.id}) |> response(200) + + updated_task = Repo.get(UserTask, user_task.id) + assert updated_task.user_id == new_user.id + end + + test "renders 401 when unauthenticated", %{conn: conn} do + user_task = insert(:user_task) + new_user = insert(:user) + + assert conn |> request_update(user_task, %{user_id: new_user.id}) |> json_response(401) + end + + @tag :authenticated + test "renders 404 when id is nonexistent", %{conn: conn} do + assert conn |> request_update(:not_found) |> json_response(404) + end + end + describe "delete" do @tag :authenticated test "deletes chosen resource", %{conn: conn, current_user: current_user} do diff --git a/test/models/user_task_test.exs b/test/models/user_task_test.exs index a20cb5c26..2d068982e 100644 --- a/test/models/user_task_test.exs +++ b/test/models/user_task_test.exs @@ -40,4 +40,25 @@ defmodule CodeCorps.UserTaskTest do assert_error_message(response_changeset, :user, "has already been taken") end end + + describe "update_changeset/2" do + @required_attrs ~w(user_id) + + test "requires #{@required_attrs}" do + user_task = insert(:user_task) + + changeset = UserTask.update_changeset(user_task, %{user_id: nil}) + + assert_validation_triggered(changeset, :user_id, :required) + end + + test "ensures associated User record exists" do + user_task = insert(:user_task) + + changeset = UserTask.update_changeset(user_task, %{user_id: -1}) + + {:error, response_changeset} = Repo.update(changeset) + assert_error_message(response_changeset, :user, "does not exist") + end + end end diff --git a/test/policies/user_task_policy_test.exs b/test/policies/user_task_policy_test.exs index cadb0d57f..43775cc44 100644 --- a/test/policies/user_task_policy_test.exs +++ b/test/policies/user_task_policy_test.exs @@ -3,7 +3,7 @@ defmodule CodeCorps.UserTaskPolicyTest do use CodeCorps.PolicyCase - import CodeCorps.UserTaskPolicy, only: [create?: 2, delete?: 2] + import CodeCorps.UserTaskPolicy, only: [create?: 2, update?: 2, delete?: 2] import CodeCorps.UserTask, only: [create_changeset: 2] alias CodeCorps.UserTask @@ -79,6 +79,56 @@ defmodule CodeCorps.UserTaskPolicyTest do end end + describe "update?" do + test "returns false when user is not member of organization" do + {user, task} = generate_data_for("non-member") + + user_task = insert(:user_task, task: task) + + refute update?(user, user_task) + end + + test "returns false when user is pending member of organization" do + {user, task} = generate_data_for("pending") + + user_task = insert(:user_task, task: task) + + refute update?(user, user_task) + end + + test "returns true when user is contributor of organization" do + {user, task} = generate_data_for("contributor") + + user_task = insert(:user_task, task: task) + + assert update?(user, user_task) + end + + test "returns true when user is admin of organization" do + {user, task} = generate_data_for("admin") + + user_task = insert(:user_task, task: task) + + assert update?(user, user_task) + end + + test "returns true when user is owner of organization" do + {user, task} = generate_data_for("owner") + + user_task = insert(:user_task, task: task) + + assert update?(user, user_task) + end + + test "returns true when user is author of task" do + {user, task} = generate_data_for("author") + + user_task = insert(:user_task, task: task) + + assert update?(user, user_task) + end + end + describe "delete?" do test "returns false when user is not member of organization" do {user, task} = generate_data_for("non-member") diff --git a/web/controllers/user_task_controller.ex b/web/controllers/user_task_controller.ex index e0576c8b1..a23a336fc 100644 --- a/web/controllers/user_task_controller.ex +++ b/web/controllers/user_task_controller.ex @@ -8,7 +8,7 @@ defmodule CodeCorps.UserTaskController do plug :load_resource, model: UserTask, only: [:show], preload: [:task, :user] plug :load_and_authorize_changeset, model: UserTask, only: [:create] - plug :load_and_authorize_resource, model: UserTask, only: [:delete] + plug :load_and_authorize_resource, model: UserTask, only: [:update, :delete] plug JaResource @spec filter(Plug.Conn.t, Ecto.Query.t, String.t, String.t) :: Plug.Conn.t @@ -20,4 +20,9 @@ defmodule CodeCorps.UserTaskController do def handle_create(_conn, attributes) do %UserTask{} |> UserTask.create_changeset(attributes) end + + @spec handle_update(Plug.Conn.t, UserTask.t, map) :: Ecto.Changeset.t + def handle_update(_conn, user_task, attributes) do + user_task |> UserTask.update_changeset(attributes) + end end diff --git a/web/models/abilities.ex b/web/models/abilities.ex index bac39e08b..616c0a267 100644 --- a/web/models/abilities.ex +++ b/web/models/abilities.ex @@ -131,6 +131,7 @@ defmodule Canary.Abilities do def can?(%User{} = user, :delete, %UserSkill{} = user_skill), do: UserSkillPolicy.delete?(user, user_skill) def can?(%User{} = user, :create, %Changeset{data: %UserTask{}} = changeset), do: UserTaskPolicy.create?(user, changeset) + def can?(%User{} = user, :update, %UserTask{} = user_task), do: UserTaskPolicy.update?(user, user_task) def can?(%User{} = user, :delete, %UserTask{} = user_task), do: UserTaskPolicy.delete?(user, user_task) end end diff --git a/web/models/user_task.ex b/web/models/user_task.ex index b4e611bd3..a90f20cc1 100644 --- a/web/models/user_task.ex +++ b/web/models/user_task.ex @@ -14,8 +14,8 @@ defmodule CodeCorps.UserTask do timestamps() end - @permitted_attrs [:user_id, :task_id] - @required_attrs @permitted_attrs + @permitted_create_attrs [:user_id, :task_id] + @required_create_attrs @permitted_create_attrs @doc """ Builds a changeset used to insert a record into the database @@ -23,10 +23,24 @@ defmodule CodeCorps.UserTask do @spec create_changeset(CodeCorps.UserTask.t, map) :: Ecto.Changeset.t def create_changeset(struct, params \\ %{}) do struct - |> cast(params, @permitted_attrs) - |> validate_required(@required_attrs) + |> cast(params, @permitted_create_attrs) + |> validate_required(@required_create_attrs) |> assoc_constraint(:task) |> assoc_constraint(:user) |> unique_constraint(:user, name: :user_tasks_user_id_task_id_index) end + + @permitted_update_attrs [:user_id] + @required_update_attrs @permitted_update_attrs + + @doc """ + Builds a changeset used to update an existing record in the database + """ + @spec update_changeset(CodeCorps.UserTask.t, map) :: Ecto.Changeset.t + def update_changeset(struct, params \\ %{}) do + struct + |> cast(params, @permitted_update_attrs) + |> validate_required(@required_update_attrs) + |> assoc_constraint(:user) + end end diff --git a/web/policies/user_task_policy.ex b/web/policies/user_task_policy.ex index 1da74fcf6..a753cb6e4 100644 --- a/web/policies/user_task_policy.ex +++ b/web/policies/user_task_policy.ex @@ -25,6 +25,15 @@ defmodule CodeCorps.UserTaskPolicy do end end + @spec update?(User.t, UserTask.t) :: boolean + def update?(%User{} = user, %UserTask{} = user_task) do + cond do + user_task |> get_task |> get_project |> get_membership(user) |> get_role |> contributor_or_higher? -> true + user_task |> get_task |> task_authored_by?(user) -> true + true -> false + end + end + @spec delete?(User.t, UserTask.t) :: boolean def delete?(%User{} = user, %UserTask{} = user_task) do cond do diff --git a/web/router.ex b/web/router.ex index 5d52c045f..2c21a819e 100644 --- a/web/router.ex +++ b/web/router.ex @@ -83,7 +83,7 @@ defmodule CodeCorps.Router do resources "/user-categories", UserCategoryController, only: [:create, :delete] resources "/user-roles", UserRoleController, only: [:create, :delete] resources "/user-skills", UserSkillController, only: [:create, :delete] - resources "/user-tasks", UserTaskController, only: [:create, :delete] + resources "/user-tasks", UserTaskController, only: [:create, :update, :delete] end scope "/", CodeCorps, host: "api." do