From 8d79fd7d2a5f9e19409d8e2395fbbeabe3527342 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Wed, 12 Oct 2022 12:11:51 -0400 Subject: [PATCH] fix: non relay keyset pagination was broken when relay was introduced --- lib/graphql/resolver.ex | 73 ++++++++++++++++++++++++---------- lib/resource/resource.ex | 2 - test/paginate_test.exs | 53 +++++++++++++++++++++++- test/support/resources/post.ex | 5 +++ 4 files changed, 109 insertions(+), 24 deletions(-) diff --git a/lib/graphql/resolver.ex b/lib/graphql/resolver.ex index 9681ed7..d5e139a 100644 --- a/lib/graphql/resolver.ex +++ b/lib/graphql/resolver.ex @@ -144,7 +144,7 @@ defmodule AshGraphql.Graphql.Resolver do def resolve( %{arguments: args, context: context} = resolution, - {api, resource, %{type: :list, action: action, modify_resolution: modify}} + {api, resource, %{type: :list, relay?: relay?, action: action, modify_resolution: modify}} ) do opts = [ actor: Map.get(context, :actor), @@ -157,7 +157,7 @@ defmodule AshGraphql.Graphql.Resolver do {result, modify_args} = with {:ok, opts} <- validate_resolve_opts(resolution, pagination, opts, args), - result_fields <- get_result_fields(pagination), + result_fields <- get_result_fields(pagination, relay?), initial_query <- query |> Ash.Query.set_tenant(Map.get(context, :tenant)) @@ -172,7 +172,7 @@ defmodule AshGraphql.Graphql.Resolver do authorize?: AshGraphql.Api.Info.authorize?(api) ) |> api.read(opts) do - result = paginate(resource, action, page) + result = paginate(resource, action, page, relay?) {result, [query, result]} else {:error, error} -> @@ -279,15 +279,19 @@ defmodule AshGraphql.Graphql.Resolver do {:ok, opts} end - defp get_result_fields(%{keyset?: true}) do + defp get_result_fields(%{keyset?: true}, true) do ["edges", "node"] end - defp get_result_fields(%{offset?: true}) do + defp get_result_fields(%{keyset?: true}, false) do ["results"] end - defp get_result_fields(_pagination) do + defp get_result_fields(%{offset?: true}, _) do + ["results"] + end + + defp get_result_fields(_pagination, _) do [] end @@ -299,12 +303,17 @@ defmodule AshGraphql.Graphql.Resolver do [] end - defp paginate(_resource, _action, %Ash.Page.Keyset{ - results: results, - more?: more, - after: after_cursor, - before: before_cursor - }) do + defp paginate( + _resource, + _action, + %Ash.Page.Keyset{ + results: results, + more?: more, + after: after_cursor, + before: before_cursor + }, + true + ) do {start_cursor, end_cursor} = case results do [] -> @@ -351,22 +360,44 @@ defmodule AshGraphql.Graphql.Resolver do } end - defp paginate(_resource, _action, %Ash.Page.Offset{results: results, count: count}) do + defp paginate( + _resource, + _action, + %Ash.Page.Keyset{ + results: results, + count: count + }, + false + ) do {:ok, %{results: results, count: count}} end - defp paginate(resource, action, page) do + defp paginate(_resource, _action, %Ash.Page.Offset{results: results, count: count}, _) do + {:ok, %{results: results, count: count}} + end + + defp paginate(resource, action, page, relay?) do case Ash.Resource.Info.action(resource, action).pagination do %{offset?: true} -> - paginate(resource, action, %Ash.Page.Offset{results: page, count: Enum.count(page)}) + paginate( + resource, + action, + %Ash.Page.Offset{results: page, count: Enum.count(page)}, + relay? + ) %{keyset?: true} -> - paginate(resource, action, %Ash.Page.Keyset{ - results: page, - more?: false, - after: nil, - before: nil - }) + paginate( + resource, + action, + %Ash.Page.Keyset{ + results: page, + more?: false, + after: nil, + before: nil + }, + relay? + ) _ -> {:ok, page} diff --git a/lib/resource/resource.ex b/lib/resource/resource.ex index 3da165e..8a347d1 100644 --- a/lib/resource/resource.ex +++ b/lib/resource/resource.ex @@ -1078,7 +1078,6 @@ defmodule AshGraphql.Resource do name: "first", identifier: :first, type: :integer, - default_value: action.pagination.default_limit, description: "The number of records to return from the beginning." <> max_message, __reference__: ref(__ENV__) }, @@ -1100,7 +1099,6 @@ defmodule AshGraphql.Resource do name: "last", identifier: :last, type: :integer, - default_value: action.pagination.default_limit, description: "The number of records to return to the end." <> max_message, __reference__: ref(__ENV__) } diff --git a/test/paginate_test.exs b/test/paginate_test.exs index 6f7866c..5935c1d 100644 --- a/test/paginate_test.exs +++ b/test/paginate_test.exs @@ -9,7 +9,58 @@ defmodule AshGraphql.PaginateTest do end) end - describe "pagination" do + describe "keyset pagination" do + setup do + letters = ["a", "b", "c", "d", "e"] + + for text <- letters do + post = + AshGraphql.Test.Post + |> Ash.Changeset.for_create(:create, text: text, published: true) + |> AshGraphql.Test.Api.create!() + + for text <- letters do + AshGraphql.Test.Comment + |> Ash.Changeset.for_create(:create, text: text) + |> Ash.Changeset.manage_relationship(:post, post, type: :append_and_remove) + |> AshGraphql.Test.Api.create!() + end + end + + :ok + end + + test "default_limit records are fetched" do + doc = """ + query KeysetPaginatedPosts { + keysetPaginatedPosts(sort: [{field: TEXT}]) { + count + results{ + text + } + } + } + """ + + assert {:ok, + %{ + data: %{ + "keysetPaginatedPosts" => %{ + "count" => 5, + "results" => [ + %{"text" => "a"}, + %{"text" => "b"}, + %{"text" => "c"}, + %{"text" => "d"}, + %{"text" => "e"} + ] + } + } + }} = Absinthe.run(doc, AshGraphql.Test.Schema) + end + end + + describe "offset pagination" do setup do letters = ["a", "b", "c", "d", "e"] diff --git a/test/support/resources/post.ex b/test/support/resources/post.ex index 7095a1c..77ffe7f 100644 --- a/test/support/resources/post.ex +++ b/test/support/resources/post.ex @@ -77,6 +77,7 @@ defmodule AshGraphql.Test.Post do list :post_library, :library list :post_score, :score list :paginated_posts, :paginated + list :keyset_paginated_posts, :keyset_paginated list :paginated_posts_without_limit, :paginated_without_limit list :paginated_posts_limit_not_required, :paginated_limit_not_required end @@ -160,6 +161,10 @@ defmodule AshGraphql.Test.Post do pagination(required?: true, offset?: true, countable: true, default_limit: 20) end + read :keyset_paginated do + pagination(required?: true, keyset?: true, countable: true, default_limit: 20) + end + read :paginated_without_limit do pagination(required?: true, offset?: true, countable: true) end