uploads with max_entries: 1 does not seem to auto upload every twice

[ykato123]

Environment

• Elixir version (elixir -v): 1.13.4
• Phoenix version (mix deps): 1.6.12
• Phoenix LiveView version (mix deps): 0.18.1
• Operating system: Windows
• Browsers you attempted to reproduce this bug on (the more the merrier): Chrome
• Does the problem persist after removing "assets/node_modules" and trying again? Yes/no: Yes

Actual behavior

I tried allow_upload/3 with max_entries: 1, auto_upload: true but auto upload seems to be not working every two times(2,4,6,...).

With my heex code below, progress bar will move and complete soon in the first try.
When I upload the second file, which should replace the former file, the progress bar stays 0.
If I try uploading again, progress bar will move soon, but if I try another time, it doesn't move again.
The progress bar moves when I submit the form, so I'm guessing it is something about auto uploading.

<form id="upload-form" phx-submit="save" phx-change="validate">
  <%= for entry <- @uploads.file.entries do %>
    <.live_img_preview entry={entry} />
    <progress value={entry.progress} max="100">
      <%= entry.progress %>%
    </progress>
  <% end %>
  <.live_file_input upload={@uploads.file} />
  <button type="submit">Upload</button>
</form>

Expected behavior

The progress bar should move and soon complete as the file is uploaded for the second time.

[MullPointer]

I also reproduced this issue:

• Elixir version (elixir -v): 1.14.3
• Phoenix version (mix deps): 1.7.2
• Phoenix LiveView version (mix deps): main dc4c139
• Operating system: Linux Mint 21.1 Vera
• Browsers you attempted to reproduce this bug on (the more the merrier): 112.0.2

Starting from what @Gazler found, I fell into a debugging rabbit hole. While #2443 does appear to fix the issue for me also, I do not think it fixes to the root of the problem (more details in a comment on that pull request).

Phoenix.LiveView.UploadConfig.put_entries/2 updates the upload config when new uploads arrive. When allow_upload has max_entries: 1 and the validation message is sent for a second upload, the existing upload is cancelled by calling Phoenix.LiveView.UploadConfig.maybe_replace_sole_entry/2, which calls cancel_entry in those circumstances. However, because the upload has started and has an upload channel associated, the update is not immediately removed from the UploadConfig, instead it is just marked with cancelled? field set to true. The actual removal of upload entries is triggered when the upload's Phoenix.LiveView.Channel stops, which triggers drop_entry for the upload in UploadConfig (see Phoenix.LiveView.Channel). This removal does not happen until after the processing of the validation message for the second upload is completed, so the response to that validation is as if the first upload is not cancelled, and an update to the page is sent with a too_many_files error, which prevents the upload from continuing to the next stage (as the error means the data-phx-auto-upload attribute is removed). However, the user never actually sees this error, as the dropping of the upload by the channel closing triggers another update to the page with the first upload removed and just the second upload active. But the upload has now missed the opportunity to be automatically started. On the third upload, the second upload gets immediately cancelled on validation because it never started and does not have an associated channel, so all works as intended.

The cleanest fix for this issue seems to me to change the behaviour of cancel_upload to fully remove the upload when it is called. I cannot see the need for the two stage remove process in live view's own code. The only advantage I can see is that it allows users to do some processing on the upload being replaced in their validate event handler, but this is not something I would expect and does not seem all that useful. I am submitting a pull request that makes this change, but I am not sure if it is the right approach. I think I may be missing some reason why this two stage approach is beneficial and was taken originally.

Alternatively, the behaviour of Phoenix.LiveView.UploadConfig.too_many_files?/1and maybe also Phoenix.Component.live_file_input/1 could be changed to ignore any uploads marked as cancelled and it be documented that the UploadConfig seen in the validate handler will include the automatically cancelled previous upload.

[chrismccord]

This should be fixed on main. Thanks!