edisonywh/gearbox

Transitions changes the status in-memory, which will fail Ecto's dirty changeset tracking

edisonywh opened this issue · 1 comments

Apologies for the slight oversight, but I found a "bug" where Gearbox's transition/3 (and the bang variant) will change the status in memory (returning a new copy, with updated status).

While this is great for normal, pure Elixir usage, this will actually make working with Ecto a bit difficult.

For example:

Let's say I have an item, called Issue, and I want to transition the state from unread to read`.

  def update_issue_status(%Issue{} = issue, status) do
    issue
    |> Gearbox.transition!(IssueMachine, status)
    |> Issue.status_changeset(%{status: status})
    |> Repo.update()
  end

This would fail, because Gearbox.transition/3 will change the issue's status in memory, and when it's passed to changeset (and being casted), it'll be ignored, because there are no "changes". To illustrate the step by step flow of status:

  def update_issue_status(%Issue{} = issue, status) do
    issue # status is now `unread`
    |> Gearbox.transition!(IssueMachine, status) # status is now `read`
    |> Issue.status_changeset(%{status: status}) # Because `status` is updated to `read`, we are now telling Ecto to change from `status: 'read'` to `status: 'read'`, which will generate a changeset with no changes.
    |> Repo.update() # This will continue executing because changeset without changes is technically valid.
 
  end

I'm still thinking about how to deal with this, discussions are very welcomed. One possible way is to have a validate_transition API that just does validation without changing the structure in place, but that feels a little weird as well, because this may change the usage/mission of Gearbox (previously was intended to use it everywhere but your schema, but with the change above, it will now require users to use Gearbox at the schema levels. I'm not sure if this is the correct direction to take.

Temporary Fix:

You can temporarily circumvent this bug by making sure when you feed a structure into changeset, you feed the old version of it, so the Changeset will be able to generate a changeset.

I've found this to be a very useful library, and after seeing this limitation I created a branch that proposes a solution for this (a separate method that just returns a changeset). Please take a look to see if it fits, and for additional tests/documentation that can be added.

https://github.com/mauricew/gearbox/tree/ecto-changeset