Skip to content

feat: Relationship Through#2567

Merged
zachdaniel merged 29 commits intoash-project:mainfrom
ken-kost:feat/relationship-through-2
Apr 12, 2026
Merged

feat: Relationship Through#2567
zachdaniel merged 29 commits intoash-project:mainfrom
ken-kost:feat/relationship-through-2

Conversation

@ken-kost
Copy link
Copy Markdown
Contributor

@ken-kost ken-kost commented Feb 17, 2026

Contributor checklist

#72
#1780

Related to: ash-project/ash_postgres#686 & ash-project/ash_sql#212

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

@ken-kost
Copy link
Copy Markdown
Contributor Author

@zachdaniel Reponed.

I force pushed something to main and it looks like it closed all PRs and broke the history somehow

Interesting. & scary - force pushing to main 😆

@zachdaniel
Copy link
Copy Markdown
Contributor

Yeah it's a long story 😢 I literally force pushed an entire different project to ash main. Was ridiculous (and it wasn't claude, it was just me 🤦)

@ken-kost ken-kost marked this pull request as ready for review March 2, 2026 12:06
@ken-kost
Copy link
Copy Markdown
Contributor Author

ken-kost commented Mar 2, 2026

Okay, I added some tests all over the place. On the last one I found missing tenant propagation. ash_postgres still has my initial suit of tests. Is this enough? 🤔

@zachdaniel
Copy link
Copy Markdown
Contributor

Will reevaluate in a few days! Thanks for your hard work! 🙇

Comment thread lib/ash/resource/relationships/belongs_to.ex Outdated
@zachdaniel
Copy link
Copy Markdown
Contributor

I think the only other missing piece here is a verifier that runs to confirm that all relationships along the path exist and match.

@ken-kost ken-kost requested a review from zachdaniel March 5, 2026 13:28
@zachdaniel
Copy link
Copy Markdown
Contributor

This PR doesn't appear to test authorization on intermediate hops for through
relationships. Looking at the lateral join path in relationships.ex, the new
is_list(Map.get(relationship, :through)) branch builds the path without
calling Ash.can on intermediate queries, unlike the many_to_many branch which
explicitly does.

Here's a minimal test that demonstrates the issue:

  defmodule Ash.Test.Actions.ThroughAuthorizationTest do
    use ExUnit.Case, async: true

    alias Ash.Test.Actions.ThroughAuthorizationTest, as: ThisTest

    defmodule Domain do
      use Ash.Domain
      resources do
        allow_unregistered? true
      end
    end

    defmodule Post do
      use Ash.Resource, domain: ThisTest.Domain, data_layer: Ash.DataLayer.Ets
      ets do private?(true) end
      actions do
        default_accept :*
        defaults [:read, :destroy, create: :*, update: :*]
      end
      attributes do
        uuid_primary_key :id
        attribute :title, :string, public?: true
      end
      relationships do
        has_many :post_links, ThisTest.PostLink, public?: true,
  destination_attribute: :source_id
        has_many :linked_posts, __MODULE__, through: [:post_links, :destination]
      end
    end

    defmodule PostLink do
      use Ash.Resource,
        domain: ThisTest.Domain,
        data_layer: Ash.DataLayer.Ets,
        authorizers: [Ash.Policy.Authorizer]
      ets do private? true end
      actions do
        default_accept :*
        defaults [:read, :destroy, create: :*, update: :*]
      end
      policies do
        policy action_type(:create) do
          authorize_if always()
        end
        policy action_type(:read) do
          authorize_if expr(visible == true)
        end
      end
      attributes do
        uuid_primary_key :id
        attribute :source_id, :uuid, public?: true
        attribute :visible, :boolean, public?: true, default: true
      end
      relationships do
        belongs_to :destination, ThisTest.Post, public?: true
      end
    end

    test "loading a through relationship respects policies on the intermediate
  resource" do
      source = Post |> Ash.Changeset.for_create(:create, %{title: "source"}) |>
  Ash.create!()
      visible_dest = Post |> Ash.Changeset.for_create(:create, %{title:
  "visible"}) |> Ash.create!()
      hidden_dest = Post |> Ash.Changeset.for_create(:create, %{title:
  "hidden"}) |> Ash.create!()

      PostLink
      |> Ash.Changeset.for_create(:create, %{source_id: source.id,
  destination_id: visible_dest.id, visible: true})
      |> Ash.create!()

      PostLink
      |> Ash.Changeset.for_create(:create, %{source_id: source.id,
  destination_id: hidden_dest.id, visible: false})
      |> Ash.create!()

      # Without auth — both links traversable
      assert length(Ash.load!(source, :linked_posts, authorize?:
  false).linked_posts) == 2

      # With auth — PostLink policy should filter out the hidden link
      result = Ash.load!(source, :linked_posts, authorize?: true, actor: %{})
      assert length(result.linked_posts) == 1
      assert hd(result.linked_posts).title == "visible"
    end
  end

This fails — result.linked_posts returns 2 records instead of 1, confirming
that the intermediate PostLink read policy (visible == true) is not being
enforced during through relationship loading.

@zachdaniel
Copy link
Copy Markdown
Contributor

found an issue, had claude put the above together

@ken-kost
Copy link
Copy Markdown
Contributor Author

@zachdaniel Reminder to check this; I addressed the last issue. Should I look for more bugs, add more tests?

@zachdaniel
Copy link
Copy Markdown
Contributor

Okay this looks good. One final thought: we need to add a capability in Ash.DataLayer for "can do through relationships", and then add a transformer that checks if a resource's data layer can do through relationships. If not, we should show an error. That will let me merge this one without someone thinking they can use that feature before teh postgres/sql ones are merged.

Great work, we're almost there!

@ken-kost
Copy link
Copy Markdown
Contributor Author

Done! I get the warning when running non local tests. 👍

@zachdaniel zachdaniel merged commit 24f41e2 into ash-project:main Apr 12, 2026
45 checks passed
@zachdaniel
Copy link
Copy Markdown
Contributor

🚀 Thank you for your contribution! 🚀

Just want to call out that this feature is a long standing ask from the community and took a lot of grit to get through all the different angles. Really well done 🔥

@ken-kost
Copy link
Copy Markdown
Contributor Author

🚀 Thank you for your contribution! 🚀

Just want to call out that this feature is a long standing ask from the community and took a lot of grit to get through all the different angles. Really well done 🔥

Yea, reaching the goal feels great! For me as well, this feature was in my mind from day one of finding ash. But I feel I appreciate the journey even more than the goal. & you play a big part in that, so thank you as well. 😄

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

Successfully merging this pull request may close these issues.

2 participants