Skip to content

Added field closed_at for Tasks and setup changeset accordingly #984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
joshsmith merged 1 commit into from
Sep 27, 2017
+139 −99
Merged

Added field closed_at for Tasks and setup changeset accordingly #984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions lib/code_corps/model/task.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ defmodule CodeCorps.Task do
import EctoOrdered

alias CodeCorps.Services.MarkdownRendererService
alias Ecto.Changeset

@type t :: %__MODULE__{}

schema "tasks" do
field :closed_at, :utc_datetime
Copy link
Contributor

@joshsmith joshsmith Sep 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I've seen utc_datetime before. Is this new in ecto?

Copy link
Contributor Author

@vishaldeepak vishaldeepak Sep 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old :datetime field now shows deprecation warning on migration. We need to use either naive_datetime or utc_datetime.

field :body, :string
field :markdown, :string
field :number, :integer, read_after_writes: true
Expand Down Expand Up @@ -58,6 +60,7 @@ defmodule CodeCorps.Task do
|> changeset(params)
|> cast(params, [:status])
|> validate_inclusion(:status, statuses())
|> set_closed_at()
Copy link
Contributor

@joshsmith joshsmith Sep 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand why you placed this in update but I do think since it's possible we'll need to write another issue to address this with GitHub sync. Can you open such an issue? We'd need to handle the closed event on GitHub and propagate that close to here. This could happen on create so I want to be sure we're not using the create changeset here.

Copy link
Contributor Author

@vishaldeepak vishaldeepak Sep 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure raised an issue. But I am unaware of GithubSync feel free to edit the issue -#992

end

def apply_position(changeset) do
Expand All @@ -71,4 +74,15 @@ defmodule CodeCorps.Task do
defp statuses do
~w{ open closed }
end

defp set_closed_at(changeset) do
case changeset do
%Changeset{valid?: true, changes: %{status: "closed"}} ->
put_change(changeset, :closed_at, DateTime.utc_now)
%Changeset{valid?: true, changes: %{status: "open"}} ->
put_change(changeset, :closed_at, nil)
_ ->
changeset
end
end
end
9 changes: 9 additions & 0 deletions priv/repo/migrations/20170927100300_add_closed_at_to_tasks.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
defmodule CodeCorps.Repo.Migrations.AddClosedAtToTasks do
use Ecto.Migration

def change do
alter table(:tasks) do
add :closed_at, :utc_datetime
end
end
end
Loading