`Flop.Phoenix.table` breaks with multiple dates in a filter

[mjrusso]

I have filtering by date implemented in a similar manner to #155 (same field repeated, with :<= and :>= filters respectively).

I'm attempting to consolidate into a single field (to capture both the start and end of the range). It's relatively straightforward: I built a custom "daterange" input, which itself renders two normal date inputs.

Relevant part of the schema:

    adapter_opts: [
      custom_fields: [
        started_at_date: [
          filter: {CustomFlopFilters, :date_range_filter, [source: :started_at]},
          ecto_type: :date,
          operators: [:in]
        ]
      ]
    ]

And the filter field:

    started_at_date: [
      label: "Started at",
      multiple: true,
      op: :in,
      type: "daterange"
    ]

Using the :in operator in this way (and multiple: true) is definitely a hack. I don't love it, but it (almost) works.

Params come in from the form like this, as we would expect:

%{
  "filters" => %{
    "0" => %{
      "field" => "started_at_date",
      "op" => "in",
      "value" => ["2023-01-04", "2023-12-04"]
    }
  }
}

(The custom filter interprets the first value as "from" and the second value as "to" in the range.)

And serializing to and from the URL works properly.

The only trouble is in Flop.Phoenix.table, which crashes in build_path/3 (similar-ish to #282):

[error] GenServer #PID<0.1082.0> terminating
** (ArgumentError) cannot encode maps inside lists when the map has 0 or more than 1 element, got: ~D[2023-01-04]
    (plug 1.15.2) lib/plug/conn/query.ex:370: anonymous fn/3 in Plug.Conn.Query.encode_pair/3
    (elixir 1.15.4) lib/enum.ex:4317: Enum.flat_map_list/2
    (plug 1.15.2) lib/plug/conn/query.ex:379: Plug.Conn.Query.encode_pair/3
    (plug 1.15.2) lib/plug/conn/query.ex:406: anonymous fn/3 in Plug.Conn.Query.encode_kv/3
    (elixir 1.15.4) lib/enum.ex:1260: anonymous fn/3 in Enum.flat_map/2
    (stdlib 5.0.2) maps.erl:416: :maps.fold_1/4
    (elixir 1.15.4) lib/enum.ex:2522: Enum.flat_map/2
    (plug 1.15.2) lib/plug/conn/query.ex:410: Plug.Conn.Query.encode_kv/3
    (plug 1.15.2) lib/plug/conn/query.ex:406: anonymous fn/3 in Plug.Conn.Query.encode_kv/3
    (elixir 1.15.4) lib/enum.ex:1260: anonymous fn/3 in Enum.flat_map/2
    (stdlib 5.0.2) maps.erl:416: :maps.fold_1/4
    (elixir 1.15.4) lib/enum.ex:2522: Enum.flat_map/2
    (plug 1.15.2) lib/plug/conn/query.ex:410: Plug.Conn.Query.encode_kv/3
    (plug 1.15.2) lib/plug/conn/query.ex:406: anonymous fn/3 in Plug.Conn.Query.encode_kv/3
    (elixir 1.15.4) lib/enum.ex:1260: anonymous fn/3 in Enum.flat_map/2
    (stdlib 5.0.2) maps.erl:416: :maps.fold_1/4
    (elixir 1.15.4) lib/enum.ex:2522: Enum.flat_map/2
    (plug 1.15.2) lib/plug/conn/query.ex:410: Plug.Conn.Query.encode_kv/3
    (plug 1.15.2) lib/plug/conn/query.ex:348: Plug.Conn.Query.encode/2
    (flop_phoenix 0.22.4) lib/flop_phoenix.ex:1863: Flop.Phoenix.build_path/3

It looks like Flop Phoenix is doing something like the following, which is crashing with the error above:

 Plug.Conn.Query.encode(%{
   "filters" => %{
     "0" => %{
       "field" => "started_at_date",
       "op" => "in",
       "value" => [~D[2023-01-12], ~D[2023-05-12]]
     }
   }
 })

([~D[2023-01-12]] doesn't work either. ~D[2023-01-12] does, which is what we'd expect otherwise casting anything as a date wouldn't work.)

All this being said, I'm not sure if Flop Phoenix should do anything about this, but I did want to file this issue as a form of documentation in case anyone runs into a similar issue.

[mjrusso]

Note that in this particular case, I (belatedly) realized that I can simply remove the ecto_type: :date cast in the custom field definition, and handle casting directly in the filter, which entirely sidesteps the problem documented above.

There's also another advantage of dropping the cast: because Flop will discard empty dates (i.e., empty strings) when the date cast is configured in the schema, it actually works out better to just manually handle casting. (I want the filter to work if the user only provides a value for one end of the date range, but if Flop filters out an empty string, then the custom filter will only see a single value -- but it has no way to know which end of the range the date corresponds to. There are a number of ways to work around this, but manually casting in the filter is probably the cleanest.)

[mjrusso]

Stepping back a bit, the only reason I started down this path of consolidating the date range filtering into a single field is because I couldn't get the UI I wanted with separate fields using Flop.Phoenix.filter_fields directly (I want it to be very clear that the user can provide a range). I did end up getting the multiple field approach to work using a hybrid of Flop.Phoenix.filter_fields (for the other fields) and Flop.Phoenix.hidden_inputs_for_filter + Phoenix.HTML.Form.inputs_for (for the date range), but the code was a mess and I was worried about the maintainability, which set me down this alternative path.

But now that I've typed all this up I realized that it's just so much conceptually simpler if I drop all the two-date-inputs-with-one-flop-field shenanigans and just use two separate inputs, like the original version:

      started_at_date: [
        label: "Start Date (From)",
        op: :>=,
        type: "date"
      ],
      started_at_date: [
        label: "Start Date (To)",
        op: :<=,
        type: "date"
      ]

(I'll take another stab at the form component; there's probably a better way to approach it.)

[woylie]

But now that I've typed all this up I realized that it's just so much conceptually simpler if I drop all the two-date-inputs-with-one-flop-field shenanigans and just use two separate inputs, like the original version:

      started_at_date: [
        label: "Start Date (From)",
        op: :>=,
        type: "date"
      ],
      started_at_date: [
        label: "Start Date (To)",
        op: :<=,
        type: "date"
      ]

I always modeled situations like this with two separate filter fields, as you did in the last example. If you want to have a single filter with multiple fields instead, I think it's better to build the inputs so that you get map parameters:

<input
   type="date"
   id={@id <> "_lower"}
   name={@name <> "[lower]"}
   value={date_range_value(:lower, @value)}
/>
<input
  type="date"
  id={@id <> "_upper"}
  name={@name <> "[upper]"}
  value={date_range_value(:upper, @value)}
/>

The date_range_value/2 function needs to get the date from the value and convert it into a string in that case. I've done something similar to deal with Postgres range columns. Since it's a custom field, you can ignore the operator and just use :== (which is the default). The ecto_type would be :map in this case.

[mjrusso]

Thanks @woylie! That :map trick is really neat (and I'll probably find a way to use it at some point 😄). In this case I'm definitely going to use two separate filter fields.

Playing around with this a bit more, it's clear that my issue isn't the filters or the inputs; what I really need is to be able to easily access a group of inputs/ form fields when rendering, ideally with ergonomics similar to Flop.Phoenix.filter_fields/1.

I'm sure there's a smarter way to do this, but here's the gist of what I have so far:

<.form for={@form} id={@id} phx-target={@target} phx-change={@on_change} phx-submit={@on_change}>
  <div class={@wrapper_class}>
    <Flop.Phoenix.hidden_inputs_for_filter form={@form} />
    <div :for={
      {ff, opts} <- [
        Keyword.get(@form_fields, :field_a),
        Keyword.get(@form_fields, :field_b),
        Keyword.get(@form_fields, :field_c)
      ]
    }>
      <.filter_form_input ff={ff} opts={opts} />
    </div>

    <div>
      <!-- Simplified date range component -->
      <.label>Started at</.label>

      <%= with [{ff_from, opts_from}, {ff_to, opts_to}] <- Keyword.get_values(@form_fields, :started_at_date) do %>
        <div class="flex items-center">
          <span class="mr-4">between</span>
          <.filter_form_input ff={ff_from} opts={opts_from} />

          <span class="mx-4">and</span>
          <.filter_form_input ff={ff_to} opts={opts_to} />
        </div>
      <% end %>
    </div>
  </div>
</.form>

Above, @form_fields is the result of calling this function:

  def filter_form_fields(%Phoenix.HTML.Form{} = form, fields) do
    field_names = Keyword.keys(fields)
    field_opts = Keyword.values(fields)

    inputs_for_filters =
      form.source
      |> form.impl.to_form(form, :filters, fields: fields)
      |> Enum.zip(field_opts)

    Enum.zip(field_names, inputs_for_filters)
  end

(Implementation mostly copied from Flop.Phoenix.filter_fields/1.)

And for completeness, here's filter_form_input/1:

  attr :ff, Phoenix.HTML.Form, required: true
  attr :opts, :list, required: true

  def filter_form_input(assigns) do
    ~H"""
    <Flop.Phoenix.hidden_inputs_for_filter form={@ff} />
    <.input
      field={@ff[:value]}
      label={input_label(@ff, @opts[:label])}
      type={@opts[:type]}
      phx-debounce={120}
      {Keyword.drop(@opts, [:label, :op, :type])}
    />
    """
  end

  defp input_label(_form, text) when is_binary(text), do: text
  defp input_label(form, nil), do: form |> input_value(:field) |> humanize()

I'm probably missing something, but in the off chance I'm not, there might be an opportunity for Flop Phoenix to make this use case (accessing multiple form fields/inputs at render time) slightly easier.