subvisual / fsmx Goto Github PK

Thanks for the package. Is there a possibility to have multiple transition sets per schema, for example

field(:state, :string, default: "start")
field(:second_set_state, :string, default: "start")
field(:third_set_state, :string, default: "start")
...

If I understand correctly, state is hard-coded here.

Is there a way to encode a transition from a state to any state, or do you have to list all the states you could transition from individually?

And is there any documentation of what the transitions key means exactly? I only see examples in the readme, such as:

  use Fsmx.Struct, transitions: %{
    "one" => ["two", "three"],
    "two" => ["three", "four"],
    "three" => "four"
  }

Hello

When will you be releasing the current master?
we would need the bugfix regarding the IO.inspect

thanks and kind regards

Hey 👋

Thanks for a nice & clean library.
One question (pardon if a silly one) is about this callback definitions:

defmodule Fsmx.Fsm do
  ...
  if Code.ensure_loaded?(Ecto) do
    @callback transition_changeset(struct, Fsmx.state_t, Fsmx.state_t) :: Ecto.Changeset.t()
    @callback after_transition_multi(struct, Fsmx.state_t, Fsmx.state_t) ::
                {:ok, struct} | {:error, any}
  end
  ...

I thought the idea was to use @impl when overriding transition_changeset, like here:

defmodule App.StateMachineSchema do
  # ...

  @impl Fsmx.Fsm
  def transition_changeset(changeset, "one", "two", params) do
    # changeset already includes a :state field change
    changeset
    |> cast(params, [:data])
    |> validate_required([:data])
  end

but transition_changeset above accepts 4 params, while callback defines only 3 params.
Also there's no @behaviour in defmacro __using__.

This is not a bug report - just curious what was the original intent around @callback 🙏

There seems to be a change since version 0.5.0 that's breaking when there is no callback defined for a valid transition_changeset/4 callback. Previously the implementation of this callback was optional.

The documentation for before_transition/3 mentions this:

You only need to pattern-match on the scenarios you want to catch. No need to add a catch-all/do-nothing function at the end (the library already does that for you).

This also applied to the transition_changeset/4 callback before, but that has changed, resulting in a FunctionClauseError error when a valid transition is triggered.

I'm curious if this is intended. It's trivial to supply a default implementation for this callback, but I actually preferred the previous behaviour.

See this livebook for a reproduction (use the import functionality, choose "From URL", and paste this url):
https://gist.github.com/linusdm/a79c241c2f9b7a1c3ac158b1aa530ca2#file-repro-livemd

Thanks in advance for any feedback!

Hey team, thank you for the work you are doing here.

Quick question, any specific reason why atoms are not valid states in the type spec?

This works just fine but I'm getting mismatching type in the dialyzer

  def submit_order(order, order_params) do
    {:ok, %{new: new}} =
      Multi.new()
      |> Fsmx.transition_multi(order, :new, :submitted, order_params)
      |> Repo.transaction()

    {:ok, Repo.reload(new)}
  end

The current implementation is naive - its assuming that DB state is equal to loaded (runtime) state. A correct implementation would need to assure that with some form of locking. For example, before transitioning, we could lock the row (via a write lock or a simple advisory lock) and re-read the state column, making sure our copy of it is up to date.

With non-transactional transition its ignored completely, but maybe that is fine for people who use it (????).
Now, a user of the library might think it's possible to achieve correctness with transition_multi. However, it has no way to accept the loaded schema inside transaction, and doesn't fetch the record itself. It accepts the schema argument which must be known (loaded) outside of transaction context.

An alternative approach would be to use a transition query with WHERE state=^previous_state, but then it wouldn't work with a changeset.

When looking at the </> buttons on hexdocs.pm, or any documentation tool (e.g. "Dash", or "Zeal"), ex_doc attempts to use the library version to construct a link, and assumes there's an equivalent git tag.

Could you push some git tags matching those versions so I can more easily review things when implementing? Main example is from when I need a reminder of what the use Fsmx.Fsm does.

If there's another way you'd prefer that's fine too. I'd submit a PR, but tags aren't PR-able, and anything else would probably be something you'd wanna discuss first 😅