From 4f4b4079d809755b718bb5fd1f59303fef9f25f8 Mon Sep 17 00:00:00 2001 From: Jason Pollentier Date: Mon, 28 Oct 2024 09:49:25 -0600 Subject: [PATCH 1/4] chore: reproduce the bug reported in #185 --- test/soft_delete_repo_test.exs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/soft_delete_repo_test.exs b/test/soft_delete_repo_test.exs index 98c6da6..d1cc1da 100644 --- a/test/soft_delete_repo_test.exs +++ b/test/soft_delete_repo_test.exs @@ -115,6 +115,16 @@ defmodule Ecto.SoftDelete.Repo.Test do assert Enum.member?(results, soft_deleted_user) end + test "handles subquery that does not expose deleted_at as 'main' schema" do + Repo.insert!(%User{email: "test0@example.com"}) + Repo.insert!(%Nondeletable{value: "stuff"}) + + subq = User |> where([u], u.email == "test0@example.com") |> select([u], %{id: u.id, email: u.email}) + query = subquery(subq) |> join(:left, [u], nondel in Nondeletable, on: u.id == nondel.id) + + Repo.all(query) + end + test "includes soft deleted records if where not is_nil(deleted_at) clause is present" do user = Repo.insert!(%User{email: "test0@example.com"}) From 36ebba394a34051e0c07f973043668d2c597e886 Mon Sep 17 00:00:00 2001 From: Glen Holcomb Date: Wed, 30 Oct 2024 15:49:15 -0600 Subject: [PATCH 2/4] Ignore `.lexical` --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index b9d8296..71660da 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,4 @@ erl_crash.dump *.ez /.elixir_ls +/.lexical From 35f376197f961a14e6154b27d35456193700b03d Mon Sep 17 00:00:00 2001 From: Glen Holcomb Date: Wed, 30 Oct 2024 15:52:41 -0600 Subject: [PATCH 3/4] Recurse query and apply filtering directly as appropriate --- lib/ecto/soft_delete_repo.ex | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/ecto/soft_delete_repo.ex b/lib/ecto/soft_delete_repo.ex index f0bc4ea..8de61b4 100644 --- a/lib/ecto/soft_delete_repo.ex +++ b/lib/ecto/soft_delete_repo.ex @@ -81,11 +81,37 @@ defmodule Ecto.SoftDelete.Repo do if has_include_deleted_at_clause?(query) || opts[:with_deleted] || !soft_deletable?(query) do {query, opts} else - query = from(x in query, where: is_nil(x.deleted_at)) + query = filter_soft_deleted(query) {query, opts} end end + # We need to check the entire query and apply filtering + # where appropriate. So, we recurse the query here and + # rebuild it with filtering where appropriate. This + # currently only considers the source and does not handle + # things like joins... + defp filter_soft_deleted(%Ecto.Query{from: %{source: {_schema, _module}}} = query) do + if Ecto.SoftDelete.Query.soft_deletable?(query) do + from(x in query, where: is_nil(x.deleted_at)) + else + query + end + end + + defp filter_soft_deleted(%Ecto.SubQuery{query: query} = sub) do + if Ecto.SoftDelete.Query.soft_deletable?(query) do + from(x in query, where: is_nil(x.deleted_at)) |> subquery() + else + sub + end + end + + defp filter_soft_deleted(%Ecto.Query{from: from} = query) do + updated_from = %{from | source: filter_soft_deleted(from.source)} + %{query | from: updated_from} + end + # Checks the query to see if it contains a where not is_nil(deleted_at) # if it does, we want to be sure that we don't exclude soft deleted records defp has_include_deleted_at_clause?(%Ecto.Query{wheres: wheres}) do From 5fe190cd48919ac39d23dec628567498764ce890 Mon Sep 17 00:00:00 2001 From: Glen Holcomb Date: Sat, 2 Nov 2024 05:13:19 -0600 Subject: [PATCH 4/4] Update lib/ecto/soft_delete_repo.ex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don’t throw away other possible subquery context. Co-authored-by: Jason Pollentier <802805+grossvogel@users.noreply.github.com> --- lib/ecto/soft_delete_repo.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ecto/soft_delete_repo.ex b/lib/ecto/soft_delete_repo.ex index 8de61b4..9a0e782 100644 --- a/lib/ecto/soft_delete_repo.ex +++ b/lib/ecto/soft_delete_repo.ex @@ -101,7 +101,7 @@ defmodule Ecto.SoftDelete.Repo do defp filter_soft_deleted(%Ecto.SubQuery{query: query} = sub) do if Ecto.SoftDelete.Query.soft_deletable?(query) do - from(x in query, where: is_nil(x.deleted_at)) |> subquery() + %{sub | query: from(x in query, where: is_nil(x.deleted_at))} else sub end