Add pull_requests && done to task_list #1157
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| defmodule CodeCorps.Repo.Migrations.AddDoneToTaskList do | ||
| use Ecto.Migration | ||
|
|
||
| import Ecto.Query | ||
|
|
||
| alias CodeCorps.Repo | ||
|
|
||
| def up do | ||
| alter table(:task_lists) do | ||
| add :done, :boolean, default: false | ||
| end | ||
|
|
||
| flush() | ||
|
|
||
| from(tl in "task_lists", where: [name: "Done"], update: [set: [done: true]]) | ||
| |> Repo.update_all([]) | ||
|
|
||
| task_list_query = | ||
| from(tl in "task_lists", where: [done: true], select: [:id]) | ||
|
|
||
| # tests do not have any data, so we need to account for potential nil | ||
| case task_list_query |> Repo.one do | ||
| %{id: done_list_id} -> | ||
| task_update_query = from t in "tasks", | ||
| where: [status: "closed"], | ||
| update: [set: [task_list_id: ^done_list_id]] | ||
| task_update_query |> Repo.update_all([]) | ||
| nil -> nil | ||
| end | ||
| end | ||
|
|
||
| def down do | ||
| alter table(:task_lists) do | ||
| remove :done | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| defmodule CodeCorps.Repo.Migrations.AddPullRequestsToTaskList do | ||
| use Ecto.Migration | ||
|
|
||
| import Ecto.Query | ||
|
|
||
| alias CodeCorps.Repo | ||
|
|
||
| def up do | ||
| alter table(:task_lists) do | ||
| add :pull_requests, :boolean, default: false | ||
| end | ||
|
|
||
| flush() | ||
|
|
||
| # set all "In Progress" task lists to now contain pull requests | ||
| from( | ||
| tl in "task_lists", | ||
| where: [name: "In Progress"], | ||
| update: [set: [pull_requests: true]] | ||
| ) |> Repo.update_all([]) | ||
|
|
||
| # get projects paired with associated pull request task list as ids | ||
| task_parent_data = from( | ||
| p in "projects", | ||
| left_join: | ||
| tl in "task_lists", | ||
| on: tl.project_id == p.id, | ||
| where: tl.pull_requests == true, | ||
| select: {p.id, tl.id} | ||
| ) |> Repo.all | ||
|
|
||
| # get all tasks for projects, associated to github pull requests and | ||
| # assign them to the pull request task list | ||
| task_parent_data |> Enum.each(fn {project_id, pr_list_id} -> | ||
| from( | ||
| t in "tasks", | ||
| where: [project_id: ^project_id], | ||
| where: t.status != "closed", | ||
| where: not is_nil(t.github_issue_id), | ||
| inner_join: | ||
| gi in "github_issues", | ||
| on: t.github_issue_id == gi.id, | ||
| where: not is_nil(gi.github_pull_request_id), | ||
| update: [set: [task_list_id: ^pr_list_id]] | ||
| ) |> Repo.update_all([]) | ||
| end) | ||
| end | ||
|
|
||
| def down do | ||
| alter table(:task_lists) do | ||
| remove :pull_requests | ||
| end | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| defmodule CodeCorps.Repo.Migrations.AddUniqueConstraintsToSpecificTaskLists do | ||
| @moduledoc false | ||
|
|
||
| use Ecto.Migration | ||
|
|
||
| def change do | ||
| # There is already a "task_lists_project_id_index", so we name explicitly | ||
|
|
||
| create unique_index( | ||
| "task_lists", [:project_id], | ||
| where: "done = true", name: "task_lists_project_id_done_index") | ||
|
|
||
|
|
||
| create unique_index( | ||
| "task_lists", [:project_id], | ||
| where: "pull_requests = true", name: "task_lists_project_id_pull_requests_index") | ||
|
|
||
| create unique_index( | ||
| "task_lists", [:project_id], | ||
| where: "inbox = true", name: "task_lists_project_id_inbox_index") | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ defmodule CodeCorps.TaskListTest do | |
| use CodeCorps.ModelCase | ||
|
|
||
| alias CodeCorps.TaskList | ||
| alias Ecto.Changeset | ||
|
|
||
| @valid_attrs %{name: "some content", position: 42} | ||
| @invalid_attrs %{} | ||
|
|
@@ -16,12 +17,122 @@ defmodule CodeCorps.TaskListTest do | |
| refute changeset.valid? | ||
| end | ||
|
|
||
| test "defaults :done to 'false'" do | ||
| {:ok, record} = | ||
| %TaskList{} |> TaskList.changeset(@valid_attrs) |> Repo.insert | ||
| assert record.done == false | ||
| end | ||
|
|
||
| test "defaults :inbox to 'false'" do | ||
| {:ok, record} = | ||
| %TaskList{} |> TaskList.changeset(@valid_attrs) |> Repo.insert | ||
| assert record.inbox == false | ||
| end | ||
|
|
||
| test "defaults :pull_requests to 'false'" do | ||
| {:ok, record} = | ||
| %TaskList{} |> TaskList.changeset(@valid_attrs) |> Repo.insert | ||
| assert record.pull_requests == false | ||
|
Contributor
There was a problem hiding this comment. @snewcomer Your assertion here was |
||
| end | ||
|
|
||
| describe "create_changeset" do | ||
| test "casts done" do | ||
| attrs = @valid_attrs |> Map.merge(%{done: true}) | ||
| changeset = %TaskList{} |> TaskList.create_changeset(attrs) | ||
| assert changeset |> Changeset.get_change(:done) == true | ||
| end | ||
|
|
||
| test "casts inbox" do | ||
| attrs = @valid_attrs |> Map.merge(%{inbox: true}) | ||
| changeset = %TaskList{} |> TaskList.create_changeset(attrs) | ||
| assert changeset |> Changeset.get_change(:inbox) == true | ||
| end | ||
|
|
||
| test "casts pull_requests" do | ||
| attrs = @valid_attrs |> Map.merge(%{pull_requests: true}) | ||
| changeset = %TaskList{} |> TaskList.create_changeset(attrs) | ||
| assert changeset |> Changeset.get_change(:pull_requests) == true | ||
| end | ||
|
|
||
| test "requires done" do | ||
| attrs = @valid_attrs |> Map.merge(%{done: nil}) | ||
| changeset = %TaskList{} |> TaskList.create_changeset(attrs) | ||
| refute changeset.valid? | ||
| assert changeset |> Map.get(:errors) |> Keyword.get(:done) | ||
| end | ||
|
|
||
| test "requires inbox" do | ||
| attrs = @valid_attrs |> Map.merge(%{inbox: nil}) | ||
| changeset = %TaskList{} |> TaskList.create_changeset(attrs) | ||
| refute changeset.valid? | ||
| assert changeset |> Map.get(:errors) |> Keyword.get(:inbox) | ||
| end | ||
|
|
||
| test "requires pull_requests" do | ||
| attrs = @valid_attrs |> Map.merge(%{pull_requests: nil}) | ||
| changeset = %TaskList{} |> TaskList.create_changeset(attrs) | ||
| refute changeset.valid? | ||
| assert changeset |> Map.get(:errors) |> Keyword.get(:pull_requests) | ||
| end | ||
|
|
||
| test "ensures a unique 'done' task list per project" do | ||
| %{id: project_id} = insert(:project) | ||
| attrs = @valid_attrs |> Map.merge(%{done: true}) | ||
|
|
||
| {:ok, _task_list} = | ||
| %TaskList{} | ||
| |> TaskList.create_changeset(attrs) | ||
| |> Changeset.put_change(:project_id, project_id) | ||
| |> Repo.insert | ||
|
|
||
| {:error, changeset} = | ||
| %TaskList{} | ||
| |> TaskList.create_changeset(attrs) | ||
| |> Changeset.put_change(:project_id, project_id) | ||
| |> Repo.insert | ||
|
|
||
| refute changeset.valid? | ||
| assert changeset |> Map.get(:errors) |> Keyword.get(:done) | ||
| end | ||
|
|
||
| test "ensures a unique 'inbox' task list per project" do | ||
| %{id: project_id} = insert(:project) | ||
| attrs = @valid_attrs |> Map.merge(%{inbox: true}) | ||
|
|
||
| {:ok, _task_list} = | ||
| %TaskList{} | ||
| |> TaskList.create_changeset(attrs) | ||
| |> Changeset.put_change(:project_id, project_id) | ||
| |> Repo.insert | ||
|
|
||
| {:error, changeset} = | ||
| %TaskList{} | ||
| |> TaskList.create_changeset(attrs) | ||
| |> Changeset.put_change(:project_id, project_id) | ||
| |> Repo.insert | ||
|
|
||
| refute changeset.valid? | ||
| assert changeset |> Map.get(:errors) |> Keyword.get(:inbox) | ||
| end | ||
|
|
||
| test "ensures a unique 'pull_requests' task list per project" do | ||
| %{id: project_id} = insert(:project) | ||
| attrs = @valid_attrs |> Map.merge(%{pull_requests: true}) | ||
|
|
||
| {:ok, _task_list} = | ||
| %TaskList{} | ||
| |> TaskList.create_changeset(attrs) | ||
| |> Changeset.put_change(:project_id, project_id) | ||
| |> Repo.insert | ||
|
|
||
| {:error, changeset} = | ||
| %TaskList{} | ||
| |> TaskList.create_changeset(attrs) | ||
| |> Changeset.put_change(:project_id, project_id) | ||
| |> Repo.insert | ||
|
|
||
| refute changeset.valid? | ||
| assert changeset |> Map.get(:errors) |> Keyword.get(:pull_requests) | ||
| end | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshsmith I really feel this is the way to go with migrations.
If our schema modules do not match the schemas in any migration step, the code will fail, but if we go like this, "schemaless", the only case where the migration step would not match the actual state of the schema were if we somehow run it manually, out of order, which would be incorrect usage of the migration.
I understand using actual schema modules and changesets in a seed script, but they should not be used in migrations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately it looks like this will override some of the work that was done in the earlier migration that moved tasks to closed, because this is not checking for whether the issues are closed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshsmith You are correct about checking for gi not closed. You could also check for task not closed, it would likely serve the same purpose, since presumably, the task is otherwise in sync with the issue.
On a similar note, my other migration should probably add a clause for archived. I'm not sure what the exact query would be, but it would likely involve a
where: date_add(t.modified_at, 30, "day") > ^Date.utc_todaysomewhere in it.I can't push the change myself due to not being home, but that should help.