germsvel / phoenix_test

MIT License
144 stars 20 forks source link

Allow passing arguments to `render_submit` in `submit_form` #54

Closed dabaer closed 4 months ago

dabaer commented 5 months ago

Hello again, I have a component like LiveSelect that uses JavaScript to tickle a hidden input, which provides the Magic™️.

I see there was a discussion about the issue in #37 but this seems like a nice thing to have in PhoenixTest itself.

Perhaps we could add an optional fourth argument to submit_form? My workaround thus far has been:

test "creates new work order with valid params", %{session: session} do
  truck = insert(:truck)
  params = params_for(:work_order, truck: truck)

  session =
    session
    |> visit(~p"/service_work_order")
    |> click_link("span.visible a", "New")

  {:ok, view, _} =
    session.view
    |> form("#work-order-form")
    |> render_submit(work_order: params)
    |> follow_redirect(session.conn)

  %PhoenixTest.Live{view: view, conn: session.conn}
  |> assert_has("span", "Truck #{truck.unit_no}")
  |> click_link("a[data-title='Audit']", "Audit")
  |> assert_has("td", "create")
  |> assert_has("td", params.complaint)
end

but breaking in and out of PhoenixTest like this feels naughty.

Here is a POC: https://github.com/dabaer/phoenix_test/commit/f745cfe7a9060575efa2f92ba316910e0949534f

And the much cleaner test 🎊

test "creates new work order with valid params", %{session: session} do
  truck = insert(:truck)
  params = params_for(:work_order, truck: truck)

  session =
    session
    |> visit(~p"/service_work_order")
    |> click_link("span.visible a", "New")
    |> submit_form("#work-order-form", %{}, work_order: params)
    |> assert_has("span", "Truck #{truck.unit_no}")

  session
  |> click_link("a[data-title='Audit']", "Audit")
  |> assert_has("td", "create")
  |> assert_has("td", params.complaint)
end

What do you think? Also, I wasn't quite sure how to go about presenting this to the DeadView side of things (or if it's even needed there), so the option is black-holed for now.

germsvel commented 5 months ago

Hi @dabaer thanks for opening this issue.

I'm currently making changes to PhoenixTest so it automatically includes all hidden fields and other defaults (e.g. a default checked radio button).

Would that ☝️ solve your issue without you having to break out of PhoenixTest?

If it doesn't, I have been considering how to best offer an escape hatch so you can drop in to LiveView when you want.

My guess for now would be to do something like this:

session
|> visit("/")
|> # ... do other phoenix test things
|> in_live_view(fn view -> 
  view
  |> element("#some-hook")
  |> render_hook(%{params_from_js: "hello world"})
end)
|> assert_has(text: "hello world")

I like that since it is a full escape hatch (instead of patching submit_form to take in more params and then having to come up with another solution for things like testing hooks.

What do you think?

In the meantime, the escape hatch you're using is okay. It's technically private API, but I don't know how much it'll change (just be warned in case it breaks at some point in a weird way).

If you keep using that escape hatch, I might do it like this so that you keep more of whatever is in the Session struct:

test "creates new work order with valid params", %{session: session} do
  truck = insert(:truck)
  params = params_for(:work_order, truck: truck)

  session
  |> visit(~p"/service_work_order")
  |> click_link("span.visible a", "New")
  |> then(fn session -> 
  {:ok, view, _} =
    session.view
    |> form("#work-order-form")
    |> render_submit(work_order: params)
    |> follow_redirect(session.conn)

    %{ session | view: view}
  end)
  |> assert_has("span", "Truck #{truck.unit_no}")
  |> click_link("a[data-title='Audit']", "Audit")
  |> assert_has("td", "create")
  |> assert_has("td", params.complaint)
end
dabaer commented 4 months ago

A hands-off option for working with hidden fields would be a fantastic option, though I'm not familiar enough with how LiveView.Test handles hidden inputs under the hood to say immediately if it will work in this case so I will need to try it out.

For other cases an official escape hatch is also a great idea!

I don't actually use the escape hatch I outlined anymore, as I use the commit I linked as the dep in my project and specify the hidden fields as the third argument (though it's not as nice as the first idea you mentioned).

Using then to break out definitely looks a lot cleaner as well, I wish I had thought of that initially 🤦

I look forward to the changes you are implementing and will try them out!

As far as this issue is concerned, I believe either of the options you mentioned would negate an actual change to submit_form so I will close it down.