Repository: elixir-ecto/ecto
Pull request #2478 (2018-03-26 19:06:16)

Update Ecto.Query inspect to include `on` with `assoc`

ebert[bot]: Ebert has finished reviewing this Pull Request and has found:

* 1 possible new issue (including those that may have been commented here).
* 1 fixed issue! 🎉

But beware that this branch is **[2 commits behind the `elixir-ecto:master` branch](https://github.com/elixir-ecto/ecto/compare/wojtekmach:wm-inspect-query-assoc-on...elixir-ecto:master)**, and a review of an up to date branch would produce more accurate results.

You can see more details about this review at https://ebertapp.io/github/elixir-ecto/ecto/pulls/2478.

<!-- Ebert auto generated comment -->


Patch:
diff --git a/lib/ecto/query/inspect.ex b/lib/ecto/query/inspect.ex
index 4c7e3144..0c4c6e94 100644
--- a/lib/ecto/query/inspect.ex
+++ b/lib/ecto/query/inspect.ex
@@ -1,7 +1,7 @@
 import Inspect.Algebra
 import Kernel, except: [to_string: 1]
 
-alias Ecto.Query.{DynamicExpr, JoinExpr}
+alias Ecto.Query.{DynamicExpr, JoinExpr, QueryExpr}
 
 defimpl Inspect, for: Ecto.Query.DynamicExpr do
   def inspect(%DynamicExpr{binding: binding} = dynamic, opts) do
@@ -89,9 +89,9 @@ defimpl Inspect, for: Ecto.Query do
     |> Enum.flat_map(fn {expr, ix} -> join(expr, elem(names, expr.ix || ix + 1), names) end)
   end
 
-  defp join(%JoinExpr{qual: qual, assoc: {ix, right}, as: as}, name, names) do
+  defp join(%JoinExpr{qual: qual, assoc: {ix, right}, on: on, as: as}, name, names) do
     string = "#{name} in assoc(#{elem(names, ix)}, #{inspect right})"
-    [{join_qual(qual), string}] ++ kw_inspect(:as, as)
+    [{join_qual(qual), string}] ++ kw_inspect(:as, as) ++ maybe_on(on, names)
   end
 
   defp join(%JoinExpr{qual: qual, source: {:fragment, _, _} = source, on: on, as: as} = part, name, names) do
@@ -104,6 +104,9 @@ defimpl Inspect, for: Ecto.Query do
     [{join_qual(qual), string}] ++ kw_inspect(:as, as) ++ [on: expr(on, names)]
   end
 
+  defp maybe_on(%QueryExpr{expr: true}, _names), do: []
+  defp maybe_on(%QueryExpr{} = on, names), do: [on: expr(on, names)]
+
   defp preloads([]),       do: []
   defp preloads(preloads), do: [preload: inspect(preloads)]
 


Test patch:
diff --git a/test/ecto/query/inspect_test.exs b/test/ecto/query/inspect_test.exs
index 256af560..6e059d8f 100644
--- a/test/ecto/query/inspect_test.exs
+++ b/test/ecto/query/inspect_test.exs
@@ -65,6 +65,9 @@ defmodule Ecto.Query.InspectTest do
   end
 
   test "join" do
+    assert i(from(x in Post, join: y in Comment)) ==
+           ~s{from p in Inspect.Post, join: c in Inspect.Comment, on: true}
+
     assert i(from(x in Post, join: y in Comment, on: x.id == y.id)) ==
            ~s{from p in Inspect.Post, join: c in Inspect.Comment, on: p.id == c.id}
 
@@ -77,6 +80,9 @@ defmodule Ecto.Query.InspectTest do
     assert i(from(x in Post, left_join: y in assoc(x, :comments))) ==
            ~s{from p in Inspect.Post, left_join: c in assoc(p, :comments)}
 
+    assert i(from(x in Post, left_join: y in assoc(x, :comments), on: y.published == true)) ==
+           ~s{from p in Inspect.Post, left_join: c in assoc(p, :comments), on: c.published == true}
+
     assert i(from(x in Post, right_join: y in assoc(x, :post), join: z in assoc(y, :post))) ==
            ~s{from p0 in Inspect.Post, right_join: p1 in assoc(p0, :post), join: p2 in assoc(p1, :post)}