Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

live_file_input component and multiple files not validating for submitting #3319

Open
ghenry opened this issue Jun 21, 2024 · 12 comments
Open

Comments

@ghenry
Copy link
Contributor

ghenry commented Jun 21, 2024

Environment

  • Elixir version (elixir -v): Erlang/OTP 26 [erts-14.2.5] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [jit:ns]
    Elixir 1.16.2 (compiled with Erlang/OTP 26)

  • Phoenix version (mix deps): phoenix (Hex package) (mix)
    locked at 1.7.14 (phoenix) c7859bc5

  • Phoenix LiveView version (mix deps): phoenix_live_view (Hex package) (mix)
    locked at 0.20.15 (phoenix_live_view) 45c48ad1

  • Operating system: Fedora 40 x86_64 Workstation

  • Browsers you attempted to reproduce this bug on (the more the merrier): Firefox and Chrome latest

  • Does the problem persist after removing "assets/node_modules" and trying again? Yes/no: Yes, same

Actual behavior

Multiple files are accepted, but input field shows "Please select file" when Submit is clicked. When switching back to max_entries: 1, all is OK.

20240620_161612

<div
              id="dropzone"
              class="mt-2 flex justify-center cursor-pointer rounded-lg border border-dashed border-gray-900/25 px-6 py-10"
              phx-drop-target={@uploads.loa.ref}
              phx-click={JS.dispatch("click", to: "##{@uploads.loa.ref}", bubbles: false)}
              phx-hook="ResiSwitchPdfPreview"
            >
              <div class="text-center">
                <svg
                  viewBox="0 0 512 512"
                  class="mx-auto h-12 w-12 text-gray-300"
                  viewBox="0 0 24 24"
                  fill="currentColor"
                  aria-hidden="true"
                >
                  >
                  <path
                    fill-rule="evenodd"
                    d="M0 64C0 28.7 28.7 0 64 0H224V128c0 17.7 14.3 32 32 32H384V304H176c-35.3 0-64 28.7-64 64V512H64c-35.3 0-64-28.7-64-64V64zm384 64H256V0L384 128zM176 352h32c30.9 0 56 25.1 56 56s-25.1 56-56 56H192v32c0 8.8-7.2 16-16 16s-16-7.2-16-16V448 368c0-8.8 7.2-16 16-16zm32 80c13.3 0 24-10.7 24-24s-10.7-24-24-24H192v48h16zm96-80h32c26.5 0 48 21.5 48 48v64c0 26.5-21.5 48-48 48H304c-8.8 0-16-7.2-16-16V368c0-8.8 7.2-16 16-16zm32 128c8.8 0 16-7.2 16-16V400c0-8.8-7.2-16-16-16H320v96h16zm80-112c0-8.8 7.2-16 16-16h48c8.8 0 16 7.2 16 16s-7.2 16-16 16H448v32h32c8.8 0 16 7.2 16 16s-7.2 16-16 16H448v48c0 8.8-7.2 16-16 16s-16-7.2-16-16V432 368z"
                    clip-rule="evenodd"
                  />
                </svg>
                <div class="mt-4 text-sm leading-6 text-gray-600">
                  <label
                    for={@uploads.loa.ref}
                    class="relative cursor-pointer rounded-md bg-white font-semibold text-indigo-600 focus-within:outline-none focus-within:ring-2 focus-within:ring-indigo-600 focus-within:ring-offset-2 hover:text-indigo-500"
                  >
                    <span>Upload a PDF</span>
                  </label>
                  <.live_file_input upload={@uploads.loa} required class="sr-only" />
                  <p class="pl-1">or drag and drop up to 10MB</p>
                </div>
              </div>
            </div>

Screenshot from 2024-06-20 16-15-14

Expected behavior

Submit works and multiple files are uploaded.

Thanks for taking the time to read this and for the wonderful Phoenix and Phoenix LiveView.

@SteffenDE
Copy link
Collaborator

Please try to update LV, 0.20.15 had a bug with patching form contents.

@ghenry
Copy link
Contributor Author

ghenry commented Jun 22, 2024

Hi @SteffenDE

I'm on 0.20.17 now, exactly the same. I can't see that I've missed any config or code changes to handle this. The only difference I can see that is needed is going from max_entries: 1 to max_entries: 5 etc.

The multiple attribute is there on the input field as expected:

