fix: pass actor to Ash.Query.for_read for aggregate and sort

Author: zachdaniel

lib/aggregate.ex

@@ -969,41 +969,11 @@ defmodule AshSql.Aggregate do
         {:cont, {:ok, [aggregate | aggregates]}}
 
       aggregate, {:ok, aggregates} ->
-        related = Ash.Resource.Info.related(resource, aggregate.relationship_path)
-
-        read_action =
-          aggregate.read_action || Ash.Resource.Info.primary_action!(related, :read).name
-
-        with %{valid?: true} = aggregate_query <- Ash.Query.for_read(related, read_action),
-             %{valid?: true} = aggregate_query <-
-               Ash.Query.build(aggregate_query, filter: aggregate.filter, sort: aggregate.sort) do
-          Ash.Query.Aggregate.new(
-            resource,
-            aggregate.name,
-            aggregate.kind,
-            path: aggregate.relationship_path,
-            query: aggregate_query,
-            field: aggregate.field,
-            default: aggregate.default,
-            filterable?: aggregate.filterable?,
-            type: aggregate.type,
-            sortable?: aggregate.filterable?,
-            include_nil?: aggregate.include_nil?,
-            constraints: aggregate.constraints,
-            implementation: aggregate.implementation,
-            uniq?: aggregate.uniq?,
-            read_action:
-              aggregate.read_action ||
-                Ash.Resource.Info.primary_action!(
-                  Ash.Resource.Info.related(resource, aggregate.relationship_path),
-                  :read
-                ).name,
-            authorize?: aggregate.authorize?
-          )
-        else
-          %{errors: errors} ->
-            {:error, errors}
-        end
+        resource
+        |> resource_aggregate_to_aggregate(aggregate,
+          actor: private_context[:actor],
+          tenant: private_context[:tenant]
+        )
         |> case do
           {:ok, aggregate} ->
             aggregate =
@@ -1027,6 +997,44 @@ defmodule AshSql.Aggregate do
     end)
   end
 
