Skip to content

Bugfix/ecto order by clashing with counts #16

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

Merged
merged 2 commits into from
Mar 25, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ It's important that you understand GraphQL first and then Relay second. Relay is

def deps do
[
{:graphql_relay, "~> 0.0.15"},
{:graphql_relay, "~> 0.0.16"},
{:plug_graphql, "~> 0.2"} # Most likely going to need this as well
]
end
Expand Down
20 changes: 16 additions & 4 deletions lib/graphql/relay/connection/ecto.ex
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ if Code.ensure_loaded?(Ecto) do
nil -> false
_ ->
first_limit = first + 1
has_more_records_query = remove_select(from things in query, limit: ^first_limit)
has_more_records_query = make_query_countable(from things in query, limit: ^first_limit)
has_more_records_query = from things in has_more_records_query, select: count(things.id)
repo.one(has_more_records_query) > first
end
Expand All @@ -39,7 +39,7 @@ if Code.ensure_loaded?(Ecto) do
nil -> false
_ ->
last_limit = last + 1
has_prev_records_query = remove_select(from things in query, limit: ^last_limit)
has_prev_records_query = make_query_countable(from things in query, limit: ^last_limit)
has_prev_records_query = from things in has_prev_records_query, select: count(things.id)
repo.one(has_prev_records_query) > last
end
Expand Down Expand Up @@ -114,15 +114,27 @@ if Code.ensure_loaded?(Ecto) do
end

def connection_count(repo, query) do
query = remove_select(query)
query = make_query_countable(query)
count_query = from things in query, select: count(things.id)
repo.one(count_query)
end

defp make_query_countable(query) do
query
|> remove_select
|> remove_order
end

# Remove select if it exists so that we avoid `only one select
# expression is allowed in query` Ecto exception
defp remove_select(query) do
%{ query | select: nil }
query |> Ecto.Query.exclude(:select)
end

# Remove order by if it exists so that we avoid `field X in "order_by"
# does not exist in the model source in query`
defp remove_order(query) do
query |> Ecto.Query.exclude(:order_by)
end
end
end
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
defmodule GraphQL.Relay.Mixfile do
use Mix.Project

@version "0.0.15"
@version "0.0.16"
@description "Elixir implementation of Relay for GraphQL"
@repo_url "https://github.com/graphql-elixir/graphql_relay"

Expand Down
1 change: 1 addition & 0 deletions priv/repo/migrations/20160307040347_create_letter.exs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ defmodule EctoTest.Repo.Migrations.CreateLetter do
def change do
create table(:letters) do
add :letter, :string
add :second_column, :string
timestamps
end
end
Expand Down
2 changes: 2 additions & 0 deletions test/graphql/relay/connection/ecto_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ defmodule GraphQL.Relay.Connection.EctoTest do

schema "letters" do
field :letter, :string
field :second_column, :string
timestamps
end
end
Expand Down Expand Up @@ -62,6 +63,7 @@ defmodule GraphQL.Relay.Connection.EctoTest do
test "querying for counts does not raise exception if select already exists" do
query = letters_query
|> select([l], %{id: l.id, letter: l.letter})
|> order_by([l], asc: l.second_column)
assert(Connection.Ecto.resolve(query, %{repo: Repo}))
end

Expand Down