<input data-phx-id="m35-phx-F9sMyDH-COEVCCKB" id="phx-F9tJDY1mkBgfEbmI" type="file" name="loa" accept=".pdf" data-phx-hook="Phoenix.LiveFileUpload" data-phx-update="ignore" data-phx-upload-ref="phx-F9tJDY1mkBgfEbmI" data-phx-active-refs="" data-phx-done-refs="" data-phx-preflighted-refs="" required="" class="sr-only" multiple="">

Thanks.

@ghenry
Copy link
Contributor Author

ghenry commented Jun 26, 2024

Any ideas?

@SteffenDE
Copy link
Collaborator

@ghenry Sorry, I'm very busy at the moment and didn't have the time to look into it yet. It'd be great if you could create a single file script to reproduce the problem. This should be a good start: https://github.com/phoenixframework/phoenix_live_view/blob/main/.github/single-file-samples/main.exs

@ghenry
Copy link
Contributor Author

ghenry commented Jun 26, 2024

Will do. That will also allow me to prove if I've got some cruft blocking this simple thing from working.

@ghenry
Copy link
Contributor Author

ghenry commented Jul 10, 2024

Will get this example over in the next few days. Was away on holiday.

@kwbock
Copy link

kwbock commented Jul 15, 2024

I believe this example highlights the issue here. We were working on upgrading libraries/elixir/otp and discovered this in our regession cases.

This seems to only be an issue when auto_upload: true in the allow_upload/3 options.

In the example code below I have set max_entries: 2, auto_upload: true.

Repro Steps

  1. Click "Choose Files"
  2. Select 3 files to upload

Expected Behvaior

Error shown and files do not begin uploading

Actual Behavior

Error is shown and files begin uploading

Application.put_env(:sample, Example.Endpoint,
  http: [ip: {127, 0, 0, 1}, port: 5001],
  server: true,
  live_view: [signing_salt: "aaaaaaaa"],
  secret_key_base: String.duplicate("a", 64),
  pubsub_server: Example.PubSub
)

Mix.install([
  {:plug_cowboy, "~> 2.5"},
  {:jason, "~> 1.0"},
  {:phoenix, "~> 1.7"},
  # please test your issue using the latest version of LV from GitHub!
  {:phoenix_live_view, github: "phoenixframework/phoenix_live_view", branch: "main", override: true},
])

# build the LiveView JavaScript assets (this needs mix and npm available in your path!)
path = Phoenix.LiveView.__info__(:compile)[:source] |> Path.dirname() |> Path.join("../")
System.cmd("mix", ["deps.get"], cd: path, into: IO.binstream())
System.cmd("npm", ["install"], cd: Path.join(path, "./assets"), into: IO.binstream())
System.cmd("mix", ["assets.build"], cd: path, into: IO.binstream())

defmodule Example.ErrorView do
  def render(template, _), do: Phoenix.Controller.status_message_from_template(template)
end

