From 1f4bd092dcd393c809bff08fb6bef55a6340ae9f Mon Sep 17 00:00:00 2001 From: Josh Smith Date: Sun, 19 Feb 2017 19:23:36 -0800 Subject: [PATCH] Remove task types --- blueprint/api.apib | 1 - .../analytics/segment_traits_builder.ex | 2 -- lib/code_corps/helpers/query.ex | 8 +---- ...0220032224_remove_task_type_from_tasks.exs | 9 ++++++ priv/repo/seeds.exs | 1 - priv/repo/structure.sql | 3 +- test/controllers/comment_controller_test.exs | 2 -- test/controllers/task_controller_test.exs | 28 ---------------- .../services/markdown_renderer_test.exs | 1 - test/models/task_test.exs | 12 +------ test/policies/task_policy_test.exs | 32 ++++++------------- test/support/factories.ex | 1 - test/views/task_view_test.exs | 1 - web/controllers/task_controller.ex | 3 +- web/models/task.ex | 10 ++---- web/policies/task_policy.ex | 15 +++------ web/views/task_view.ex | 2 +- 17 files changed, 29 insertions(+), 102 deletions(-) create mode 100644 priv/repo/migrations/20170220032224_remove_task_type_from_tasks.exs diff --git a/blueprint/api.apib b/blueprint/api.apib index 5410e6fd1..0a9d6e5cc 100644 --- a/blueprint/api.apib +++ b/blueprint/api.apib @@ -2759,7 +2759,6 @@ This endpoint allows you to check whether a username is valid (by running a vali + number: 1 (string) + status: `open` (string) + state: `published` (string) -+ `task-type`: `task` (string) + title: `Example task` (string) + `updated-at`: `2016-07-08T03:03:51.967Z` (string) diff --git a/lib/code_corps/analytics/segment_traits_builder.ex b/lib/code_corps/analytics/segment_traits_builder.ex index 41db92b7d..b510eb57d 100644 --- a/lib/code_corps/analytics/segment_traits_builder.ex +++ b/lib/code_corps/analytics/segment_traits_builder.ex @@ -26,7 +26,6 @@ defmodule CodeCorps.Analytics.SegmentTraitsBuilder do comment_id: comment.id, task: comment.task.title, task_id: comment.task.id, - task_type: comment.task.task_type, project_id: comment.task.project_id } end @@ -56,7 +55,6 @@ defmodule CodeCorps.Analytics.SegmentTraitsBuilder do %{ task: task.title, task_id: task.id, - task_type: task.task_type, project_id: task.project_id } end diff --git a/lib/code_corps/helpers/query.ex b/lib/code_corps/helpers/query.ex index eb090171a..f8cafe604 100644 --- a/lib/code_corps/helpers/query.ex +++ b/lib/code_corps/helpers/query.ex @@ -1,5 +1,5 @@ defmodule CodeCorps.Helpers.Query do - import CodeCorps.Helpers.String, only: [coalesce_id_string: 1, coalesce_string: 1] + import CodeCorps.Helpers.String, only: [coalesce_id_string: 1] import Ecto.Query, only: [where: 3, limit: 2, order_by: 2] def id_filter(query, id_list) do @@ -44,12 +44,6 @@ defmodule CodeCorps.Helpers.Query do end def task_list_filter(query, _), do: query - def task_type_filter(query, %{"task_type" => task_type_list}) do - task_types = task_type_list |> coalesce_string - query |> where([object], object.task_type in ^task_types) - end - def task_type_filter(query, _), do: query - def task_status_filter(query, %{"status" => status}) do query |> where([object], object.status == ^status) end diff --git a/priv/repo/migrations/20170220032224_remove_task_type_from_tasks.exs b/priv/repo/migrations/20170220032224_remove_task_type_from_tasks.exs new file mode 100644 index 000000000..28f380526 --- /dev/null +++ b/priv/repo/migrations/20170220032224_remove_task_type_from_tasks.exs @@ -0,0 +1,9 @@ +defmodule CodeCorps.Repo.Migrations.RemoveTaskTypeFromTasks do + use Ecto.Migration + + def change do + alter table(:tasks) do + remove :task_type + end + end +end diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index c90ed3f69..4d43141eb 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -303,7 +303,6 @@ cond do |> Task.create_changeset(%{ title: "test task #{i}", markdown: "test *body* #{i}", - task_type: Enum.random(~w{idea issue task}), status: "open", number: i, project_id: 1, diff --git a/priv/repo/structure.sql b/priv/repo/structure.sql index 4b763d9c1..3c8d5c9e6 100644 --- a/priv/repo/structure.sql +++ b/priv/repo/structure.sql @@ -1105,7 +1105,6 @@ CREATE TABLE tasks ( body text, markdown text, number integer NOT NULL, - task_type character varying(255) DEFAULT 'task'::character varying NOT NULL, state character varying(255) NOT NULL, status character varying(255) DEFAULT 'open'::character varying NOT NULL, title text NOT NULL, @@ -2520,5 +2519,5 @@ ALTER TABLE ONLY user_tasks -- PostgreSQL database dump complete -- -INSERT INTO "schema_migrations" (version) VALUES (20160723215749), (20160804000000), (20160804001111), (20160805132301), (20160805203929), (20160808143454), (20160809214736), (20160810124357), (20160815125009), (20160815143002), (20160816020347), (20160816034021), (20160817220118), (20160818000944), (20160818132546), (20160820113856), (20160820164905), (20160822002438), (20160822004056), (20160822011624), (20160822020401), (20160822044612), (20160830081224), (20160830224802), (20160911233738), (20160912002705), (20160912145957), (20160918003206), (20160928232404), (20161003185918), (20161019090945), (20161019110737), (20161020144622), (20161021131026), (20161031001615), (20161121005339), (20161121014050), (20161121043941), (20161121045709), (20161122015942), (20161123081114), (20161123150943), (20161124085742), (20161125200620), (20161126045705), (20161127054559), (20161205024856), (20161207112519), (20161209192504), (20161212005641), (20161214005935), (20161215052051), (20161216051447), (20161218005913), (20161219160401), (20161219163909), (20161220141753), (20161221085759), (20161226213600), (20161231063614), (20170102130055), (20170102181053), (20170104113708), (20170104212623), (20170104235423), (20170106013143), (20170115035159), (20170115230549), (20170121014100), (20170131234029), (20170201014901), (20170201025454), (20170201035458), (20170201183258); +INSERT INTO "schema_migrations" (version) VALUES (20160723215749), (20160804000000), (20160804001111), (20160805132301), (20160805203929), (20160808143454), (20160809214736), (20160810124357), (20160815125009), (20160815143002), (20160816020347), (20160816034021), (20160817220118), (20160818000944), (20160818132546), (20160820113856), (20160820164905), (20160822002438), (20160822004056), (20160822011624), (20160822020401), (20160822044612), (20160830081224), (20160830224802), (20160911233738), (20160912002705), (20160912145957), (20160918003206), (20160928232404), (20161003185918), (20161019090945), (20161019110737), (20161020144622), (20161021131026), (20161031001615), (20161121005339), (20161121014050), (20161121043941), (20161121045709), (20161122015942), (20161123081114), (20161123150943), (20161124085742), (20161125200620), (20161126045705), (20161127054559), (20161205024856), (20161207112519), (20161209192504), (20161212005641), (20161214005935), (20161215052051), (20161216051447), (20161218005913), (20161219160401), (20161219163909), (20161220141753), (20161221085759), (20161226213600), (20161231063614), (20170102130055), (20170102181053), (20170104113708), (20170104212623), (20170104235423), (20170106013143), (20170115035159), (20170115230549), (20170121014100), (20170131234029), (20170201014901), (20170201025454), (20170201035458), (20170201183258), (20170220032224); diff --git a/test/controllers/comment_controller_test.exs b/test/controllers/comment_controller_test.exs index 371d5b55a..abf975d20 100644 --- a/test/controllers/comment_controller_test.exs +++ b/test/controllers/comment_controller_test.exs @@ -56,7 +56,6 @@ defmodule CodeCorps.CommentControllerTest do comment_id: String.to_integer(json["data"]["id"]), task: task.title, task_id: task.id, - task_type: task.task_type, project_id: task.project_id } @@ -94,7 +93,6 @@ defmodule CodeCorps.CommentControllerTest do comment_id: comment.id, task: task.title, task_id: task.id, - task_type: task.task_type, project_id: task.project_id } diff --git a/test/controllers/task_controller_test.exs b/test/controllers/task_controller_test.exs index 765222e64..3f16fafd8 100644 --- a/test/controllers/task_controller_test.exs +++ b/test/controllers/task_controller_test.exs @@ -3,14 +3,12 @@ defmodule CodeCorps.TaskControllerTest do @valid_attrs %{ title: "Test task", - task_type: "issue", markdown: "A test task", status: "open" } @invalid_attrs %{ title: nil, - task_type: "issue", status: "nonexistent" } @@ -59,30 +57,6 @@ defmodule CodeCorps.TaskControllerTest do assert json["data"] |> Enum.count == 2 end - test "lists all tasks filtered by task_type", %{conn: conn} do - project_1 = insert(:project) - user = insert(:user) - insert(:task, task_type: "idea", project: project_1, user: user) - insert(:task, task_type: "issue", project: project_1, user: user) - insert(:task, task_type: "task", project: project_1, user: user) - - json = - conn - |> get("projects/#{project_1.id}/tasks?task_type=idea,issue") - |> json_response(200) - - assert json["data"] |> Enum.count == 2 - - task_types = - json["data"] - |> Enum.map(fn(task_json) -> task_json["attributes"] end) - |> Enum.map(fn(task_attributes) -> task_attributes["task-type"] end) - - assert task_types |> Enum.member?("issue") - assert task_types |> Enum.member?("idea") - refute task_types |> Enum.member?("task") - end - test "lists all tasks filtered by status", %{conn: conn} do project = insert(:project) task_1 = insert(:task, status: "open", project: project) @@ -150,7 +124,6 @@ defmodule CodeCorps.TaskControllerTest do tracking_properties = %{ task: @valid_attrs.title, task_id: String.to_integer(json["data"]["id"]), - task_type: @valid_attrs.task_type, project_id: project.id } @@ -179,7 +152,6 @@ defmodule CodeCorps.TaskControllerTest do tracking_properties = %{ task: task.title, task_id: task.id, - task_type: task.task_type, project_id: task.project.id } diff --git a/test/lib/code_corps/services/markdown_renderer_test.exs b/test/lib/code_corps/services/markdown_renderer_test.exs index dbcc128e6..362b884f6 100644 --- a/test/lib/code_corps/services/markdown_renderer_test.exs +++ b/test/lib/code_corps/services/markdown_renderer_test.exs @@ -8,7 +8,6 @@ defmodule CodeCorps.Services.MarkdownRendererServiceTest do @valid_attrs %{ title: "Test task", task_list_id: 1, - task_type: "issue", markdown: "A **strong** body", status: "open" } diff --git a/test/models/task_test.exs b/test/models/task_test.exs index c38243c0f..c9f6e4ebf 100644 --- a/test/models/task_test.exs +++ b/test/models/task_test.exs @@ -5,12 +5,9 @@ defmodule CodeCorps.TaskTest do @valid_attrs %{ title: "Test task", - task_type: "issue", markdown: "A test task" } - @invalid_attrs %{ - task_type: "nonexistent" - } + @invalid_attrs %{} describe "create/2" do test "is invalid with invalid attributes" do @@ -18,12 +15,6 @@ defmodule CodeCorps.TaskTest do refute changeset.valid? end - test "only allows specific values for task_type" do - changes = Map.put(@valid_attrs, :task_type, "nonexistent") - changeset = Task.changeset(%Task{}, changes) - refute changeset.valid? - end - test "renders body html from markdown" do user = insert(:user) project = insert(:project) @@ -47,7 +38,6 @@ defmodule CodeCorps.TaskTest do task_list = insert(:task_list) changeset = Task.create_changeset(%Task{}, %{ markdown: "some content", - task_type: "issue", title: "some content", project_id: project.id, user_id: user.id, diff --git a/test/policies/task_policy_test.exs b/test/policies/task_policy_test.exs index 57fe1b2a0..337f6f33f 100644 --- a/test/policies/task_policy_test.exs +++ b/test/policies/task_policy_test.exs @@ -11,28 +11,14 @@ defmodule CodeCorps.TaskPolicyTest do user = build(:user, admin: true) changeset = %Task{} |> create_changeset(%{}) - assert create?(user, changeset) + assert create?(user, changeset) end test "returns false when user is not the author" do user = build(:user) - changeset = %Task{} |> create_changeset(%{task_type: "issue", user_id: "other"}) + changeset = %Task{} |> create_changeset(%{user_id: "other"}) - refute create?(user, changeset) - end - - test "returns true when task is an issue" do - user = insert(:user) - changeset = %Task{} |> create_changeset(%{task_type: "issue", user_id: user.id}) - - assert create?(user, changeset) - end - - test "returns true when task is an idea" do - user = insert(:user) - changeset = %Task{} |> create_changeset(%{task_type: "idea", user_id: user.id}) - - assert create?(user, changeset) + refute create?(user, changeset) end test "returns true when user is at least contributor of organization" do @@ -42,19 +28,19 @@ defmodule CodeCorps.TaskPolicyTest do insert(:organization_membership, role: "contributor", member: user, organization: organization) - changeset = %Task{} |> create_changeset(%{project_id: project.id, task_type: "task", user_id: user.id}) + changeset = %Task{} |> create_changeset(%{project_id: project.id, user_id: user.id}) - assert create?(user, changeset) + assert create?(user, changeset) end - test "returns false when user is not contributor and type is 'task'" do + test "returns true when user is not contributor" do user = insert(:user) organization = insert(:organization) project = insert(:project, organization: organization) - changeset = %Task{} |> create_changeset(%{project_id: project.id, task_type: "task", user_id: user.id}) + changeset = %Task{} |> create_changeset(%{project_id: project.id, user_id: user.id}) - refute create?(user, changeset) + assert create?(user, changeset) end end @@ -63,7 +49,7 @@ defmodule CodeCorps.TaskPolicyTest do user = build(:user, admin: true) task = build(:task) - assert update?(user, task) + assert update?(user, task) end end end diff --git a/test/support/factories.ex b/test/support/factories.ex index da8fd722c..e8eafe260 100644 --- a/test/support/factories.ex +++ b/test/support/factories.ex @@ -48,7 +48,6 @@ defmodule CodeCorps.Factories do def task_factory do %CodeCorps.Task{ title: "Test task", - task_type: "issue", markdown: "A test task", status: "open", state: "published", diff --git a/test/views/task_view_test.exs b/test/views/task_view_test.exs index 895251329..13a06f08d 100644 --- a/test/views/task_view_test.exs +++ b/test/views/task_view_test.exs @@ -19,7 +19,6 @@ defmodule CodeCorps.TaskViewTest do "order" => task.order, "status" => task.status, "state" => task.state, - "task-type" => task.task_type, "title" => task.title, "updated-at" => task.updated_at }, diff --git a/web/controllers/task_controller.ex b/web/controllers/task_controller.ex index 0ce6dcd37..5d1244b3e 100644 --- a/web/controllers/task_controller.ex +++ b/web/controllers/task_controller.ex @@ -4,7 +4,7 @@ defmodule CodeCorps.TaskController do import CodeCorps.Helpers.Query, only: [ project_filter: 2, project_id_with_number_filter: 2, task_list_id_with_number_filter: 2, - sort_by_order: 1, task_list_filter: 2, task_type_filter: 2, task_status_filter: 2 + sort_by_order: 1, task_list_filter: 2, task_status_filter: 2 ] alias CodeCorps.Task @@ -18,7 +18,6 @@ defmodule CodeCorps.TaskController do Task |> project_filter(params) |> task_list_filter(params) - |> task_type_filter(params) |> task_status_filter(params) |> sort_by_order |> Repo.all diff --git a/web/models/task.ex b/web/models/task.ex index c59e721f9..e9283e668 100644 --- a/web/models/task.ex +++ b/web/models/task.ex @@ -14,7 +14,6 @@ defmodule CodeCorps.Task do field :order, :integer field :state, :string field :status, :string, default: "open" - field :task_type, :string field :title, :string field :position, :integer, virtual: true @@ -33,9 +32,8 @@ defmodule CodeCorps.Task do def changeset(struct, params \\ %{}) do struct - |> cast(params, [:title, :markdown, :task_type, :task_list_id, :position]) - |> validate_required([:title, :markdown, :task_list_id, :task_type]) - |> validate_inclusion(:task_type, task_types()) + |> cast(params, [:title, :markdown, :task_list_id, :position]) + |> validate_required([:title, :markdown, :task_list_id]) |> assoc_constraint(:task_list) |> apply_position() |> set_order(:position, :order, :task_list_id) @@ -69,10 +67,6 @@ defmodule CodeCorps.Task do end end - defp task_types do - ~w{ idea issue task } - end - defp statuses do ~w{ open closed } end diff --git a/web/policies/task_policy.ex b/web/policies/task_policy.ex index 9343c7c42..d5cfd332e 100644 --- a/web/policies/task_policy.ex +++ b/web/policies/task_policy.ex @@ -1,25 +1,18 @@ defmodule CodeCorps.TaskPolicy do import CodeCorps.Helpers.Policy, - only: [get_project: 1, get_membership: 2, get_role: 1, admin_or_higher?: 1, contributor_or_higher?: 1] + only: [get_project: 1, get_membership: 2, get_role: 1, admin_or_higher?: 1] alias CodeCorps.Task alias CodeCorps.User alias Ecto.Changeset - # TODO: Need to be able to see what resource is being created here - # Previously, any user could create issues and ideas, but only - # approved members of organization could create other task types def create?(%User{admin: true}, %Changeset{}), do: true - def create?(%User{} = user, %Changeset{changes: %{user_id: author_id, task_type: task_type}} = changeset) do + def create?(%User{} = user, %Changeset{changes: %{user_id: author_id}}) do cond do # can't create for some other user user.id != author_id -> false - # any registered user can create ideas or issues - task_type in ["idea", "issue"] -> true - # organization admin or higher can update other people's tasks - changeset |> get_project |> get_membership(user) |> get_role |> contributor_or_higher? -> true - # do not permit for any other case - true -> false + # permit any user to create + true -> true end end def create?(%User{}, %Changeset{}), do: false diff --git a/web/views/task_view.ex b/web/views/task_view.ex index ce00a4165..ae7fc3653 100644 --- a/web/views/task_view.ex +++ b/web/views/task_view.ex @@ -4,7 +4,7 @@ defmodule CodeCorps.TaskView do use CodeCorps.Web, :view use JaSerializer.PhoenixView - attributes [:body, :markdown, :number, :task_type, :status, :state, :title, :order, :inserted_at, :updated_at] + attributes [:body, :markdown, :number, :status, :state, :title, :order, :inserted_at, :updated_at] has_one :project, serializer: CodeCorps.ProjectView has_one :task_list, serializer: CodeCorps.TaskListView