Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: subscription dsl #97

Merged
merged 48 commits into from
Sep 27, 2024
Merged

feat: subscription dsl #97

merged 48 commits into from
Sep 27, 2024

Conversation

barnabasJ
Copy link
Contributor

@barnabasJ barnabasJ commented Oct 15, 2023

Contributor checklist

  • Bug fixes include regression tests
  • Features include unit/acceptance tests

@barnabasJ
Copy link
Contributor Author

@zachdaniel I tried to create a test case for the current behaviour before trying to move the code into the dsl but I got a bit stuck.

I copied stuff from https://github.com/absinthe-graphql/absinthe/blob/master/test/absinthe/execution/subscription_test.exs and https://github.com/ash-project/ash/blob/main/test/notifier/pubsub_test.exs but I can't see any messages. I think I'm missing something, Maybe you can give me a hint to get unstuck.

Thanks

@zachdaniel
Copy link
Contributor

Will take a look tomorrow 👍

@zachdaniel
Copy link
Contributor

The issue doesn't jump out to me immediately...the first step might be to add a custom notifier and make sure notifications are going out at all.

@barnabasJ
Copy link
Contributor Author

build_read_one_query
|> Ash.can(....)
|> case do
  {true, query} ->
     if query.authorize_results == [] && we_can_tell_the_record_matches(record, query.filter) do
      just load any necessary data on the record we got
    else
      do a read one w/ the load
    end
end

Can you explain what query.authorize_results == [] means? Because I'm not sure what is stored there and it makes me unsure of what to do if Ash.can only returns {:ok, true}.

for we_can_tell_the_record_matches can I use Ash.Filter.Runtime.filter_matches(query.domain, [data], query.filter)?

@zachdaniel
Copy link
Contributor

query.authorize_results is a set of hooks that authorizers can add that run after the query is run. If you just get back {:ok, true} then that can be treated as the same as query.authorize_results == [] (it means there is nothing else to do). Although IIRC if you pass alter_source?: true you're guaranteed to get back the query.

For evaluating the filter against the record, I'd use something like Ash.Expr.eval(expression, resource: resource, unknown_on_unknown_refs?: true). That will do a sort of "shallow check" without running any queries or anything. If that returns :unknown then you have to run the query. Otherwise you know what the result will be.

@barnabasJ barnabasJ marked this pull request as ready for review September 27, 2024 10:04
@barnabasJ barnabasJ self-assigned this Sep 27, 2024
@barnabasJ barnabasJ added the enhancement New feature or request label Sep 27, 2024
zachdaniel
zachdaniel previously approved these changes Sep 27, 2024
Copy link
Contributor

@zachdaniel zachdaniel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 lets gooooooooo.

Awesome work!

@zachdaniel
Copy link
Contributor

Feel free to merge at your convenience :)

@barnabasJ
Copy link
Contributor Author

@zachdaniel accepting your suggestion dismissed your approval, now merging is blocked again 😅

@barnabasJ barnabasJ merged commit 4c377e5 into main Sep 27, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants