Merge pull request #16 from graphql-elixir/bugfix/ecto-order-by-clashing-with-counts

Bugfix/ecto order by clashing with counts

Author: seanabrahams

README.md

@@ -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

lib/graphql/relay/connection/ecto.ex

@@ -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
@@ -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
@@ -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

mix.exs

@@ -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"
 

priv/repo/migrations/20160307040347_create_letter.exs

@@ -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

test/graphql/relay/connection/ecto_test.exs

@@ -11,6 +11,7 @@ defmodule GraphQL.Relay.Connection.EctoTest do
 
     schema "letters" do
       field :letter, :string
+      field :second_column, :string
       timestamps
     end
   end
@@ -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