ash-project / ash_state_machine

The extension for building state machines with Ash resources.
https://hexdocs.pm/ash_state_machine
MIT License
19 stars 8 forks source link

Getting attempted to transition to an unknown state using forms auto #70

Closed MikaelFangel closed 5 days ago

MikaelFangel commented 1 week ago

When using form auto and I try to edit it using the state machine, I get an error message which I have attached. The error message is about a missing transition. My current solution is to add nil as an extra_state which doesn't seem like the right solution to this error message, as the attribute I'm defining it on uses a one_of constraint and has allow_nil? false defined.

The error message:

[error] Attempted to transition to an unknown state.

This usually means that one of the following is true:

* You have a missing transition definition in your state machine

  To remediate this, add a transition.

* You are using `:*` to include a state that appears nowhere in the state machine definition

  To remediate this, add the `extra_states` option and include the state nil

This issue refers to this discussion on the elixir forum:

zachdaniel commented 6 days ago

Actually, I'm curious about how you have nil state here. Are you explicitly accepting state as an input? May be best to make a small demo of this issue.

MikaelFangel commented 6 days ago

I made a small example that can reproduce the error I'm seeing while still trying to look similar to my implementation. The reproducible example can be found here: https://github.com/MikaelFangel/ash_state_machine_bug.

To reproduce:

  1. Run migrations
  2. Run phx.server
  3. Go to localhost:4000/
  4. Create a ticket and set any state
  5. Save the ticket
  6. You'll now be redirected to the update form and see the error in the debug messages.

I hope that my example will help to find the bug or just point towards that I'm doing something incorrectly.

zachdaniel commented 5 days ago

So, the only issue that I found in ash_state_machine itself was handling the case where state was transitioning to/from nil. The other issues were issues in your reproduction. Comments indicate what needed to change.

1.

  @impl true
  # params are in the `"form"` key
  def handle_event("validate", %{"form" => params}, socket) do
    form = Form.validate(socket.assigns.form, params)
    {:noreply, assign(socket, form: form)}
  end

  @impl true
  # params are in the `"form"` key
  def handle_event("save", %{"form" => params}, socket) do
    case AshPhoenix.Form.submit(socket.assigns.form, params: params) do
      {:ok, ticket} ->
        {:noreply,
         socket
         |> push_navigate(to: ~p"/#{ticket.id}/edit")}

      {:error, form} ->
        {:noreply, assign(socket, :form, form)}
    end
  end
    update :update do
      primary? true
      accept [:subject]

      argument :state, :atom, allow_nil?: false
      change transition_state(arg(:state)) # this erroneously said `arg(:status)` before.
    end
    <.input
      # name="state" <- needed to remove this
      # because the name is set automatically and if set manually then it should be `form[state]`
      type="select"
      field={@form[:state]}
      options={[
        {gettext("State A"), :state_a},
        {gettext("State B"), :state_b},
        {gettext("State C"), :state_c},
        {gettext("State D"), :state_d},
        {gettext("State E"), :state_e}
      ]}
    />
zachdaniel commented 5 days ago

Fix for error message display issues: 63af776cae96ebbb66a427337fd1a8bb3f50e825

MikaelFangel commented 5 days ago

Thank you! ❤️

MikaelFangel commented 5 days ago

@zachdaniel I tried to implement the changes suggested in the comment from you and also changed the ash_state_machine dep to build from main. However, the issue still seems to persist?

https://github.com/user-attachments/assets/2e68e9db-a721-424d-8f4e-cd7420a0c246

Changes: https://github.com/MikaelFangel/ash_state_machine_bug/commit/6a734ccfc049f7abd328f2a9c5ca6a2d3ea88cf3

MikaelFangel commented 5 days ago

Just to provide some more context. I have put in some inspects for debugging, and it looks like it "validates" immediately when entering the update form. This results in the target being nil, but when you submit, the values are present and your state machine works as intended.

The IO.inspects are in ash_state_machine.ex on line 132 and 133 and you can see the debug logs here, a bit simplified.

[target: nil]
old: update: :state_d
target: update: nil
------------------------------------
error msg
------------------------------------
[debug] Replied in 76ms
[debug] HANDLE EVENT "validate" in HelpdeskWeb.StateLive.Edit
  Component: HelpdeskWeb.StateLive.FormComponent
  Parameters: %{"_target" => ["form", "state"], "form" => %{"_unused_subject" => "", "state" => "state_e", "subject" => "t"}}
[target: :state_e]
old: update: :state_d
target: update: :state_e
[debug] Replied in 9ms
[debug] HANDLE EVENT "save" in HelpdeskWeb.StateLive.Edit
  Component: HelpdeskWeb.StateLive.FormComponent
  Parameters: %{"form" => %{"state" => "state_e", "subject" => "t"}}
