fix: massive tests, fixed query param and filter minor bugs (#19)

Author: zoedsoupe

.gitignore

@@ -34,3 +34,5 @@ result
 .elixir-tools/
 .lexical/
 /priv/plts/
+
+/.claude/

lib/supabase/postgrest/filter_builder.ex

@@ -230,7 +230,20 @@ defmodule Supabase.PostgREST.FilterBuilder do
   end
 
   def process_condition({op, column, value}) when is_filter_op(op) do
-    Enum.join([column, op, value], ".")
+    formatted_value =
+      case {op, value} do
+        {:in, values} when is_list(values) -> "(#{Enum.join(values, ",")})"
+        {:is, nil} -> "null"
+        {_, values} when is_list(values) -> "[#{Enum.join(values, ",")}]"
+        {_, v} -> v
+      end
+
+    Enum.join([column, op, formatted_value], ".")
+  end
+
+  # Handle between operator separately
+  def process_condition({:between, column, [from, to]}) do
+    "#{column}.between.[#{from},#{to}]"
   end
 
   @doc """

lib/supabase/postgrest/helpers.ex

@@ -0,0 +1,25 @@
+defmodule Supabase.PostgREST.Helpers do
+  @moduledoc """
+  Helper functions for PostgREST operations
+  """
+
+  @doc """
+  Get a header value from a headers list
+  """
+  def get_header(headers, key) when is_list(headers) do
+    case List.keyfind(headers, key, 0) do
+      {^key, value} -> value
+      nil -> nil
+    end
+  end
+
+  @doc """
+  Get a query parameter value from a query list
+  """
+  def get_query_param(query, key) when is_list(query) do
+    case List.keyfind(query, key, 0) do
+      {^key, value} -> value
+      nil -> nil
+    end
+  end
+end

lib/supabase/postgrest/query_builder.ex

@@ -76,20 +76,26 @@ defmodule Supabase.PostgREST.QueryBuilder do
   @impl true
   def insert(%Request{} = b, data, opts \\ []) when is_map(data) do
     on_conflict = Keyword.get(opts, :on_conflict)
-    on_conflict = if on_conflict, do: "on_conflict=#{on_conflict}"
+    on_conflict_header = if on_conflict, do: "on_conflict=#{on_conflict}"
     upsert = if on_conflict, do: "resolution=merge-duplicates"
     returning = Keyword.get(opts, :returning, :representation)
     count = Keyword.get(opts, :count, :exact)
-    prefer = ["return=#{returning}", "count=#{count}", on_conflict, upsert]
+    prefer = ["return=#{returning}", "count=#{count}", on_conflict_header, upsert]
     prefer = Enum.join(Enum.reject(prefer, &is_nil/1), ",")
 
     b
     |> Request.with_method(:post)
     |> Request.with_headers(%{"prefer" => prefer})
-    |> Request.with_query(%{"on_conflict" => on_conflict})
+    |> maybe_add_conflict_query(on_conflict)
     |> Request.with_body(data)
   end
 
+  defp maybe_add_conflict_query(request, nil), do: request
+
+  defp maybe_add_conflict_query(request, on_conflict) do
+    Request.with_query(request, %{"on_conflict" => on_conflict})
+  end
+
   @doc """
   Upserts data into a table, allowing for conflict resolution and specifying return options.
 
@@ -107,16 +113,23 @@ defmodule Supabase.PostgREST.QueryBuilder do
   @impl true
   def upsert(%Request{} = b, data, opts \\ []) when is_map(data) do
     on_conflict = Keyword.get(opts, :on_conflict)
+    on_conflict_header = if on_conflict, do: "on_conflict=#{on_conflict}"
     returning = Keyword.get(opts, :returning, :representation)
     count = Keyword.get(opts, :count, :exact)
 
-    prefer =
-      Enum.join(["resolution=merge-duplicates", "return=#{returning}", "count=#{count}"], ",")
+    prefer_parts = [
+      "resolution=merge-duplicates",
+      "return=#{returning}",
+      "count=#{count}",
+      on_conflict_header
+    ]
+
+    prefer = Enum.join(Enum.reject(prefer_parts, &is_nil/1), ",")
 
     b
     |> Request.with_method(:post)
     |> Request.with_headers(%{"prefer" => prefer})
-    |> Request.with_query(%{"on_conflict" => on_conflict})
+    |> maybe_add_conflict_query(on_conflict)
     |> Request.with_body(data)
   end
 

lib/supabase/postgrest/transform_builder.ex

@@ -6,6 +6,7 @@ defmodule Supabase.PostgREST.TransformBuilder do
   """
 
   alias Supabase.Fetcher.Request
+  alias Supabase.PostgREST.Helpers
 
   @behaviour Supabase.PostgREST.TransformBuilder.Behaviour
 
@@ -211,7 +212,8 @@ defmodule Supabase.PostgREST.TransformBuilder do
 
     # postgrest-ex sends always only one Accept header
     # and always sets a default (application/json)
-    for_mediatype = "for=#{b.headers["accept"]}"
+    accept_header = Helpers.get_header(b.headers, "accept") || "application/json"
+    for_mediatype = "for=#{accept_header}"
 
     plan = "application/vnd.pgrst.plan#{format};#{for_mediatype};#{opts}"
 

test/supabase/postgrest/filter_builder_test.exs

@@ -78,4 +78,162 @@ defmodule Supabase.PostgREST.FilterBuilderTest do
   test "process empty or condition" do
     assert process_condition({:or, []}) == "or()"
   end
+
+  describe "complex filter combinations" do
+    test "handles triple nested conditions" do
+      result =
+        process_condition(
+          {:and,
+           [
+             {:or, [{:eq, "type", "admin"}, {:eq, "type", "moderator"}]},
+             {:not,
+              {:and,
+               [
+                 {:lt, "created_at", "2023-01-01"},
+                 {:or, [{:eq, "status", "banned"}, {:eq, "status", "suspended"}]}
+               ]}}
+           ]}
+        )
+
+      assert result ==
+               "and(or(type.eq.admin,type.eq.moderator),not.and(created_at.lt.2023-01-01,or(status.eq.banned,status.eq.suspended)))"
+    end
+
+    test "handles multiple not operators in sequence" do
+      result = process_condition({:not, {:not, {:eq, "active", true}}})
+      assert result == "not.not.active.eq.true"
+    end
+
+    test "processes complex array conditions with modifiers" do
+      result =
+        process_condition(
+          {:and,
+           [
+             {:eq, "roles", ["admin", "editor"], any: true},
+             {:not, {:eq, "permissions", ["delete", "archive"], all: true}}
+           ]}
+        )
+
+      assert result ==
+               "and(roles=eq(any).{admin,editor},not.permissions=eq(all).{delete,archive})"
+    end
+
+    test "handles mixed operator types" do
+      result =
+        process_condition(
+          {:or,
+           [
+             {:gte, "score", 90},
+             {:and, [{:between, "score", [80, 89]}, {:eq, "bonus", true}]},
+             {:lte, "score", 50}
+           ]}
+        )
+
+      assert result == "or(score.gte.90,and(score.between.[80,89],bonus.eq.true),score.lte.50)"
+    end
+  end
+
+  describe "additional operators" do
+    test "process lt (less than) operator" do
+      assert process_condition({:lt, "price", 100}) == "price.lt.100"
+    end
+
+    test "process gte (greater than or equal) operator" do
+      assert process_condition({:gte, "quantity", 5}) == "quantity.gte.5"
+    end
+
+    test "process lte (less than or equal) operator" do
+      assert process_condition({:lte, "stock", 10}) == "stock.lte.10"
+    end
+
+    test "process neq (not equal) operator" do
+      assert process_condition({:neq, "status", "deleted"}) == "status.neq.deleted"
+    end
+
+    test "process like operator" do
+      assert process_condition({:like, "email", "%@example.com"}) == "email.like.%@example.com"
+    end
+
+    test "process ilike (case insensitive like) operator" do
+      assert process_condition({:ilike, "name", "%john%"}) == "name.ilike.%john%"
+    end
+
+    test "process in operator" do
+      assert process_condition({:in, "category", ["electronics", "books", "clothing"]}) ==
+               "category.in.(electronics,books,clothing)"
+    end
+
+    test "process is operator for null checks" do
+      assert process_condition({:is, "deleted_at", nil}) == "deleted_at.is.null"
+      assert process_condition({:is, "verified", true}) == "verified.is.true"
+      assert process_condition({:is, "archived", false}) == "archived.is.false"
+    end
+
+    test "process between operator" do
+      assert process_condition({:between, "age", [18, 65]}) == "age.between.[18,65]"
+    end
+  end
+
+  describe "edge cases and error handling" do
+    test "handles string values with special characters" do
+      assert process_condition({:eq, "name", "O'Brien"}) == "name.eq.O'Brien"
+
+      assert process_condition({:eq, "description", "Line 1\nLine 2"}) ==
+               "description.eq.Line 1\nLine 2"
+    end
+
+    test "handles numeric values" do
+      assert process_condition({:eq, "price", 19.99}) == "price.eq.19.99"
+      assert process_condition({:gt, "count", 1000}) == "count.gt.1000"
+    end
+
+    test "handles boolean values" do
+      assert process_condition({:eq, "active", true}) == "active.eq.true"
+      assert process_condition({:neq, "archived", false}) == "archived.neq.false"
+    end
+
+    test "single element and/or conditions" do
+      assert process_condition({:and, [{:eq, "status", "active"}]}) == "and(status.eq.active)"
+      assert process_condition({:or, [{:gt, "age", 18}]}) == "or(age.gt.18)"
+    end
+
+    test "deeply nested empty conditions" do
+      assert process_condition({:and, [{:or, []}, {:and, []}]}) == "and(or(),and())"
+    end
+
+    test "array values without modifiers default behavior" do
+      # This should raise or have specific behavior - adjust based on actual implementation
+      assert process_condition({:eq, "tags", ["tag1", "tag2"]}) == "tags.eq.[tag1,tag2]"
+    end
+  end
+
+  describe "filter builder integration" do
+    alias Supabase.Fetcher.Request
+    alias Supabase.PostgREST.FilterBuilder
+
+    setup do
+      client = Supabase.init_client!("http://example.com", "test-key")
+      request = Request.new(client)
+      {:ok, %{request: request}}
+    end
+
+    test "multiple filter functions create proper query params", %{request: request} do
+      result =
+        request
+        |> FilterBuilder.eq("status", "active")
+        |> FilterBuilder.gt("age", 18)
+        |> FilterBuilder.within("role", ["admin", "moderator"])
+
+      assert %Request{query: query} = result
+      assert {"status", "eq.active"} in query
+      assert {"age", "gt.18"} in query
+      assert {"role", "in.(admin,moderator)"} in query
+    end
+
+    test "filter functions handle nil values", %{request: request} do
+      result = FilterBuilder.is(request, "deleted_at", nil)
+      assert %Request{query: query} = result
+      assert {"deleted_at", "is.null"} in query
+    end
+  end
 end