fix: check for operators first

improvement: don't return ids by default

lib/ash_phoenix/filter_form/filter_form.ex

@@ -233,11 +233,11 @@ defmodule AshPhoenix.FilterForm do
     ref = Ash.Query.expr(ref(^field, ^path))
 
     expr =
-      if Ash.Filter.get_function(operator, resource) do
+      if Ash.Filter.get_operator(operator) do
-        {:ok, %Ash.Query.Call{name: operator, args: [ref, value]}}
+        {:ok, %Ash.Query.Call{name: operator, args: [ref, value], operator?: true}}
       else
-        if Ash.Filter.get_operator(operator) do
+        if Ash.Filter.get_function(operator, resource) do
-          {:ok, %Ash.Query.Call{name: operator, args: [ref, value], operator?: true}}
+          {:ok, %Ash.Query.Call{name: operator, args: [ref, value]}}
         else
           {:error, {:operator, "No such function or operator #{operator}", []}}
         end
@@ -502,6 +502,11 @@ defmodule AshPhoenix.FilterForm do
       type: :string,
       doc:
         "The group id to add the predicate to. If not set, will be added to the top level group."
+    ],
+    return_id?: [
+      type: :boolean,
+      default: false,
+      doc: "If set to `true`, the function returns `{form, predicate_id}`"
     ]
   ]
 
@@ -528,13 +533,20 @@ defmodule AshPhoenix.FilterForm do
         form
       )
 
-    if opts[:to] && opts[:to] != form.id do
+    new_form =
-      {set_validity(%{
+      if opts[:to] && opts[:to] != form.id do
-         form
+        set_validity(%{
-         | components: Enum.map(form.components, &do_add_predicate(&1, opts[:to], predicate))
+          form
-       }), predicate_id}
+          | components: Enum.map(form.components, &do_add_predicate(&1, opts[:to], predicate))
+        })
+      else
+        set_validity(%{form | components: form.components ++ [predicate]})
+      end
+
+    if opts[:return_id?] do
+      {new_form, predicate_id}
     else
-      {set_validity(%{form | components: form.components ++ [predicate]}), predicate_id}
+      new_form
     end
   end
 
@@ -624,6 +636,11 @@ defmodule AshPhoenix.FilterForm do
       type: {:one_of, [:and, :or]},
       default: :and,
       doc: "The operator that the group should have internally."
+    ],
+    return_id?: [
+      type: :boolean,
+      default: false,
+      doc: "If set to `true`, the function returns `{form, predicate_id}`"
     ]
   ]
 
@@ -640,13 +657,20 @@ defmodule AshPhoenix.FilterForm do
     group_id = Ash.UUID.generate()
     group = %__MODULE__{operator: opts[:operator], id: group_id}
 
-    if opts[:to] && opts[:to] != form.id do
+    new_form =
-      {set_validity(%{
+      if opts[:to] && opts[:to] != form.id do
-         form
+        set_validity(%{
-         | components: Enum.map(form.components, &do_add_group(&1, opts[:to], group))
+          form
-       }), group_id}
+          | components: Enum.map(form.components, &do_add_group(&1, opts[:to], group))
+        })
+      else
+        set_validity(%{form | components: form.components ++ [group]})
+      end
+
+    if opts[:return_id?] do
+      {new_form, group_id}
     else
-      {set_validity(%{form | components: form.components ++ [group]}), group_id}
+      new_form
     end
   end
 

test/filter_form_test.exs

@@ -14,8 +14,8 @@ defmodule AshPhoenix.FilterFormTest do
     test "a group can be added" do
       form = FilterForm.new(Post)
 
-      {form, group_id} = FilterForm.add_group(form, operator: :or)
+      {form, group_id} = FilterForm.add_group(form, operator: :or, return_id?: true)
-      {form, _} = FilterForm.add_predicate(form, :title, :eq, "new post", to: group_id)
+      form = FilterForm.add_predicate(form, :title, :eq, "new post", to: group_id)
 
       assert %FilterForm{
                components: [
@@ -35,8 +35,8 @@ defmodule AshPhoenix.FilterFormTest do
     test "a group can be removed" do
       form = FilterForm.new(Post)
 
-      {form, group_id} = FilterForm.add_group(form, operator: :or)
+      {form, group_id} = FilterForm.add_group(form, operator: :or, return_id?: true)
-      {form, _} = FilterForm.add_predicate(form, :title, :eq, "new post", to: group_id)
+      form = FilterForm.add_predicate(form, :title, :eq, "new post", to: group_id)
 
       form = FilterForm.remove_group(form, group_id)
 
@@ -48,8 +48,10 @@ defmodule AshPhoenix.FilterFormTest do
     test "a predicate can be removed from a group" do
       form = FilterForm.new(Post)
 
-      {form, group_id} = FilterForm.add_group(form, operator: :or)
+      {form, group_id} = FilterForm.add_group(form, operator: :or, return_id?: true)
-      {form, predicate_id} = FilterForm.add_predicate(form, :title, :eq, "new post", to: group_id)
+
+      {form, predicate_id} =
+        FilterForm.add_predicate(form, :title, :eq, "new post", to: group_id, return_id?: true)
 
       form = FilterForm.remove_predicate(form, predicate_id)
 
@@ -65,8 +67,10 @@ defmodule AshPhoenix.FilterFormTest do
     test "with `remove_empty_groups?: true` empty groups are removed on component removal" do
       form = FilterForm.new(Post, remove_empty_groups?: true)
 
-      {form, group_id} = FilterForm.add_group(form, operator: :or)
+      {form, group_id} = FilterForm.add_group(form, operator: :or, return_id?: true)
-      {form, predicate_id} = FilterForm.add_predicate(form, :title, :eq, "new post", to: group_id)
+
+      {form, predicate_id} =
+        FilterForm.add_predicate(form, :title, :eq, "new post", to: group_id, return_id?: true)
 
       form = FilterForm.remove_predicate(form, predicate_id)
 
@@ -99,6 +103,36 @@ defmodule AshPhoenix.FilterFormTest do
              )
     end
 
+    test "the is_nil predicate correctly chooses the operator" do
+      form =
+        FilterForm.new(Post,
+          params: %{
+            field: :title,
+            operator: :is_nil,
+            value: "true"
+          }
+        )
+
+      assert Ash.Query.equivalent_to?(
+               FilterForm.filter!(Post, form),
+               is_nil(title)
+             )
+
+      form =
+        FilterForm.new(Post,
+          params: %{
+            field: :title,
+            operator: :is_nil,
+            value: "false"
+          }
+        )
+
+      assert Ash.Query.equivalent_to?(
+               IO.inspect(FilterForm.filter!(Post, form)),
+               not is_nil(title)
+             )
+    end
+
     test "predicates that map to functions work as well" do
       form =
         FilterForm.new(Post,