Added field closed_at for Tasks and setup changeset accordingly #984
Conversation
| @type t :: %__MODULE__{} | ||
|
|
||
| schema "tasks" do | ||
| field :closed_at, :utc_datetime |
There was a problem hiding this comment.
I'm not sure I've seen utc_datetime before. Is this new in ecto?
There was a problem hiding this comment.
The old :datetime field now shows deprecation warning on migration. We need to use either naive_datetime or utc_datetime.
| |> changeset(params) | ||
| |> cast(params, [:status]) | ||
| |> validate_inclusion(:status, statuses()) | ||
| |> set_closed_at() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Sure raised an issue. But I am unaware of GithubSync feel free to edit the issue -#992
joshsmith
force-pushed
the
979-add-closed-at-task
branch
from
0889153 to
081f07e
Compare
joshsmith
force-pushed
the
979-add-closed-at-task
branch
from
081f07e to
14734c2
Compare
| test "only allows specific values for status" do | ||
| changes = Map.put(@valid_attrs, :status, "nonexistent") | ||
| changeset = Task.update_changeset(%Task{}, changes) | ||
| changeset = Task.update_changeset(%Task{task_list_id: 1}, changes) |
There was a problem hiding this comment.
Not setting a task_list_id would refute the changeset anyway, making the test invalid
2 participants
What's in this PR?
Also found a bug in a test case that I fixed
References
Fixes #979.