fix: edit stop broken after dependency update #1031
Conversation
| @@ -130,7 +129,7 @@ defmodule ArrowWeb.StopViewLive do | |||
| end | |||
|
|
|||
| def handle_event("validate", %{"stop" => stop_params}, socket) do | |||
| form = Stops.change_stop(%Stop{}, stop_params) |> to_form(action: :validate) | |||
| form = Stops.change_stop(socket.assigns.stop, stop_params) |> to_form(action: :validate) | |||
There was a problem hiding this comment.
The initial bug happened because we use unsafe_validate_unique to ensure we don't duplicate stop_ids and with us always comparing to an empty %Stop{}, it always thought we changed the stop_id so ran the validation. Building our changeset against the original stop prevents that.
I'm unsure why the original code was OK with it.
| @@ -103,14 +103,13 @@ defmodule ArrowWeb.StopViewLive do | |||
| def mount(_params, session, socket) do | |||
| logout_url = session["logout_url"] | |||
| form = to_form(Stops.change_stop(%Stop{})) | |||
| stops = Stops.list_stops() | |||
There was a problem hiding this comment.
stops was never used, so I removed it. It is unrelated to the issue at hand, so I can put it back if we'd rather be conservative.
There was a problem hiding this comment.
I think it's clear enough to me that it's unused, plus pulling in all of the stops probably adds a smidgen of slowness to the initial component mount, so I'm fine with removing it.
| @@ -54,7 +54,7 @@ defmodule ArrowWeb.Router do | |||
| live("/stops/new", StopViewLive, :new) | |||
| live("/stops/:id/edit", StopViewLive, :edit) | |||
| get("/stops", StopController, :index) | |||
| post("/stops/:id", StopController, :update) | |||
| put("/stops/:id", StopController, :update) | |||
There was a problem hiding this comment.
I'm more than a little baffled as to why this is a PUT now when submitting the form. It makes sense from a REST perspective, but from an engineering one I haven't worked out what's changing the method on the form when we are doing an edit vs a create (create is still post, I tested)
There was a problem hiding this comment.
I've tracked this down to the call to to_form generating a form structure with the method options: [method: "put"], when being called on a changeset based on an existing stop, and options: [method: "post"] when the changeset is based on an empty Stop, but best I can tell this is undocumented, and doesn't appear in the changelog.
There was a problem hiding this comment.
OK, I tracked this down. The behavior comes from phoenix_html, and it's been this way for a long time. The reason it was POST in the version of the code before is because while the form would initially be generated from the stop with and therefore have the method set to "put", the validate event when it fired was replacing the form with one based on a new stop (form = Stops.change_stop(%Stop{}, stop_params) |> to_form(action: :validate)), and so got replaced with a form with the method: "post". This new version continues to use the existing stop, so the form remains "put"
There was a problem hiding this comment.
Aha, that's another weird issue I ran into with the form that I had been wondering about!
Whoops
requested review from
a team and
lemald
and removed request for
a team
| @@ -122,7 +122,7 @@ defmodule ArrowWeb.StopLiveTest do | |||
| conn = follow_trigger_action(form, conn) | |||
| assert conn.method == "POST" | |||
There was a problem hiding this comment.
I wonder why this conn.method is still POST? 🤔
The test fails if you change it
Summary of changes
Asana Ticket: 🐛🏹 Fix Stop ID Validation in Stop Form
Reviewer Checklist