germsvel / phoenix_test

PhoenixTest provides a unified way of writing feature tests -- regardless of whether you're testing LiveView pages or static (non-LiveView) pages.
https://hex.pm/packages/phoenix_test
MIT License
177 stars 23 forks source link

Check/uncheck not working with field name ending in `?` #97

Closed randycoulman closed 3 months ago

randycoulman commented 3 months ago

I'm building a form for bulk-updating the active? field on an entity (Fund in the output below).

The relevant part of the render function is:

      <.simple_form
        for={@form}
        id="activation-form"
        phx-target={@myself}
        phx-change="validate"
        phx-submit="save"
      >
        <.inputs_for :let={fund} field={@form[:funds]}>
          <.input field={fund[:active?]} label={fund[:name].value} type="checkbox" />
        </.inputs_for>
      </.simple_form>

My core_components.ex is pretty much identical to that generated by mix phx.new; I have definitely not modified the checkbox component.

When I try to do |> uncheck("Some fund name") or |> check("Some other fund name"), I get the following error:

     ** (ArgumentError) Could not find "form" for an element with selector "#activation_funds_2_active?".

     Found other potential "form":

     <form method="post" id="activation-form" phx-change="validate" phx-submit="save" phx-target="2">
       <div class="mt-10 space-y-8 bg-white">
         <input type="hidden" name="activation[funds][0][_persistent_id]" value="0"/>
         <input type="hidden" name="activation[funds][0][id]" value="141706"/>
         <div>
           <label class="flex items-center gap-4 text-sm leading-6 text-zinc-600 ">
             <input type="hidden" name="activation[funds][0][active?]" value="false"/>
             <input type="checkbox" id="activation_funds_0_active?" name="activation[funds][0][active?]" value="true" checked="checked" class="rounded border-zinc-300 text-zinc-900 focus:ring-0"/>
             Can Deactivate
           </label>
         </div>
         <input type="hidden" name="activation[funds][1][_persistent_id]" value="1"/>
         <input type="hidden" name="activation[funds][1][id]" value="141707"/>
         <div>
           <label class="flex items-center gap-4 text-sm leading-6 text-zinc-600 ">
             <input type="hidden" name="activation[funds][1][active?]" value="false"/>
             <input type="checkbox" id="activation_funds_1_active?" name="activation[funds][1][active?]" value="true" class="rounded border-zinc-300 text-zinc-900 focus:ring-0"/>
             Inactive
           </label>
         </div>
         <input type="hidden" name="activation[funds][2][_persistent_id]" value="2"/>
         <input type="hidden" name="activation[funds][2][id]" value="141709"/>
         <div>
           <label class="flex items-center gap-4 text-sm leading-6 text-zinc-600 ">
             <input type="hidden" name="activation[funds][2][active?]" value="false"/>
             <input type="checkbox" id="activation_funds_2_active?" name="activation[funds][2][active?]" value="true" checked="checked" class="rounded border-zinc-300 text-zinc-900 focus:ring-0"/>
             To Deactivate
           </label>
         </div>
         <div class="mt-2 flex items-center justify-between gap-6">
           <button type="submit" class="phx-submit-loading:opacity-75 rounded-lg bg-zinc-900 hover:bg-zinc-700 py-2 px-3 text-sm font-semibold leading-6 text-white active:text-white/80 " phx-disable-with="Updating...">
             <span class="hero-check-circle-mini">
             </span>
             Update Funds
           </button>
         </div>
       </div>
     </form>

     code: |> uncheck("To Deactivate")
     stacktrace:
       (phoenix_test 0.3.1) lib/phoenix_test/query.ex:380: PhoenixTest.Query.find_ancestor!/3
       (phoenix_test 0.3.1) lib/phoenix_test/form.ex:20: PhoenixTest.Form.find_by_descendant!/2
       (phoenix_test 0.3.1) lib/phoenix_test/live.ex:142: PhoenixTest.Live.fill_in_field_data/2
       test/freedom_account_web/components/activation_test.exs:21: (test)

I tried debugging through what PhoenixTest was doing, at it looks like it's returning zero matches in Query.find/2. But there's obviously an input with the right ID that is nested inside the form, so I'm not sure why it isn't finding it like it should.

If I rename the field from active? to active, everything works as I'd expect.

I can share more example code if needed, but wanted to get this bug report in.

germsvel commented 3 months ago

If I rename the field from active? to active, everything works as I'd expect.

Wow, that's odd behavior! Thanks for opening this issue @randycoulman.

I gave it a quick look, and it seems like this is a Floki issue.

Giving this a try:

    test "floki doesn't accept ? in id" do
      html = """
      <div id="hello?">
        Hello world
      </div>
      """

      text =
        html
        |> Floki.parse_fragment!()
        |> Floki.find("#hello?")
        |> Floki.text()

      assert text =~ "Hello world"
    end

That test fails because Floki.find/2 returns [].

If we change that to drop the ?, Floki finds the correct tag.

Would you be interested in opening an issue on Floki's repo?

randycoulman commented 3 months ago

Would you be interested in opening an issue on Floki's repo?

Done

germsvel commented 3 months ago

@randycoulman I went ahead and implemented the finding by [id='<id>']. So I think this should now work in main.

randycoulman commented 3 months ago

@germsvel I just now had a chance to test your fix, and was still getting the same error.

It looks like the same fix needs to be applied to Form.descendant_selector/1 as well.

Doing a quick search of the code suggests a couple of other places that might also need the same fix. I haven't seen evidence of them failing, and I didn't dig in deeply, so they may be fine.

randycoulman commented 3 months ago

Looks like GitHub doesn't give me permission to reopen this issue, so let me know if you'd like me to open a new one instead.

germsvel commented 3 months ago

Thanks @randycoulman!

germsvel commented 3 months ago

Just updated those spots to pass the id like that. Honestly, it's not ideal. Error messages are less nice (in my opinion). I wonder if it would be better to not support ? in IDs, but since it seems to be valid HTML, I'm including it.

alecostard commented 2 months ago

I think there are a few spots remaining, @germsvel

https://github.com/germsvel/phoenix_test/blob/86344236df2d5e63496a0b7d02b001beb90d8412/lib/phoenix_test/form.ex#L66 https://github.com/germsvel/phoenix_test/blob/86344236df2d5e63496a0b7d02b001beb90d8412/lib/phoenix_test/button.ex#L83

The first one is currently biting me. The unit test below should trigger the error.

test "issue with question mark form field" do
  alias PhoenixTest.Field
  alias PhoenixTest.Form

  html =
    """
    <form id="post-form">
      <label>
        <input type="hidden" name="post[done?]" value="false">
        <input type="checkbox" id="post_done?" name="post[done?]" value="true">
        Done?
      </label>
    </form>
    """

  field = Field.find_input!(html, "Done?")
  Form.find_by_descendant!(html, field)
end

Changing the selector to use the [id=...] approach seems to fix it. If you prefer I open a new issue, just tell me. Thanks for the library, BTW.

germsvel commented 2 months ago

@alecostard this got completely lost in my notifications. I didn't have any visibility, but just ran across it accidentally in my email.

I just fixed that in https://github.com/germsvel/phoenix_test/commit/a0a99a37c2a11c373aaa16cd0d22b47e7e121908