Skip to content
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.

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

Purpose of @callback transition_changeset is not clear #12

Open
vovayartsev opened this issue Jan 10, 2023 · 1 comment
Open

Purpose of @callback transition_changeset is not clear #12

vovayartsev opened this issue Jan 10, 2023 · 1 comment

Comments

@vovayartsev
Copy link

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 🙏

@zamith
Copy link
Member

zamith commented Aug 4, 2023

The purpose of this callback is indeed to override as you've done in your example. The callback definition was wrong and is fixed as part of other changes in #16 . But yes, by not having @behavior the callback definitions are merely informative and not being enforced.

If you'd like to open a pull request changing this, it could be a good first contribution, if you are interested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants