Validation that tests relationship value (without using custom validator?)

The main thing that makes validations across tables like this hard is that, unlike other validations, there is no guarantee that they will be true as the related resource can change on its own. But that doesn’t necessarily have to be a problem, it’s just on you to understand that that is the case.

Unfortunately there is no builtin validation, so you will in fact need to drop down to the complicated version of things. Good thing I’m here!

defmodule ValidateWithinSprintRange do
  use Ash.Resource.Validation

  def atomic(changeset, _, _) do
     {:atomic, 
       
       # the fields involved. Used by the framework (technically its not but it will some day)
       [:start_date, :end_date], 
       
       # the expression that should *not* be true. i.e if this is true, the error will be raised
       expr(^atomic_ref(:date) < sprint.start_date or ^atomic_ref(:date) > sprint.end_date), 
       
       # an expression that calls `error/2` to produce the expression
       expr(
         error(
            Ash.Error.Changes.InvalidAttribute, %{
              field: :date,
              value: ^atomic_ref(:date),
              message: "date must be between the sprint's start and end date"
            }
         )
      )
    }
  end
end

Something along those lines should do the trick. By defining it this way (with only an atomic callback) the validation will always be deferred to when the query is run. But in the case of checking related data, thats typically the time you’d want to do it anyway. What you can do if you want the eager validation, like in a form, is add a regular validate/3 callback that does the same work manually.

I think this can be made much more ergonomic over time, but it’s good to know how to do this

Thanks for this @zachdaniel, that was hugely helpful in furthering my understanding of atomics.

I did run headlong into:

