diff --git a/lib/graphql/relay/connection/ecto.ex b/lib/graphql/relay/connection/ecto.ex index 384971e..57c5310 100644 --- a/lib/graphql/relay/connection/ecto.ex +++ b/lib/graphql/relay/connection/ecto.ex @@ -82,14 +82,44 @@ if Code.ensure_loaded?(Ecto) do GraphQL.Relay.Connection.Ecto.resolve(Repo, query, args) end """ + defp expr({{:., _, [{:&, _, [idx]}, field]}, _, []}, sources, _query) when is_atom(field) do + {_, name, _} = elem(sources, idx) + "#{name}.#{quote_name(field)}" + end def resolve(repo, query, args \\ %{}) do before = cursor_to_offset(args[:before]) # `after` is a keyword http://elixir-lang.org/docs/master/elixir/Kernel.SpecialForms.html#try/1 a_after = cursor_to_offset(args[:after]) first = args[:first] last = args[:last] - ordered_by_property = args[:ordered_by] || :id - ordered_by_direction = get_ordered_by_direction(args[:ordered_by_direction] || :asc) + + # Deprecation messages + # Emit deprecation warning once we hit >= v0.7 per RELEASE.md#Deprecations + # case args do + # %{ordered_by_direction: _} -> + # :elixir_errors.warn __ENV__.line, __ENV__.file, "Use of :ordered_by_direction in args is deprecated as it's no longer needed! Use `Ecto.Query.order_by`." + # %{ordered_by: _} -> + # :elixir_errors.warn __ENV__.line, __ENV__.file, "Use of :ordered_by in args is deprecated as it's no longer needed! Use `Ecto.Query.order_by`." + # end + IO.inspect query.order_bys + ordered_by_direction = case query.order_bys do + [] -> get_ordered_by_direction(args[:ordered_by_direction] || :asc) + [%Ecto.Query.QueryExpr{} = query_expr] -> + case query_expr.expr do + [asc: _] -> :asc + [desc: _] -> :desc + end + end + IO.inspect ordered_by_direction + ordered_by_property = case query.order_bys do + [] -> + args[:ordered_by] || :id + [%Ecto.Query.QueryExpr{expr: [asc: {{:., _, [{:&, [], [idx]}, field]}, [], []}]}] -> + field + [%Ecto.Query.QueryExpr{expr: [desc: {{:., _, [{:&, [], [idx]}, field]}, [], []}]}] -> + field + end + IO.inspect ordered_by_property opposite_ordered_by_direction = if ordered_by_direction == :asc, do: :desc, else: :asc limit = Enum.min([first, last, connection_count(repo, query)]) @@ -135,15 +165,26 @@ if Code.ensure_loaded?(Ecto) do end query = if first do - sort_values = Keyword.new([{ordered_by_direction, ordered_by_property}]) - query |> order_by(^sort_values) |> limit(^limit) + case query.order_bys do + [] -> + sort_values = Keyword.new([{ordered_by_direction, ordered_by_property}]) + query |> order_by(^sort_values) |> limit(^limit) + [%Ecto.Query.QueryExpr{} = query_expr] -> + query |> limit(^limit) + end else query end query = if last do - sort_values = Keyword.new([{opposite_ordered_by_direction, ordered_by_property}]) - query |> order_by(^sort_values) |> limit(^limit) + case query.order_bys do + [] -> + sort_values = Keyword.new([{opposite_ordered_by_direction, ordered_by_property}]) + query |> order_by(^sort_values) |> limit(^limit) + [%Ecto.Query.QueryExpr{} = query_expr] -> + # TODO: Update order_by direction to opposite direction + query |> limit(^limit) + end else query end diff --git a/test/graphql/relay/connection/ecto_test.exs b/test/graphql/relay/connection/ecto_test.exs index e27ed1c..5bf226e 100644 --- a/test/graphql/relay/connection/ecto_test.exs +++ b/test/graphql/relay/connection/ecto_test.exs @@ -53,12 +53,12 @@ defmodule GraphQL.Relay.Connection.EctoTest do b = Repo.insert!(%Letter{letter: "b", order: 101}) c = Repo.insert!(%Letter{letter: "c", order: 102}) d = Repo.insert!(%Letter{letter: "d", order: 103}) - Repo.insert(%Letter{letter: "e", order: 104}) + e = Repo.insert(%Letter{letter: "e", order: 104}) - Repo.insert!(%Number{number: 1, letter_id: a.id}) - Repo.insert!(%Number{number: 2, letter_id: b.id}) - Repo.insert!(%Number{number: 3, letter_id: c.id}) - Repo.insert!(%Number{number: 4, letter_id: d.id}) + one = Repo.insert!(%Number{number: 1, letter_id: a.id}) + two = Repo.insert!(%Number{number: 2, letter_id: b.id}) + three = Repo.insert!(%Number{number: 3, letter_id: c.id}) + four = Repo.insert!(%Number{number: 4, letter_id: d.id}) :ok end @@ -70,6 +70,10 @@ defmodule GraphQL.Relay.Connection.EctoTest do from l in Letter end + def numbers do + Repo.all(Number) + end + def a do Enum.at(letters, 0) end @@ -90,6 +94,22 @@ defmodule GraphQL.Relay.Connection.EctoTest do Enum.at(letters, 4) end + def one do + Enum.at(numbers, 0) + end + + def two do + Enum.at(numbers, 1) + end + + def three do + Enum.at(numbers, 2) + end + + def four do + Enum.at(numbers, 3) + end + defp edge_for_object(obj), do: edge_for_object(obj, :id) defp edge_for_object(obj, ordered_by) do %{ @@ -235,6 +255,25 @@ defmodule GraphQL.Relay.Connection.EctoTest do assert(Connection.Ecto.resolve(Repo, letters_query, %{first: 2, after: edge_for_object(b, :order).cursor, ordered_by: :order}) == expected) end + test "pagination: respects last and before with non-default ordered_by property" do + expected = %{ + edges: [ + edge_for_object(b, :order), + edge_for_object(c, :order), + ], + pageInfo: %{ + startCursor: edge_for_object(b, :order).cursor, + endCursor: edge_for_object(c, :order).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + # TODO: Confirm that this works as expected + # What should the order be with last and before? + # What should the values for hasPreviousPage and hasNextPage be? + assert(Connection.Ecto.resolve(Repo, letters_query, %{last: 2, before: edge_for_object(d, :order).cursor, ordered_by: :order}) == expected) + end + test "pagination: respects first and after with non-default ordered_by and ordered_by_direction" do expected = %{ edges: [ @@ -251,6 +290,242 @@ defmodule GraphQL.Relay.Connection.EctoTest do assert(Connection.Ecto.resolve(Repo, letters_query, %{first: 2, after: edge_for_object(e, :order).cursor, ordered_by: :order, ordered_by_direction: :desc}) == expected) end + test "pagination: respects last and before with non-default ordered_by and ordered_by_direction" do + expected = %{ + edges: [ + edge_for_object(d, :order), + edge_for_object(c, :order), + ], + pageInfo: %{ + startCursor: edge_for_object(d, :order).cursor, + endCursor: edge_for_object(c, :order).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + assert(Connection.Ecto.resolve(Repo, letters_query, %{last: 2, before: edge_for_object(e, :order).cursor, ordered_by: :order, ordered_by_direction: :desc}) == expected) + end + + test "pagination: respects first and after with order by introspection" do + expected = %{ + edges: [ + edge_for_object(c, :order), + edge_for_object(d, :order), + ], + pageInfo: %{ + startCursor: edge_for_object(c, :order).cursor, + endCursor: edge_for_object(d, :order).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + query = letters_query + |> order_by(asc: :order) + assert(Connection.Ecto.resolve(Repo, query, %{first: 2, after: edge_for_object(b, :order).cursor}) == expected) + end + + test "pagination: respects last and before with order by introspection" do + expected = %{ + edges: [ + edge_for_object(c, :order), + edge_for_object(d, :order), + ], + pageInfo: %{ + startCursor: edge_for_object(c, :order).cursor, + endCursor: edge_for_object(d, :order).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + query = letters_query + |> order_by(asc: :order) + assert(Connection.Ecto.resolve(Repo, query, %{last: 2, before: edge_for_object(e, :order).cursor}) == expected) + end + + test "pagination: respects first and after with ordered and ordered direction introspection" do + expected = %{ + edges: [ + edge_for_object(d, :order), + edge_for_object(c, :order), + ], + pageInfo: %{ + startCursor: edge_for_object(d, :order).cursor, + endCursor: edge_for_object(c, :order).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + query = letters_query + |> order_by(desc: :order) + assert(Connection.Ecto.resolve(Repo, query, %{first: 2, after: edge_for_object(e, :order).cursor}) == expected) + end + + test "pagination: respects last and before with ordered and ordered direction introspection" do + expected = %{ + edges: [ + edge_for_object(d, :order), + edge_for_object(c, :order), + ], + pageInfo: %{ + startCursor: edge_for_object(d, :order).cursor, + endCursor: edge_for_object(c, :order).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + query = letters_query + |> order_by(desc: :order) + assert(Connection.Ecto.resolve(Repo, query, %{last: 2, before: edge_for_object(e, :order).cursor}) == expected) + end + + test "pagination: respects first and after with order_by on joined field" do + expected = %{ + edges: [ + edge_for_object(two, :number), + edge_for_object(three, :number), + ], + pageInfo: %{ + startCursor: edge_for_object(two, :number).cursor, + endCursor: edge_for_object(three, :number).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + query = Letter + |> join(:left, [l], n in Number, (l.id == n.letter_id)) + |> order_by([l,n], n.number) + assert(Connection.Ecto.resolve(Repo, query, %{first: 2, after: edge_for_object(one, :number).cursor}) == expected) + end + + test "pagination: respects last and before with order_by on joined field" do + expected = %{ + edges: [ + edge_for_object(two, :number), + edge_for_object(three, :number), + ], + pageInfo: %{ + startCursor: edge_for_object(two, :number).cursor, + endCursor: edge_for_object(three, :number).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + query = Letter + |> join(:left, [l], n in Number, (l.id == n.letter_id)) + |> order_by([l,n], n.number) + assert(Connection.Ecto.resolve(Repo, query, %{last: 2, before: edge_for_object(one, :number).cursor}) == expected) + end + + test "pagination: respects first and after with order_by on joined field with explicit direction" do + expected = %{ + edges: [ + edge_for_object(three, :number), + edge_for_object(two, :number), + ], + pageInfo: %{ + startCursor: edge_for_object(three, :number).cursor, + endCursor: edge_for_object(two, :number).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + query = Letter + |> join(:left, [l], n in Number, (l.id == n.letter_id)) + |> order_by([l,n], desc: n.number) + assert(Connection.Ecto.resolve(Repo, query, %{first: 2, after: edge_for_object(four, :number).cursor}) == expected) + end + + test "pagination: respects last and before with order_by on joined field with explicit direction" do + expected = %{ + edges: [ + edge_for_object(three, :number), + edge_for_object(two, :number), + ], + pageInfo: %{ + startCursor: edge_for_object(three, :number).cursor, + endCursor: edge_for_object(two, :number).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + query = Letter + |> join(:left, [l], n in Number, (l.id == n.letter_id)) + |> order_by([l,n], desc: n.number) + assert(Connection.Ecto.resolve(Repo, query, %{last: 2, before: edge_for_object(four, :number).cursor}) == expected) + end + + test "pagination: respects first and after with multiple order_bys" do + expected = %{ + edges: [ + edge_for_object(c, :order), + edge_for_object(d, :order), + ], + pageInfo: %{ + startCursor: edge_for_object(c, :order).cursor, + endCursor: edge_for_object(d, :order).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + query = letters_query + |> order_by([l], asc: l.order, desc: l.letter) + assert(Connection.Ecto.resolve(Repo, query, %{first: 2, after: edge_for_object(b, :order).cursor}) == expected) + end + + test "pagination: respects last and before with multiple order_bys" do + expected = %{ + edges: [ + edge_for_object(c, :order), + edge_for_object(d, :order), + ], + pageInfo: %{ + startCursor: edge_for_object(c, :order).cursor, + endCursor: edge_for_object(d, :order).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + query = letters_query + |> order_by([l], asc: l.order, desc: l.letter) + assert(Connection.Ecto.resolve(Repo, query, %{last: 2, before: edge_for_object(b, :order).cursor}) == expected) + end + + test "pagination: respects first and after with multiple order_bys with explicit direction" do + expected = %{ + edges: [ + edge_for_object(d, :order), + edge_for_object(c, :order), + ], + pageInfo: %{ + startCursor: edge_for_object(d, :order).cursor, + endCursor: edge_for_object(c, :order).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + query = letters_query + |> order_by([l], desc: l.order, asc: l.letter) + assert(Connection.Ecto.resolve(Repo, query, %{first: 2, after: edge_for_object(e, :order).cursor}) == expected) + end + + test "pagination: respects last and before with multiple order_bys with explicit direction" do + expected = %{ + edges: [ + edge_for_object(d, :order), + edge_for_object(c, :order), + ], + pageInfo: %{ + startCursor: edge_for_object(d, :order).cursor, + endCursor: edge_for_object(c, :order).cursor, + hasPreviousPage: false, + hasNextPage: true, + } + } + query = letters_query + |> order_by([l], desc: l.order, asc: l.letter) + assert(Connection.Ecto.resolve(Repo, query, %{last: 2, before: edge_for_object(e, :order).cursor}) == expected) + end + test "respects first and after with long first" do expected = %{ edges: [