improvement: policy for including state machine in can? checks

config/config.exs

@@ -9,6 +9,8 @@ config :spark, :formatter,
 if Mix.env() == :test do
   config :ash, :validate_domain_resource_inclusion?, false
   config :ash, :validate_domain_config_inclusion?, false
+  config :ash_state_machine, :ash_domains, [Domain]
+  config :logger, level: :warning
 end
 
 if Mix.env() == :dev do

documentation/tutorials/get-started-with-state-machines.md

@@ -101,6 +101,20 @@ defmodule Order do
 end
 ```
 
+## Adding a state machine policy
+
+Using `Ash.can?/3` won't return `false` if a given state machine transition is invalid. This is because `Ash.can?/3` is only concerned with policies, not changes/validations. However, many folks use `Ash.can?/3` in their UI to determine whether a given button/form/etc should be shown. To help with this you can add the following to your resource:
+
+```elixir
+policies do
+  policy always() do
+    authorize_if AshStateMachine.Checks.ValidNextState
+  end
+end
+```
+
+This check is only used in _pre_flight_ authorization checks (i.e calling `Ash.can?/3`), but it will return `true` in all cases when running real authorization checks. This is because the change is validated when you use the `transition_state/1` change and `AshStateMachine.transition_state/2`, and so you would be doing extra work for no reason.
+
 ## Generating Flow Charts
 
 run `mix ash_state_machine.generate_flow_charts` to generate flow charts for your resources. See the task documentation for more. Here is a chart generated from the example above:

lib/ash_state_machine.ex

@@ -143,8 +143,9 @@ defmodule AshStateMachine do
     if target in AshStateMachine.Info.state_machine_initial_states!(changeset.resource) do
       Ash.Changeset.force_change_attribute(changeset, attribute, target)
     else
-      Ash.Changeset.add_error(
+      changeset
-        changeset,
+      |> Ash.Changeset.set_context(%{state_machine: %{attempted_change: target}})
+      |> Ash.Changeset.add_error(
         AshStateMachine.Errors.InvalidInitialState.exception(
           target: target,
           action: changeset.action.name
@@ -165,8 +166,9 @@ defmodule AshStateMachine do
     end)
     |> case do
       nil ->
-        Ash.Changeset.add_error(
+        changeset
-          changeset,
+        |> Ash.Changeset.set_context(%{state_machine: %{attempted_change: target}})
+        |> Ash.Changeset.add_error(
           AshStateMachine.Errors.NoMatchingTransition.exception(
             old_state: old_state,
             target: target,
@@ -195,8 +197,9 @@ defmodule AshStateMachine do
       To remediate this, add the `extra_states` option and include the state #{inspect(target)}
     """)
 
-    Ash.Changeset.add_error(
+    changeset
-      changeset,
+    |> Ash.Changeset.set_context(%{state_machine: %{attempted_change: target}})
+    |> Ash.Changeset.add_error(
       AshStateMachine.Errors.NoMatchingTransition.exception(
         old_state: old_state,
         target: target,

lib/checks/valid_next_state.ex

@@ -0,0 +1,66 @@
+defmodule AshStateMachine.Checks.ValidNextState do
+  @moduledoc """
+  A policy for pre_flight checking if a state transition is allowed.
+  """
+  use Ash.Policy.FilterCheck
+
+  def describe(_) do
+    "allowed to make state transition"
+  end
+
+  def filter(
+        _actor,
+        %{changeset: %Ash.Changeset{action_type: :create} = changeset} = _context,
+        _options
+      ) do
+    attribute = AshStateMachine.Info.state_machine_state_attribute!(changeset.resource)
+
+    if changeset.context[:private][:pre_flight_authorization?] do
+      new_state =
+        Ash.Changeset.get_attribute(changeset, attribute) ||
+          AshStateMachine.Info.state_machine_default_initial_state!(changeset.resource)
+
+      {:ok, new_state in AshStateMachine.Info.state_machine_initial_states!(changeset.resource)}
+    else
+      {:ok, true}
+    end
+  end
+
+  def filter(
+        _actor,
+        %{changeset: %Ash.Changeset{action_type: :update} = changeset} = _context,
+        _options
+      ) do
+    attribute = AshStateMachine.Info.state_machine_state_attribute!(changeset.resource)
+
+    if changeset.context[:private][:pre_flight_authorization?] &&
+         (Ash.Changeset.changing_attribute?(changeset, attribute) ||
+            get_in(changeset.context, [:state_machine, :attempted_change])) do
+      transitions =
+        AshStateMachine.Info.state_machine_transitions(changeset.resource, changeset.action.name)
+
+      old_state = expr(^ref(attribute))
+      target = Ash.Changeset.get_attribute(changeset, attribute)
+      all_states = AshStateMachine.Info.state_machine_all_states(changeset.resource)
+
+      if not is_nil(target) && !Ash.Expr.expr?(target) && target not in all_states do
+        {:ok, false}
+      else
+        states_expr =
+          Enum.reduce(transitions, nil, fn transition, expr ->
+            state_expr =
+              expr(
+                ^old_state in ^List.wrap(transition.from) and ^target in ^List.wrap(transition.to)
+              )
+
+            expr(^state_expr or ^expr)
+          end)
+
+        expr(is_nil(^target) || (^target in ^all_states and ^states_expr))
+      end
+    else
+      # state transitions are checked in validations when using `transition_state`
+      {:ok, true}
+    end
+  end
+end

lib/errors/no_matching_event.ex

@@ -5,11 +5,18 @@ defmodule AshStateMachine.Errors.NoMatchingTransition do
     class: :invalid
 
   def message(error) do
-    if error.old_state do
+    cond do
+      error.old_state && error.target ->
         """
         Attempted to change state from #{error.old_state} to #{error.target} in action #{error.action}, but no matching transition was configured.
         """
-    else
+
+      error.old_state ->
+        """
+        Attempted to change state from #{error.old_state} in action #{error.action}, but no matching transition was configured.
+        """
+
+      error.target ->
         """
         Attempted to change state to #{error.target} in action #{error.action}, but no matching transition was configured.
         """

mix.exs

@@ -118,7 +118,8 @@ defmodule AshStateMachine.MixProject do
   # Run "mix help deps" to learn about dependencies.
   defp deps do
     [
-      {:ash, ash_version("~> 3.0.0-rc.0")},
+      {:ash, ash_version("~> 3.0.0-rc and >= 3.0.0-rc.40")},
+      {:simple_sat, "~> 0.1", only: [:dev, :test], runtime: false},
       {:ex_doc, github: "elixir-lang/ex_doc", only: [:dev, :test], runtime: false},
       {:ex_check, "~> 0.12", only: [:dev, :test]},
       {:credo, ">= 0.0.0", only: [:dev, :test], runtime: false},

mix.lock

@@ -1,5 +1,5 @@
 %{
-  "ash": {:hex, :ash, "3.0.0-rc.38", "87a3ca7d18196e76723f485c553405373908d637e668eea5e2ed1a37c1aad1c8", [:mix], [{:comparable, "~> 1.0", [hex: :comparable, repo: "hexpm", optional: false]}, {:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8", [hex: :ets, repo: "hexpm", optional: false]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: true]}, {:plug, ">= 0.0.0", [hex: :plug, repo: "hexpm", optional: true]}, {:reactor, ">= 0.8.1 and < 1.0.0-0", [hex: :reactor, repo: "hexpm", optional: false]}, {:simple_sat, ">= 0.1.1 and < 1.0.0-0", [hex: :simple_sat, repo: "hexpm", optional: true]}, {:spark, ">= 2.1.18 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, "~> 0.2", [hex: :splode, repo: "hexpm", optional: false]}, {:stream_data, "~> 0.6", [hex: :stream_data, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.1", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "16bc98312ef1ac7aca3112ee41504e32072bc665546702722414aec1f0f8e338"},
+  "ash": {:hex, :ash, "3.0.0-rc.40", "cc3951779e531c3e736e6ec5767f1887c25a4a015d2e6b7a6127775b8678654e", [:mix], [{:comparable, "~> 1.0", [hex: :comparable, repo: "hexpm", optional: false]}, {:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8", [hex: :ets, repo: "hexpm", optional: false]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: true]}, {:plug, ">= 0.0.0", [hex: :plug, repo: "hexpm", optional: true]}, {:reactor, ">= 0.8.1 and < 1.0.0-0", [hex: :reactor, repo: "hexpm", optional: false]}, {:simple_sat, ">= 0.1.1 and < 1.0.0-0", [hex: :simple_sat, repo: "hexpm", optional: true]}, {:spark, ">= 2.1.18 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, "~> 0.2", [hex: :splode, repo: "hexpm", optional: false]}, {:stream_data, "~> 0.6", [hex: :stream_data, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.1", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "5e9eff8857226f2859ecdca7bc167a231cdebded40b4c210fc0e30ba9d761243"},
   "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"},
   "comparable": {:hex, :comparable, "1.0.0", "bb669e91cedd14ae9937053e5bcbc3c52bb2f22422611f43b6e38367d94a495f", [:mix], [{:typable, "~> 0.1", [hex: :typable, repo: "hexpm", optional: false]}], "hexpm", "277c11eeb1cd726e7cd41c6c199e7e52fa16ee6830b45ad4cdc62e51f62eb60c"},
   "credo": {:hex, :credo, "1.7.5", "643213503b1c766ec0496d828c90c424471ea54da77c8a168c725686377b9545", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "f799e9b5cd1891577d8c773d245668aa74a2fcd15eb277f51a0131690ebfb3fd"},
@@ -22,6 +22,7 @@
   "mix_audit": {:hex, :mix_audit, "2.1.3", "c70983d5cab5dca923f9a6efe559abfb4ec3f8e87762f02bab00fa4106d17eda", [:make, :mix], [{:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: false]}, {:yaml_elixir, "~> 2.9", [hex: :yaml_elixir, repo: "hexpm", optional: false]}], "hexpm", "8c3987100b23099aea2f2df0af4d296701efd031affb08d0746b2be9e35988ec"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"},
   "reactor": {:hex, :reactor, "0.8.1", "1aec71d16083901277727c8162f6dd0f07e80f5ca98911b6ef4f2c95e6e62758", [:mix], [{:libgraph, "~> 0.16", [hex: :libgraph, repo: "hexpm", optional: false]}, {:spark, "~> 2.0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, "~> 0.2", [hex: :splode, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.2", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "ae3936d97a3e4a316744f70c77b85345b08b70da334024c26e6b5eb8ede1246b"},
+  "simple_sat": {:hex, :simple_sat, "0.1.3", "f650fc3c184a5fe741868b5ac56dc77fdbb428468f6dbf1978e14d0334497578", [:mix], [], "hexpm", "a54305066a356b7194dc81db2a89232bacdc0b3edaef68ed9aba28dcbc34887b"},
   "sobelow": {:hex, :sobelow, "0.13.0", "218afe9075904793f5c64b8837cc356e493d88fddde126a463839351870b8d1e", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "cd6e9026b85fc35d7529da14f95e85a078d9dd1907a9097b3ba6ac7ebbe34a0d"},
   "sourceror": {:hex, :sourceror, "1.0.3", "111711c147f4f1414c07a67b45ad0064a7a41569037355407eda635649507f1d", [:mix], [], "hexpm", "56c21ef146c00b51bc3bb78d1f047cb732d193256a7c4ba91eaf828d3ae826af"},
   "spark": {:hex, :spark, "2.1.20", "204db8fd28378783c28a9dcb0bebdaf1d51b14a9ea106e1080457d29510a66ea", [:mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.0", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "e7a4f8f8ca7a477918af1eb65e20f2015f783a9a23e5f73d1020edf5b2ef69be"},

test/ash_state_machine_test.exs

@@ -2,155 +2,6 @@ defmodule AshStateMachineTest do
   use ExUnit.Case
   doctest AshStateMachine
 
-  defmodule Order do
-    # leaving out data layer configuration for brevity
-    use Ash.Resource,
-      domain: AshStateMachineTest.Domain,
-      extensions: [AshStateMachine]
-
-    state_machine do
-      initial_states [:pending]
-      default_initial_state :pending
-
-      transitions do
-        transition :confirm, from: :pending, to: :confirmed
-        transition :begin_delivery, from: :confirmed, to: :on_its_way
-        transition :package_arrived, from: :on_its_way, to: :arrived
-        transition :error, from: [:pending, :confirmed, :on_its_way], to: :error
-        transition :abort, from: :*, to: :aborted
-        transition :reroute, from: :*, to: :rerouted
-      end
-    end
-
-    actions do
-      default_accept :*
-      # create sets the st
-      defaults [:create, :read]
-
-      update :confirm do
-        # accept [...] you can change other attributes
-        # or do anything else an action can normally do
-        # this transition will be validated according to
-        # the state machine rules above
-        change transition_state(:confirmed)
-      end
-
-      update :begin_delivery do
-        # accept [...]
-        change transition_state(:on_its_way)
-      end
-
-      update :package_arrived do
-        # accept [...]
-        change transition_state(:arrived)
-      end
-
-      update :error do
-        accept [:error_state, :error]
-        change transition_state(:error)
-      end
-
-      update :abort do
-        # accept [...]
-        change transition_state(:aborted)
-      end
-
-      update :reroute do
-        # accept [...]
-
-        # The defined transition for this route contains a `from: :*` but does not include `to: :aborted`
-        # This should never succeed
-        change transition_state(:aborted)
-      end
-    end
-
-    changes do
-      # any failures should be captured and transitioned to the error state
-      change after_transaction(fn
-               changeset, {:ok, result}, _ ->
-                 {:ok, result}
-
-               changeset, {:error, error}, _ ->
-                 message = Exception.message(error)
-
-                 changeset.data
-                 |> Ash.Changeset.for_update(:error, %{
-                   message: message,
-                   error_state: changeset.data.state
-                 })
-             end),
-             on: [:update]
-    end
-
-    code_interface do
-      define :abort
-      define :reroute
-    end
-
-    attributes do
-      uuid_primary_key :id
-      # ...attributes like address/delivery options would go here
-      attribute :error, :string, public?: true
-      attribute :error_state, :string, public?: true
-      # :state attribute is added for you by `state_machine`
-      # however, you can add it yourself, and you will be guided by
-      # compile errors on what states need to be allowed by your type.
-    end
-  end
-
-  defmodule ThreeStates do
-    use Ash.Resource,
-      domain: AshStateMachineTest.Domain,
-      data_layer: Ash.DataLayer.Ets,
-      extensions: [AshStateMachine]
-
-    state_machine do
-      initial_states [:pending]
-      default_initial_state :pending
-
-      transitions do
-        transition(:begin, from: :pending, to: :executing)
-        transition(:complete, from: :executing, to: :complete)
-        transition(:*, from: :*, to: :pending)
-      end
-    end
-
-    actions do
-      default_accept :*
-      defaults [:read, :create]
-
-      update :begin do
-        change transition_state(:executing)
-      end
-
-      update :complete do
-        change transition_state(:complete)
-      end
-    end
-
-    ets do
-      private? true
-    end
-
-    attributes do
-      uuid_primary_key :id
-    end
-
-    code_interface do
-      define :create
-      define :begin
-      define :complete
-    end
-  end
-
-  defmodule Domain do
-    use Ash.Domain
-
-    resources do
-      allow_unregistered? true
-    end
-  end
-
   describe "transformers" do
     test "infers all states, excluding star (:*)" do
       assert Enum.sort(AshStateMachine.Info.state_machine_all_states(ThreeStates)) ==
@@ -185,6 +36,11 @@ defmodule AshStateMachineTest do
     test "`from: :*` cannot transition _to_ any state" do
       for state <- [:pending, :confirmed, :on_its_way, :arrived, :error] do
         assert {:error, reason} = Order.reroute(%Order{state: state})
+
+        if state != :aborted do
+          assert Ash.can?({%Order{state: state}, :reroute}, nil) == false
+        end
+
         assert Exception.message(reason) =~ ~r/no matching transition/i
       end
     end
@@ -206,49 +62,6 @@ defmodule AshStateMachineTest do
   end
 
   describe "next state" do
-    defmodule NextStateMachine do
-      @moduledoc false
-      use Ash.Resource,
-        domain: AshStateMachineTest.Domain,
-        extensions: [AshStateMachine]
-
-      state_machine do
-        initial_states [:a]
-        default_initial_state :a
-
-        transitions do
-          transition :next, from: :a, to: :b
-          transition :next, from: :b, to: :c
-          transition :next, from: :b, to: :d
-        end
-      end
-
-      attributes do
-        uuid_primary_key :id
-
-        attribute :state, :atom do
-          allow_nil? false
-          public? true
-          constraints one_of: [:a, :b, :c, :d]
-          default :a
-        end
-      end
-
-      actions do
-        default_accept :*
-        defaults [:read, :create]
-
-        update :next do
-          change next_state()
-        end
-      end
-
-      code_interface do
-        define :create
-        define :next
-      end
-    end
-
     test "when there is only one next state, it transitions into it" do
       assert {:ok, nsm} = NextStateMachine.create(%{state: :a})
       assert {:ok, nsm} = NextStateMachine.next(nsm)

test/support/domain.ex

@@ -0,0 +1,10 @@
+defmodule Domain do
+  @moduledoc false
+  use Ash.Domain
+
+  resources do
+    resource ThreeStates
+    resource Order
+    resource NextStateMachine
+  end
+end

test/support/next_state_machine.ex

@@ -0,0 +1,42 @@
+defmodule NextStateMachine do
+  @moduledoc false
+  use Ash.Resource,
+    domain: Domain,
+    extensions: [AshStateMachine]
+
+  state_machine do
+    initial_states [:a]
+    default_initial_state :a
+
+    transitions do
+      transition :next, from: :a, to: :b
+      transition :next, from: :b, to: :c
+      transition :next, from: :b, to: :d
+    end
+  end
+
+  attributes do
+    uuid_primary_key :id
+
+    attribute :state, :atom do
+      allow_nil? false
+      public? true
+      constraints one_of: [:a, :b, :c, :d]
+      default :a
+    end
+  end
+
+  actions do
+    default_accept :*
+    defaults [:read, :create]
+
+    update :next do
+      change next_state()
+    end
+  end
+
+  code_interface do
+    define :create
+    define :next
+  end
+end

test/support/order.ex

@@ -0,0 +1,103 @@
+defmodule Order do
+  @moduledoc false
+  # leaving out data layer configuration for brevity
+  use Ash.Resource,
+    domain: Domain,
+    extensions: [AshStateMachine],
+    authorizers: [Ash.Policy.Authorizer]
+
+  state_machine do
+    initial_states [:pending]
+    default_initial_state :pending
+
+    transitions do
+      transition :confirm, from: :pending, to: :confirmed
+      transition :begin_delivery, from: :confirmed, to: :on_its_way
+      transition :package_arrived, from: :on_its_way, to: :arrived
+      transition :error, from: [:pending, :confirmed, :on_its_way], to: :error
+      transition :abort, from: :*, to: :aborted
+      transition :reroute, from: :*, to: :rerouted
+    end
+  end
+
+  policies do
+    policy always() do
+      authorize_if AshStateMachine.Checks.ValidNextState
+    end
+  end
+
+  actions do
+    default_accept :*
+    # create sets the st
+    defaults [:create, :read]
+
+    update :confirm do
+      # accept [...] you can change other attributes
+      # or do anything else an action can normally do
+      # this transition will be validated according to
+      # the state machine rules above
+      change transition_state(:confirmed)
+    end
+
+    update :begin_delivery do
+      # accept [...]
+      change transition_state(:on_its_way)
+    end
+
+    update :package_arrived do
+      # accept [...]
+      change transition_state(:arrived)
+    end
+
+    update :error do
+      accept [:error_state, :error]
+      change transition_state(:error)
+    end
+
+    update :abort do
+      # accept [...]
+      change transition_state(:aborted)
+    end
+
+    update :reroute do
+      # accept [...]
+
+      # The defined transition for this route contains a `from: :*` but does not include `to: :aborted`
+      # This should never succeed
+      change transition_state(:aborted)
+    end
+  end
+
+  changes do
+    # any failures should be captured and transitioned to the error state
+    change after_transaction(fn
+             changeset, {:ok, result}, _ ->
+               {:ok, result}
+
+             changeset, {:error, error}, _ ->
+               message = Exception.message(error)
+
+               changeset.data
+               |> Ash.Changeset.for_update(:error, %{
+                 message: message,
+                 error_state: changeset.data.state
+               })
+           end),
+           on: [:update]
+  end
+
+  code_interface do
+    define :abort
+    define :reroute
+  end
+
+  attributes do
+    uuid_primary_key :id
+    # ...attributes like address/delivery options would go here
+    attribute :error, :string, public?: true
+    attribute :error_state, :string, public?: true
+    # :state attribute is added for you by `state_machine`
+    # however, you can add it yourself, and you will be guided by
+    # compile errors on what states need to be allowed by your type.
+  end
+end

test/support/three_states.ex

@@ -0,0 +1,45 @@
+defmodule ThreeStates do
+  @moduledoc false
+  use Ash.Resource,
+    domain: Domain,
+    data_layer: Ash.DataLayer.Ets,
+    extensions: [AshStateMachine]
+
+  state_machine do
+    initial_states [:pending]
+    default_initial_state :pending
+
+    transitions do
+      transition(:begin, from: :pending, to: :executing)
+      transition(:complete, from: :executing, to: :complete)
+      transition(:*, from: :*, to: :pending)
+    end
+  end
+
+  actions do
+    default_accept :*
+    defaults [:read, :create]
+
+    update :begin do
+      change transition_state(:executing)
+    end
+
+    update :complete do
+      change transition_state(:complete)
+    end
+  end
+
+  ets do
+    private? true
+  end
+
+  attributes do
+    uuid_primary_key :id
+  end
+
+  code_interface do
+    define :create
+    define :begin
+    define :complete
+  end
+end