%Ash.Error.Framework.CanNotBeAtomic{resource: COE.Walk.Activity, change: COE.Validations.ValidateWithinSprint, reason: "Create actions cannot be made atomic", splode: Ash.Error, bread_crumbs: [], vars: [], path: [], stacktrace: #Splode.Stacktrace<>, class: :framework}

Which I did not realize. I need to implement it as a non-atomic validate/3 function. (And presumably keep the atomic version for, e.g., updates).

The parallel writeup here was also a great read: Does validations in update actions always require you to load the fields the validations are working on - #5 by anagrius.

My first attempt was to try and load/2 the sprint inside the validate/3:

sprint = Ash.load!(changeset.data, [:sprint])

That failed. I’m assuming, because the activity is not yet persisted. But I was a bit surprised. The changeset does have the relationship. I would have thought load/2 would be smart enough but, my understanding is not complete.

So… I proceeded (hopefully correctly) with the following validate/3 function.

  def validate(changeset, _, _) do
    sprint = Sprint |> Ash.Query.filter(id == ^Ash.Changeset.get_attribute(changeset, :sprint_id)) |> Ash.read_one!()
    date = Ash.Changeset.get_attribute(changeset, :date)
    if date < sprint.start_date or date > sprint.end_date do
      {:error, field: :base, message: "activity date must be between the sprint's range of #{sprint.start_date} and #{sprint.end_date}"}
    else
      :ok
    end
  end

The only bit I’m a little uncomfortable with is the read_one call. The above works. I’m just wondering if it is less efficient than it could be, or if there’s a more idiomatic approach.

Only two notes:

1. I’d suggest adding a select statement for only the fields you want from the sprint and
2. if you want to make this truly resilient to concurrent updates on the sprint’s start date/end date, you could lock the sprint for updates. For this lock to work, however, you’d have to make sure to add before_action?: true when you use this validation, i.e validate YourModule, before_action?: true.

What that would look like:

  def validate(changeset, _, _) do
    sprint = 
     Sprint 
     |> Ash.Query.filter(id == ^Ash.Changeset.get_attribute(changeset, :sprint_id))
     |> Ash.Query.select([:start_date, :end_date])
     |> Ash.Query.lock(:for_update)
     |> Ash.read_one!()

    date = Ash.Changeset.get_attribute(changeset, :date)
    if date < sprint.start_date or date > sprint.end_date do
      {:error, field: :base, message: "activity date must be between the sprint's range of #{sprint.start_date} and #{sprint.end_date}"}
    else
      :ok
    end
  end

Thanks for that @zachdaniel! Changes implemented.

I’m getting the following compiler warning. I futzed with it for a while and can’t figure out any way to shut it up. Thinking it must have something to do with Ash.Query.filter/1 macro interpretation…?

warning: Comparing values with `nil` will always return `false`. Use `is_nil/1` instead. In: `id == nil`
  (boss_site 0.2.0) lib/coe/walk/validations/validate_within_sprint.ex:7: COE.Validations.ValidateWithinSprint.validate/3
  (ash 3.4.8) lib/ash/changeset/changeset.ex:2887: Ash.Changeset.do_validation/5
  (elixir 1.17.2) lib/enum.ex:2531: Enum."-reduce/3-lists^foldl/2-0-"/3
  (ash 3.4.8) lib/ash/changeset/changeset.ex:1743: Ash.Changeset.do_for_action/4
  (boss_site 0.2.0) deps/ash/lib/ash/code_interface.ex:682: COE.Walk.Activity.add/8
  test/coe/activity_test.exs:41: COE.Walk.ActivityTest."test activities must be persistent, and require a stereotype, [owner] and sprint"/1
  (ex_unit 1.17.2) lib/ex_unit/runner.ex:485: ExUnit.Runner.exec_test/2
  (stdlib 5.2.3) timer.erl:270: :timer.tc/2
  (ex_unit 1.17.2) lib/ex_unit/runner.ex:407: anonymous fn/6 in ExUnit.Runner.spawn_test_monitor/4

The specific line triggering it corresponding to validate_within_sprint.ex:7:

|> Ash.Query.filter(id == ^Ash.Changeset.get_attribute(changeset, :sprint_id))

I did try factoring out the function calls so it’s just |> Ash.Query.filter(id == ^sprint_id) but it makes no difference.

Trying |> Ash.Query.filter(id == "xyzzy") shuts up the warning, but obviously doesn’t work. Also tried wrapping it in expr/1 to no avail.

It seems the filter/1 macro is somehow interpreting ^sprint_id as nil.

Uh. Found one way to shut it up but…

|> Ash.Query.filter("#{id}" == "#{^Ash.Changeset.get_attribute(changeset, :sprint_id)}")

Have you determined for sure that sprint_id isn’t nil in that case? That is pretty much the only case you should get that warning.

Ahh! In this case, the sprint is not nil.

But thanks to your note, I realized there are tests in the test suite where it would be nil to protect against a bad state. That’s what’s triggering the warning.

Adding a guard fixes it in the validation – but that raises an interesting question. There are constraints in place that prevent setting the sprint to nil. It hadn’t occurred to me that the validation would run before the constraints. Is there any way to postpone the validations until after constraints are checked?

Specifically, the belongs_to relationship is allow_nil? false.

validate Something, only_when_valid?: true to run it only when the changes are valid.

The idea is that you may want to provide many validation errors at once instead of just the first one that fails.

For your particular case, you can decide how you want it to behave. On its own it might make sense to just ignore the validation entirely if there is no sprint_id. It’s something else’s job to ensure that it is set.

  def validate(changeset, _, _) do
    if spring_id = Ash.Changeset.get_attribute(changeset, :sprint_id) do
      your_logic
    else
      :ok
    end
  end

Then the experience of the caller would be “you must set spring_id”, and then “must be in range” if it’s not in range.

@zachdaniel, while Ash Framework has a bit of a learning curve, as always you’ve thought ahead and built in a great deal of flexibility and power. Looking forward to (again!) getting more familiar with it!

Would love to see some of the above make its way into the docs. While I feel like all the pieces are there, putting it together can be a challenge. Perhaps building out the existing examples with some of these more subtle (and powerful) features would be helpful, especially now with atomics on the scene…

You’re absolutely right better wholistic examples is very important, and we need more of them.