[target: :state_e]
old: update: :state_d
target: update: :state_e
[target: :state_e]
old: update: :state_d
target: update: :state_e
[debug] QUERY OK source="tickets" db=2.3ms queue=1.2ms idle=1626.4ms
UPDATE "tickets" AS t0 SET "state" = $1, "id" = (CASE WHEN NOT (t0."state"::varchar = ANY($2) OR (t0."state"::varchar = ANY($3) OR (t0."state"::varchar = ANY($4) OR (t0."state"::varchar = ANY($5) OR t0."state"::varchar = ANY($6))))) THEN ash_raise_error(jsonb_build_object('exception', $7::text, 'input', jsonb_build_object($8::text, $9::jsonb, $10::text, t0."state"::varchar, $11::text, $12::jsonb))::jsonb, NULL::uuid) ELSE t0."id"::uuid END)::uuid, "subject" = $13::text::text, "updated_at" = $14::timestamp::timestamp WHERE (t0."id"::uuid = $15::uuid) RETURNING t0."id", t0."subject", t0."state", t0."inserted_at", t0."updated_at" [:state_e, ["state_e"], ["state_d"], ["state_c"], ["state_b"], ["state_a"], "AshStateMachine.Errors.NoMatchingTransition", "action", "update", "old_state", "target", "state_e", "t", ~U[2024-11-26 23:36:37.464741Z], "5b617c89-6e8d-46ba-8209-ba95a131553c"]
↳ AshPostgres.DataLayer.update_query/4, at: lib/data_layer.ex:1417
[debug] Replied in 48ms
[debug] MOUNT HelpdeskWeb.StateLive.Edit
  Parameters: %{"id" => "5b617c89-6e8d-46ba-8209-ba95a131553c"}
  Session: %{"_csrf_token" => "_wHjta-WMb1rekZJ9xkZJThL"}
[debug] QUERY OK source="tickets" db=1.5ms idle=1648.0ms
SELECT t0."id", t0."inserted_at", t0."state", t0."subject", t0."updated_at" FROM "tickets" AS t0 WHERE (t0."id"::uuid = $1::uuid) LIMIT $2 ["5b617c89-6e8d-46ba-8209-ba95a131553c", 2]
↳ anonymous fn/3 in AshPostgres.DataLayer.run_query/2, at: lib/data_layer.ex:781
[target: nil]
old: update: :state_e
target: update: nil
------------------------------------
error msg
------------------------------------
zachdaniel commented 5 days ago

Yeah, so for those cases there isn't really anything we can do about it. Forms are expected to be able to be invalid, and almost always are when you first create them. You could set a default state in the params given when creating the form (IIRC there is a params option to for_update) so that the state is always a valid state, or you could implement the protocol suggested for that error, to show the user something like "must be a valid state" in cases where the state input is invalid.

MikaelFangel commented 5 days ago

Thanks again. 😄

I am adding the fix here for the sake of completeness, as it feels a bit unintuitive and others might encounter the same problem.

defmodule HelpdeskWeb.StateLive.Edit do
  @moduledoc false
  use HelpdeskWeb, :live_view

  alias AshPhoenix.Form
  alias Helpdesk.Support.Ticket

  @impl true
  def mount(%{"id" => id} = _params, _session, socket) do
    ticket = Ticket |> Ash.get!(id)

    form =
      ticket
      |> Form.for_update(:update,
        forms: [auto?: true],
        params: %{state: ticket.state}
      )
      |> to_form()

    {:ok, socket |> assign(:form, form)}
  end
end
zachdaniel commented 5 days ago

Oh, strange. You should not have to do that. Ahhh, I see why 🤔 it's because you're not just accepting an attribute you're using an argument. Try this instead:

    <.input
      type="select"
      field={@form[:state]}
      value={@form.source.data.state}
      options={[
        {gettext("State A"), :state_a},
        {gettext("State B"), :state_b},
        {gettext("State C"), :state_c},
        {gettext("State D"), :state_d},
        {gettext("State E"), :state_e}
      ]}
    />
MikaelFangel commented 5 days ago

Unfortunately, that doesn't work either.

zachdaniel commented 5 days ago

🤔 now that is surprising. What is @form.source.data.state there? Does it equal the previous value? I think what we need to do in AshStateMachine is add something like this:

validate ValidStateChange

that you can put on an action that allows writing the state directly. Then you'd just do accept [:state], which would connect it to the previous value and remove this issue.

MikaelFangel commented 5 days ago

The value is the correct state and equals the current value of the attribute. 👍🏻

zachdaniel commented 5 days ago

Oh, right, duh. You're right in your original solution. Its not pretty, but if you're accepting state as an argument, thats what you'd need to do for now, in lieu of validate ValidStateChange

MikaelFangel commented 4 days ago

Yes, so https://github.com/ash-project/ash_state_machine/issues/70#issuecomment-2502350097 is the better solution IMO because it doesn't require for you to allow a nil state. But the original one works as well even though it's a bit awkward.

The solution I suggested in the comments works, which makes me wonder: couldn’t we improve it further so that specifying the state in the form isn’t necessary? It’s not a high priority since there’s a workaround, but making this change could enhance the framework’s/extensions ease of use.