defmodule Example.HomeLive do
  use Phoenix.LiveView, layout: {__MODULE__, :live}

  def mount(_params, _session, socket) do
    socket =
      socket
      |> assign(:uploaded_files, [])
      |> allow_upload(:avatar, auto_upload: true, accept: ~w(.jpg .jpeg), max_entries: 2, max_file_size: 1_000_000_000)
    {:ok, socket}
  end

  def render("live.html", assigns) do
    ~H"""
    <script src="/assets/phoenix/phoenix.js"></script>
    <script src="/assets/phoenix_live_view/phoenix_live_view.js"></script>
    <%!-- uncomment to use enable tailwind --%>
    <%!-- <script src="https://cdn.tailwindcss.com"></script> --%>
    <script>
      let liveSocket = new window.LiveView.LiveSocket("/live", window.Phoenix.Socket)
      liveSocket.connect()
    </script>
    <style>
      * { font-size: 1.1em; }
    </style>
    <%= @inner_content %>
    """
  end

  def render(assigns) do
    ~H"""
    <form id="upload-form" phx-submit="save" phx-change="validate">
      <.live_file_input upload={@uploads.avatar} />
      <button type="submit">Upload</button>
    </form>

    <section>
      <%!-- render each avatar entry --%>
      <%= for entry <- @uploads.avatar.entries do %>
        <article class="upload-entry">

          <figure>
            <.live_img_preview entry={entry} style="width: 250px;"/>
            <figcaption><%= entry.client_name %></figcaption>
          </figure>

          <%!-- entry.progress will update automatically for in-flight entries --%>
          <progress value={entry.progress} max="100"> <%= entry.progress %>% </progress>

          <%!-- a regular click event whose handler will invoke Phoenix.LiveView.cancel_upload/3 --%>
          <button type="button" phx-click="cancel-upload" phx-value-ref={entry.ref} aria-label="cancel">&times;</button>

          <%!-- Phoenix.Component.upload_errors/2 returns a list of error atoms --%>
          <%= for err <- upload_errors(@uploads.avatar, entry) do %>
            <p class="alert alert-danger"><%= error_to_string(err) %></p>
          <% end %>

        </article>
      <% end %>

      <%!-- Phoenix.Component.upload_errors/1 returns a list of error atoms --%>
      <%= for err <- upload_errors(@uploads.avatar) do %>
        <p class="alert alert-danger"><%= error_to_string(err) %></p>
      <% end %>
    </section>
    """
  end

  @impl Phoenix.LiveView
  def handle_event("validate", _params, socket) do
    {:noreply, socket}
  end

  @impl Phoenix.LiveView
  def handle_event("cancel-upload", %{"ref" => ref}, socket) do
    {:noreply, cancel_upload(socket, :avatar, ref)}
  end

  @impl Phoenix.LiveView
  def handle_event("save", _params, socket) do
    uploaded_files =
      consume_uploaded_entries(socket, :avatar, fn %{path: path}, _entry ->
        # dest = Path.join(["/tmp"])
        # # You will need to create `priv/static/uploads` for `File.cp!/2` to work.
        # File.cp!(path, dest)
        # {:ok, "/tmp/#{Path.basename(dest)}"}
      end)

    {:noreply, update(socket, :uploaded_files, &(&1 ++ uploaded_files))}
  end

  defp error_to_string(:too_large), do: "Too large"
  defp error_to_string(:not_accepted), do: "You have selected an unacceptable file type"
  defp error_to_string(:too_many_files), do: "You have selected too many files"
end

defmodule Example.Router do
  use Phoenix.Router
  import Phoenix.LiveView.Router

  pipeline :browser do
    plug(:accepts, ["html"])
  end

  scope "/", Example do
    pipe_through(:browser)

    live("/", HomeLive, :index)
  end
end

defmodule Example.Endpoint do
  use Phoenix.Endpoint, otp_app: :sample
  socket("/live", Phoenix.LiveView.Socket)

  plug Plug.Static, from: {:phoenix, "priv/static"}, at: "/assets/phoenix"
  plug Plug.Static, from: {:phoenix_live_view, "priv/static"}, at: "/assets/phoenix_live_view"

  plug(Example.Router)
end

{:ok, _} = Supervisor.start_link([{Phoenix.PubSub, name: Example.PubSub}, Example.Endpoint], strategy: :one_for_one)
Process.sleep(:infinity)

@kwbock
Copy link

kwbock commented Jul 16, 2024

@SteffenDE I did some testing with the above example and it seems this behavior showed up in v0.20.0.

I haven't had a chance to test any changes locally, but I'm leaning towards this change between the v0.19.5 and v0.20.0 branches. v0.19.5...v0.20.0#diff-8a399cc9a3e142ef2ff2b7437ba397cb3affa4ed0346b53eb087d4d303b0f781L2788

@SteffenDE
Copy link
Collaborator

We have this in the changelog for 0.20:

Fix uploads with auto_upload: true failing to auto upload valid entries errors when any individual entry is invalid

which might be related? I'm sadly not too familiar with the upload code, maybe @chrismccord can have a look.

@kwbock
Copy link

kwbock commented Jul 17, 2024

Fix uploads with auto_upload: true failing to auto upload valid entries errors when any individual entry is invalid

@SteffenDE I'm curious as to the intention here. Given 3 files, two jpegs and one png, to a live_file_input that only accepts jpegs, is the desired outcome A) no files begin uploading until the invalid file is removed or B) the two jpegs upload and the png does not.

If the answer is B, wouldn't that present issues if the number of files given exceeds the max_entries property of the allow_upload. Many people use max_entries to manage the amount processed at any given time.

@ghenry
Copy link
Contributor Author

ghenry commented Jul 24, 2024

I don't have auto upload enabled:

|> allow_upload(:loa, accept: ~w(.pdf), max_entries: 2, max_file_size: 10_000_000)}

@kwbock
Copy link

kwbock commented Jul 29, 2024

I can file a separate bug, but I believe both situations are rooted in the upload refactoring.

I'll defer to @SteffenDE and @ghenry on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants