Request for code review for first live view attempt

Phoenix Forum Questions / Help
,
1

Hi, this is the first thing I’ve written with live view (and I don’t have much experience with Elixir/Phoenix in general though I’ve been programming for >10 years at this point so I have some other experience.)

I was hoping for a general code review if anyone has any comments plus some direction on whether the thing I’m doing with changesets and their actions & errors in the middle is somehow correct (feels like it can’t be) or if there is another approach I should be taking?

I’ve mostly been programming Elm for the last few years so all the Maybe, push_first & no_reply stuff is trying to resurrect some of that experience. Happy to be advised against it but it feels ok for me, right now.

defmodule OneBlockWeb.SignupLive do
  use Phoenix.LiveView

  import Phoenix.HTML.Form
  import OneBlockWeb.ErrorHelpers

  alias Phauxth.Log

  alias OneBlockWeb.{Auth.Token, Email}
  alias OneBlockWeb.Router.Helpers, as: Routes

  alias OneBlock.Accounts.SignupUser
  alias OneBlock.Building
  alias OneBlock.Flat

  def render(assigns) do
    ~L"""
    <h2>Sign Up</h2>
    <%= form_for @changeset, "", [phx_change: :change, phx_submit: :submit, class: "standard-form", as: "form"], fn f -> %>
      <%= if @changeset.action do %>
        <div class="alert alert-danger">
          Something went wrong. Please check the errors below.
        </div>
      <% end %>

      <div>
      <%= label f, :email %>
      <%= text_input f, :email %>
      <%= error_tag f, :email %>
      </div>

      <div>
      <%= label f, :display_name, "Your Name" %>
      <%= text_input f, :display_name %>
      <%= error_tag f, :display_name %>
      </div>

      <div>
        <%= label f, :password %>
        <%= password_input f, :password, value: input_value(f, :password) %>
        <%= error_tag f, :password %>
      </div>

      <div>
        <%= label f, :confirm_password %>
        <%= password_input f, :confirm_password, value: input_value(f, :confirm_password) %>
        <%= error_tag f, :confirm_password %>
      </div>

      <div>
        <%= label f, :building_id %>
        <%= select f, :building_id, @building_options %>
        <%= error_tag f, :building_id %>
      </div>

      <%= if (@changeset.changes[:building_id] != nil) && @flat_options != nil do %>
        <div>
          <%= label f, :flat_id %>
          <%= select f, :flat_id, @flat_options %>
          <%= error_tag f, :flat_id %>
        </div>

        <div>
          <%= label f, :status, "Ownership/Tenacy" %>
          <%= select f, :status, @status_options %>
          <%= error_tag f, :status %>
        </div>
      <% end %>

      <div>
        <%= submit "Save" %>
      </div>
    <% end %>
    """
  end

  def mount(_params, _session, socket) do
    changeset = SignupUser.changeset(%SignupUser{}, %{})

    building_options =
      Building.all()
      |> Enum.map(&{&1.name, &1.id})
      |> OneBlock.List.push_first({"Select your building", ""})

    status_options =
      SignupUser.status_options()
      |> OneBlock.List.push_first({"Select your status", ""})

    socket =
      socket
      |> assign(:changeset, changeset)
      |> assign(:building_options, building_options)
      |> assign(:flat_options, nil)
      |> assign(:status_options, status_options)

    {:ok, socket}
  end

  def handle_event("change", %{"_target" => target, "form" => form}, socket) do
    socket =
      case target do
        ["form", "building_id"] ->
          flat_options =
            form["building_id"]
            |> Maybe.from_string()
            |> Maybe.map(&Flat.get_by_building(&1))
            |> Maybe.map(fn list ->
              Enum.sort_by(list, &OneBlock.Sort.number_first(&1.name), :asc)
            end)
            |> Maybe.map(fn list -> Enum.map(list, &{&1.name, &1.id}) end)
            |> Maybe.map(&OneBlock.List.push_first(&1, {"Select your flat", ""}))
            |> Maybe.with_default(nil)

          assign(socket, :flat_options, flat_options)

        _ ->
          socket
      end

    fresh_changeset = SignupUser.changeset(%SignupUser{}, form)

    # Assign the action & errors from the previous changeset state to the new
    # changeset. We want the new changeset to have the latest values from the
    # form but the previous errors so that the errors stick around after
    # someone has clicked 'submit' and then starts typing in fields
    changeset = %{
      fresh_changeset
      | action: socket.assigns.changeset.action,
        errors: socket.assigns.changeset.errors
    }

    {:noreply, assign(socket, :changeset, changeset)}
  end

  def handle_event("submit", %{"form" => form}, socket) do
    changeset = SignupUser.changeset(%SignupUser{}, form)

    case SignupUser.create(changeset) do
      {:ok, user, account} ->
        Log.info(%Log{user: user.id, message: "user created"})

        key = Token.sign(%{"email" => account.email})
        Email.confirm_request(account.email, Routes.confirm_url(socket, :index, key: key))

        socket
        |> put_flash(:info, "Sign up successful! Please check for a confirmation email.")
        |> redirect(to: Routes.page_path(socket, :index))
        |> OneBlock.GenServer.no_reply()

      {:error, %Ecto.Changeset{} = changeset} ->
        socket
        |> assign(:changeset, changeset)
        |> OneBlock.GenServer.no_reply()
    end
  end
end

Any help would be much appreciated.

2 Likes