Can't use `actor` on aggregates

hanrelan
2023-06-28

hanrelan:

Hi - I’m trying to use ^actor on aggregates but it’s throwing an error. This is my aggregate:

  aggregates do
    first :max_index, :audio_files, :index do
      sort index: :desc
      filter expr(user_id == ^actor(:id))
    end
  end

But calling: Practice.load!(visit, :max_index, actor: user, authorize?: true)

causes:

** (Ecto.SubQueryError) the following exception happened when compiling a subquery.

    ** (Ecto.Query.CastError) deps/ash_postgres/lib/expr.ex:530: value `{:_actor, :id}` in `where` cannot be cast to type #Ash.Type.UUID.EctoType<[]> in query:

Removing the ^actor works as expected though (but doesn’t filter what I need to ofc)

zachdaniel:

Correct you can’t access the actor in aggregates. They are currently meant to be static.

zachdaniel:

You‘ll probably need to do it manually w/ a calculation

hanrelan:

trying to do this as a custom calculation and I got this far:

defmodule Scribble.Practice.Visit.MaxIndex do
  use Ash.Calculation

  @impl true
  # A callback to tell Ash what keys must be loaded/selected when running this calculation
  def load(_query, _opts, _context) do
    [:id]
  end

  @impl true
  def calculate(visits, opts, %{user_id: user_id}) do
    Enum.map(visits, fn visit ->
      visit_id = visit.id

      audio_file =
        Scribble.Scribe.AudioFile
        |> Ash.Query.new()
        |> Ash.Query.filter(visit_id == ^visit_id)
        |> Ash.Query.filter(user_id == ^user_id)
        |> Ash.Query.sort(index: :desc)
        |> Ash.Query.limit(1)
        |> Scribble.Scribe.read!()

      audio_file.index
    end)
  end
end

But it’s throwing a compile error:

== Compilation error in file lib/scribble/practice/resources/visit.ex ==
** (CompileError) lib/scribble/practice/resources/visit.ex:128: cannot use ^visit_id outside of match clauses

visit_id definitely exists on the AudioFile resource

zachdaniel:

require Ash.Query

zachdaniel:

at the top of the file

hanrelan:

ah ok that makes sense. Ecto has a thing where it throws a warning that tells you to do that - might be helpful for Ash

zachdaniel:

There isn’t actually a way for us to do that in this case unfortunately

zachdaniel:

well

zachdaniel:

I guess how does ecto do it

zachdaniel:

😆

zachdaniel:

I’m pretty sure its impossible…

hanrelan:

incidentally I think there might be a bug in the docs: https://ash-hq.org/docs/guides/ash/latest/topics/actions#idiomatic-actions

Ticket
|> Ash.Query.for_read(:top)
|> Ash.Query.filter(opened_at > ago(10, :minute))
|> Helpdesk.Support.read!()

The top action takes an argument user_id

hanrelan:

let me check

hanrelan:

i definitely saw it pop up recently

zachdaniel:

The user_id would only be there like that if the calculation takes an argument of user_id

zachdaniel:

You might want %{actor: %{id: user_id}}

hanrelan:

ah ok got it

hanrelan:

the docs bug is a little different though (unrelated to my code, just was looking at it while debugging this)… In the docs, :top is defined as:


read :top do
  argument :user_id, :uuid do
    allow_nil? false
  end

  prepare build(limit: 10, sort: [opened_at: :desc])

  filter expr(priority in [:medium, :high] and representative_id == ^arg(:user_id) and status == :open)
end

so it seems like this:

Ticket
|> Ash.Query.for_read(:top)
|> Ash.Query.filter(opened_at > ago(10, :minute))
|> Helpdesk.Support.read!()

wouldn’t work?

zachdaniel:

Ohh, I see what you mean

zachdaniel:

yeah, you’re right 🙂

zachdaniel:

  @impl true
  def calculate(visits, opts, %{user_id: user_id}) do
    visit_ids = Enum.map(visits, &(&1.id))
   
    audio_file_indices =
      Scribble.Scribe.AudioFile
      |> Ash.Query.filter(visit_id in ^visit_ids)
      |> Ash.Query.filter(user_id == ^user_id)
      |> Ash.Query.sort(index: :desc)
      |> Ash.Query.distinct(:visit_id)
      |> Scribble.Scribe.read!()
      |> Map.new(&{&1.visit_id, &1.index})

    
    Enum.map(visits, fn visit ->
      audio_file_indices[visit.id]
    end)
  end

zachdaniel:

I’d suggest something like that (haven’t tested it of course)

zachdaniel:

the idea of taking a list of visits is to allow you to compute the values in an optimized way

hanrelan:

ah sort of dataloader pattern

hanrelan:

makes sense

zachdaniel:

yeah, exactly

hanrelan:

thanks!

zachdaniel:

my pleasure 🙂 If you don’t mind, could you make an issue about the incorrect docs?

hanrelan:

will do

hanrelan:

though I think it may have been a different one - I’ll try to reproduce

\ ឵឵឵:

Haven’t looked at Ecto, but something like this should work:

defmodule UsuallyNeedsRequire do
  defmacro __using__(_opts \\ []) do
    quote do
      require Logger

      @before_compile UsuallyNeedsRequire
    end
  end

  defmacro __before_compile__(env) do
    if RequireMe in env.requires do
      quote do
        Logger.debug("nice one, you required RequireMe")
      end
    else
      quote do
        Logger.warning("you might want to require RequireMe")
      end
    end
  end
end

defmodule RequireMe do
end

defmodule YesRequire do
  use UsuallyNeedsRequire
  require RequireMe
end

defmodule NoRequire do
  use UsuallyNeedsRequire
end

Whether or not you’d want to in this case… <:thinkies:915154230078222336>

zachdaniel:

🤔 well, that only works if you do use TheModule right?

zachdaniel:

Ash.Query doesn’t have a __using__ macro

\ ឵឵឵:

use Ash.Calculation

\ ឵឵឵:

I don’t think you should add this warning btw, surely Ash.Query is not sufficiently universally used in Ash.Calculation impls 😄

zachdaniel:

yeah, agreed