From 6c63aaee7667d8a1b8f1d90e00f7a243c9a8b22d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Tue, 27 Jul 2021 19:14:02 +0200
Subject: [PATCH] Fix removing existing image and adding a new one

We introduced a bug in commit acbd1b023.

When editing a record and removing an existing image, we don't remove
the HTML fields associated with that image but simply hide them, and
then we add fields to create a new image when clicking on "Add image".

This is standard cocoon behavior. However, since in the case of images
there's a `has_one` relation, cocoon doesn't add unique identifiers to
the new fields, generating duplicate IDs, which is invalid HTML.

Since there's a duplicate file input ID, clicking on the "Choose image"
label we aren't clicking on the new input but on the old one. This means
we aren't correctly attaching an image. The tests passed because
Capybara uses the equivalent of a keyboard to select the field, and in
this case everything worked properly.

So we need to delete the existing elements before inserting new ones.
We're adding a test to check there aren't duplicate IDs.
---
 app/assets/javascripts/imageable.js    |  3 +++
 spec/shared/system/nested_imageable.rb | 11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/app/assets/javascripts/imageable.js b/app/assets/javascripts/imageable.js
index c2ee3e882..a6e658796 100644
--- a/app/assets/javascripts/imageable.js
+++ b/app/assets/javascripts/imageable.js
@@ -8,6 +8,9 @@
       $("#nested-image").on("cocoon:after-remove", function() {
         $("#new_image_link").removeClass("hide");
       });
+      $("#nested-image").on("cocoon:before-insert", function() {
+        $(".js-image-attachment").closest(".image").remove();
+      });
       $("#nested-image").on("cocoon:after-insert", function(e, nested_image) {
         var input;
         $("#new_image_link").addClass("hide");
diff --git a/spec/shared/system/nested_imageable.rb b/spec/shared/system/nested_imageable.rb
index 27eeeff3e..9de787867 100644
--- a/spec/shared/system/nested_imageable.rb
+++ b/spec/shared/system/nested_imageable.rb
@@ -227,6 +227,17 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p
         expect(page).not_to have_css ".image"
         expect(page).to have_css "a#new_image_link"
       end
+
+      scenario "don't duplicate fields after removing and adding an image" do
+        create(:image, imageable: imageable)
+        do_login_for user
+
+        visit send(path, arguments)
+        click_link "Remove image"
+        click_link "Add image"
+
+        expect(page).to have_css ".image", count: 1, visible: :all
+      end
     end
   end
 end