+  @doc false
+  def resource_aggregate_to_aggregate(resource, aggregate, opts \\ []) do
+    related = Ash.Resource.Info.related(resource, aggregate.relationship_path)
+
+    read_action =
+      aggregate.read_action || Ash.Resource.Info.primary_action!(related, :read).name
+
+    with %{valid?: true} = aggregate_query <-
+           Ash.Query.for_read(related, read_action, %{},
+             actor: opts[:actor],
+             tenant: opts[:tenant]
+           ),
+         %{valid?: true} = aggregate_query <-
+           Ash.Query.build(aggregate_query, filter: aggregate.filter, sort: aggregate.sort) do
+      Ash.Query.Aggregate.new(
+        resource,
+        aggregate.name,
+        aggregate.kind,
+        path: aggregate.relationship_path,
+        query: aggregate_query,
+        field: aggregate.field,
+        default: aggregate.default,
+        filterable?: aggregate.filterable?,
+        type: aggregate.type,
+        sortable?: aggregate.filterable?,
+        include_nil?: aggregate.include_nil?,
+        constraints: aggregate.constraints,
+        implementation: aggregate.implementation,
+        uniq?: aggregate.uniq?,
+        read_action: read_action,
+        authorize?: aggregate.authorize?
+      )
+    else
+      %{errors: errors} ->
+        {:error, errors}
+    end
+  end
+
   defp add_first_join_aggregate(
          query,
          _resource,

lib/sort.ex

@@ -43,52 +43,15 @@ defmodule AshSql.Sort do
               {calculation, val}
 
             %Ash.Resource.Aggregate{} = aggregate ->
-              related = Ash.Resource.Info.related(resource, aggregate.relationship_path)
-
-              read_action =
-                aggregate.read_action ||
-                  Ash.Resource.Info.primary_action!(
-                    related,
-                    :read
-                  ).name
-
-              with %{valid?: true} = aggregate_query <- Ash.Query.for_read(related, read_action),
-                   %{valid?: true} = aggregate_query <-
-                     Ash.Query.build(aggregate_query,
-                       filter: aggregate.filter,
-                       sort: aggregate.sort
-                     ) do
-                case Ash.Query.Aggregate.new(
-                       resource,
-                       aggregate.name,
-                       aggregate.kind,
-                       path: aggregate.relationship_path,
-                       query: aggregate_query,
-                       field: aggregate.field,
-                       default: aggregate.default,
-                       filterable?: aggregate.filterable?,
-                       type: aggregate.type,
-                       sortable?: aggregate.filterable?,
-                       include_nil?: aggregate.include_nil?,
-                       constraints: aggregate.constraints,
-                       implementation: aggregate.implementation,
-                       uniq?: aggregate.uniq?,
-                       read_action:
-                         aggregate.read_action ||
-                           Ash.Resource.Info.primary_action!(
-                             Ash.Resource.Info.related(resource, aggregate.relationship_path),
-                             :read
-                           ).name,
-                       authorize?: aggregate.authorize?
-                     ) do
-                  {:ok, agg} ->
-                    {agg, val}
-
-                  {:error, error} ->
-                    raise Ash.Error.to_ash_error(error)
-                end
-              else
-                %{errors: errors} -> raise Ash.Error.to_ash_error(errors)
+              case AshSql.Aggregate.resource_aggregate_to_aggregate(resource, aggregate,
+                     actor: query.__ash_bindings__.context[:private][:actor],
+                     tenant: query.__ash_bindings__.context[:private][:tenant]
+                   ) do
+                {:ok, agg} ->
+                  {agg, val}
+
+                {:error, error} ->
+                  raise Ash.Error.to_ash_error(error)
               end
 
             _ ->

test/aggregate_test.exs

@@ -0,0 +1,82 @@
+# SPDX-FileCopyrightText: 2024 ash_sql contributors <https://github.com/ash-project/ash_sql/graphs/contributors>
+#
+# SPDX-License-Identifier: MIT
+
+defmodule AshSql.AggregateTest do
+  use ExUnit.Case, async: true
+
+  defmodule Comment do
+    use Ash.Resource, domain: AshSql.AggregateTest.Domain, data_layer: Ash.DataLayer.Ets
+
+    attributes do
+      uuid_primary_key(:id)
+      attribute(:post_id, :uuid)
+      attribute(:score, :integer)
+    end
+
+    actions do
+      read :read_all do
+        primary?(true)
+      end
+    end
+  end
+
+  defmodule Post do
+    use Ash.Resource, domain: AshSql.AggregateTest.Domain, data_layer: Ash.DataLayer.Ets
+
+    attributes do
+      uuid_primary_key(:id)
+    end
+
+    relationships do
+      has_many(:comments, Comment, destination_attribute: :post_id)
+    end
+
+    aggregates do
+      max(:highest_score, :comments, :score)
+    end
+
+    actions do
+      defaults([:read])
+    end
+  end
+
+  defmodule Domain do
+    use Ash.Domain, validate_config_inclusion?: false
+
+    authorization do
+      require_actor?(true)
+    end
+
+    resources do
+      resource(Post)
+      resource(Comment)
+    end
+  end
+
+  defp build(opts) do
+    AshSql.Aggregate.resource_aggregate_to_aggregate(
+      Post,
+      Ash.Resource.Info.aggregate(Post, :highest_score),
+      opts
+    )
+  end
+
+  describe "resource_aggregate_to_aggregate/3" do
+    test "passes actor and tenant to the related read which may require an actor" do
+      actor = %{id: Ash.UUID.generate()}
+
+      assert {:ok, aggregate} = build(actor: actor, tenant: "acme")
+
+      assert aggregate.query.context.private.actor == actor
+      assert aggregate.query.tenant == "acme"
+    end
+
+    test "reads the related resource through its primary read action" do
+      assert {:ok, aggregate} = build(actor: %{id: Ash.UUID.generate()})
+
+      assert aggregate.query.resource == Comment
+      assert aggregate.read_action == :read_all
+    end
+  end
+end