Merge pull request #5353 from Benjamin-Couey/3689-check-double-clicks-on-submit

janeewheatley · web-flow · commit b4460c73cfe7 · 2025-11-29T21:27:25.000-08:00
3689 check double clicks on submit
diff --git a/app/helpers/ui_helper.rb b/app/helpers/ui_helper.rb
@@ -80,20 +80,6 @@ def update_button_to(link, options = {})
     _link_to link, { icon: "check", type: "success", text: "Restore", size: "xs" }.merge(options), properties
   end
 
-  def dropdown_button(id, options = {})
-    options[:type] = (options[:type] || "primary").prepend("btn-dropdown btn-")
-    options[:id] = id
-    additional_properties = {
-      data: {
-        "bs-toggle": "dropdown"
-      },
-      "aria-haspopup": true,
-      "aria-expanded": true
-    }
-
-    _button_to({ submit_type: "button", text: "Set the 'text' property", size: "md", icon: "caret-down" }.merge(options), additional_properties)
-  end
-
   def cancel_button_to(link, options = {})
     _link_to link, { icon: "ban", type: "outline-primary", text: "Cancel", size: "md" }.merge(options)
   end
@@ -185,8 +171,16 @@ def _link_to(link, options = {}, properties = {})
 
     klass = "#{options[:class] || ""} btn btn-#{type} btn-#{size} #{center} #{disabled}"
 
-    link_to link, properties.merge(class: klass) do
-      fa_icon icon, text: text
+    form_klass = "#{options[:form_class] || ""} d-inline-block"
+
+    if properties[:method].blank? || properties[:method] == "get"
+      link_to link, properties.merge(class: klass) do
+        fa_icon icon, text: text
+      end
+    else
+      button_to link, properties.merge(class: klass, form_class: form_klass) do
+        fa_icon icon, text: text
+      end
     end
   end
 
diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb
@@ -8,7 +8,9 @@ def gravatar_url(email, size)
 
   def reinvite_user_link(user)
     if user.reinvitable?
-      link_to tag.i("", class: 'fa fa-envelope', alt: "Re-send invitation", title: "Re-send invitation"), resend_user_invitation_organization_path(user_id: user.id), method: :post
+      button_to resend_user_invitation_organization_path(user_id: user.id), class: "btn btn-outline-primary btn-xs", data: {disable_with: "Please wait..."}, alt: "Re-send invitation", title: "Re-send invitation" do
+        fa_icon "envelope"
+      end
     end
   end
 end
diff --git a/app/views/account_requests/index.html.erb b/app/views/account_requests/index.html.erb
diff --git a/app/views/admin/barcode_items/_barcode_item_row.html.erb b/app/views/admin/barcode_items/_barcode_item_row.html.erb
@@ -9,8 +9,6 @@
     <%= link_to edit_admin_barcode_item_path(barcode_item_row), class: "btn btn-info btn-xs" do %>
       <i class="fa fa-edit"></i> Edit
     <% end %>
-    <%= link_to admin_barcode_item_path(barcode_item_row), method: :delete, class: "btn btn-danger btn-xs", data: {confirm: confirm_delete_msg(barcode_item_row.item.name)} do %>
-      <i class="fa fa-trash"></i> Delete
-    <% end %>
+    <%= delete_button_to admin_barcode_item_path(barcode_item_row), { method: :delete, class: "btn btn-danger btn-xs", data: {confirm: confirm_delete_msg(barcode_item_row.item.name)} } %>
   </td>
 </tr>
diff --git a/app/views/layouts/_lte_admin_navbar.html.erb b/app/views/layouts/_lte_admin_navbar.html.erb
@@ -13,7 +13,7 @@
 
     <div class="dropdown-divider"></div>
 
-    <%= link_to destroy_user_session_path, class:"dropdown-item dropdown-footer", method: :delete do %>
+    <%= button_to destroy_user_session_path, class:"dropdown-item dropdown-footer", method: :delete, data: {disable_with: "Please wait..."} do %>
       <i class="fa fa-sign-out mr-2"></i> Log Out
     <% end %>
   </div>
diff --git a/app/views/layouts/_lte_navbar.html.erb b/app/views/layouts/_lte_navbar.html.erb
@@ -66,7 +66,7 @@
         <% end %>
     <% end %>
     <div class="dropdown-divider"></div>
-    <%= link_to destroy_user_session_path, class:"dropdown-item dropdown-footer", method: :delete do %>
+    <%= button_to destroy_user_session_path, class:"dropdown-item dropdown-footer", method: :delete, data: {disable_with: "Please wait..."} do %>
     <i class="fa fa-sign-out mr-2"></i> Log Out
     <% end %>
   </div>
diff --git a/app/views/layouts/partners/navigation/_navbar.html.erb b/app/views/layouts/partners/navigation/_navbar.html.erb
@@ -22,7 +22,7 @@
       <% end %>
     <% end %>
     <div class="dropdown-divider"></div>
-    <%= link_to destroy_user_session_path, method: :delete, class: "dropdown-item" do %>
+    <%= button_to destroy_user_session_path, method: :delete, class: "dropdown-item", data: {disable_with: "Please wait..."} do %>
       <i class="fa fa-sign-out fa-lg"></i> Logout
     <% end %>
   </div>
diff --git a/app/views/partners/_form.html.erb b/app/views/partners/_form.html.erb
@@ -58,7 +58,9 @@
                         <li class="attached-document d-flex justify-content-between align-items-center p-2 border rounded mb-2" data-document-id="<%= doc.signed_id %>">
                           <%= link_to doc.blob.filename.to_s, rails_blob_path(doc), class: "font-weight-bold w-75 text-truncate" %>
                           <%= f.hidden_field :documents, multiple: true, value: doc.signed_id %>
-                          <%= delete_button_to attachment_path(doc), { text: "Remove", size: "md", confirm: "Are you sure you want to permanently remove this document?" } %>
+                          <%= f.button :button, formmethod: :delete, formaction: attachment_path(doc), class: "btn btn-danger btn-md", data: {disable_with: "Please wait..."} do
+                            fa_icon "trash", text: "Remove"
+                          end %>
                         </li>
                       <% end %>
                     <% end %>
diff --git a/app/views/partners/profiles/_actions.html.erb b/app/views/partners/profiles/_actions.html.erb
@@ -15,7 +15,7 @@
                 <i class="fa fa-clock"></i> Pending Approval
               <% end %>
             <% else %>
-              <%= link_to partners_approval_request_path, method: :post, class: "btn btn-success" do %>
+              <%= button_to partners_approval_request_path, method: :post, data: {disable_with: "Please wait..."}, class: "btn btn-success", form_class: "d-inline-block" do %>
                 <i class="fa fa-paper-plane"></i> Submit for Approval
               <% end %>
             <% end %>
diff --git a/app/views/users/sessions/new.html.erb b/app/views/users/sessions/new.html.erb
@@ -53,6 +53,6 @@
   </div>
 <% end %>
 
-<%= link_to user_google_oauth2_omniauth_authorize_path, method: :post, class: "d-block" do %>
+<%= button_to user_google_oauth2_omniauth_authorize_path, method: :post, class: "btn btn-block", form_class: "d-inline-block", data: {disable_with: "Please wait..."} do %>
   <img src="../img/btn_google_signin_dark_focus_web@2x.png" alt="Sign in with Google" class="d-block w-100">
 <% end %>
diff --git a/app/views/users/shared/_links.html.erb b/app/views/users/shared/_links.html.erb
@@ -1,6 +1,6 @@
 <%- if !%w[sessions consolidated_logins].include?(controller_name) %>
   <%= link_to "Log in", new_session_path(resource_name) %><br>
-  <%= link_to user_google_oauth2_omniauth_authorize_path, method: :post do %>
+  <%= button_to user_google_oauth2_omniauth_authorize_path, method: :post, class: ".btn btn-block", data: {disable_with: "Please wait..."} do %>
     <img src="../img/btn_google_signin_dark_focus_web@2x.png" alt="Sign in with Google">
   <% end %>
 <% end -%>
diff --git a/spec/requests/admin/ndbn_members_spec.rb b/spec/requests/admin/ndbn_members_spec.rb
@@ -20,8 +20,10 @@
       html = Nokogiri::HTML(response.body)
 
       expect(html.css("h1").text).to eq("NDBN Member Upload")
-      expect(html.css("input[type=file]").count).to eq(1)
-      expect(html.css("button[type=submit]").count).to eq(1)
+
+      expect(html.css("form[action='/admin/ndbn_members/upload_csv']").count).to eq(1)
+      expect(html.css("form[action='/admin/ndbn_members/upload_csv'] input[type=file]").count).to eq(1)
+      expect(html.css("form[action='/admin/ndbn_members/upload_csv'] button[type=submit]").count).to eq(1)
 
       expect(html.css("th").map(&:text)).to match_array(["NDBN Member Number", "Member Name"])
       expect(html.css("tbody tr td").map(&:text)).to match_array(["123", "A Baby Center"])
diff --git a/spec/requests/distributions_requests_spec.rb b/spec/requests/distributions_requests_spec.rb
@@ -87,7 +87,7 @@
         get distributions_path
         page = Nokogiri::HTML(response.body)
         edit = page.at_css("a[href='#{edit_distribution_path(id: distribution.id)}']")
-        reclaim = page.at_css("a.btn-danger[href='#{distribution_path(id: distribution.id)}']")
+        reclaim = page.at_css("form[action='#{distribution_path(id: distribution.id)}'] .btn-danger")
         expect(edit.attr("class")).not_to match(/disabled/)
         expect(reclaim.attr("class")).not_to match(/disabled/)
         expect(response.body).not_to match(/Has Inactive Items/)
@@ -102,7 +102,7 @@
           get distributions_path
           page = Nokogiri::HTML(response.body)
           edit = page.at_css("a[href='#{edit_distribution_path(id: distribution.id)}']")
-          reclaim = page.at_css("a.btn-danger[href='#{distribution_path(id: distribution.id)}']")
+          reclaim = page.at_css("form[action='#{distribution_path(id: distribution.id)}'] .btn-danger")
           expect(edit.attr("class")).to match(/disabled/)
           expect(reclaim.attr("class")).to match(/disabled/)
           expect(response.body).to match(/Has Inactive Items/)
diff --git a/spec/requests/donation_sites_requests_spec.rb b/spec/requests/donation_sites_requests_spec.rb
@@ -24,7 +24,7 @@
         page = Nokogiri::HTML(response.body)
         expect(response.body).to include("An Active Site")
         expect(response.body).not_to include("An Inactive Site")
-        button1 = page.css(".btn[href='/donation_sites/#{active_donation_site.id}/deactivate']")
+        button1 = page.css("form[action='/donation_sites/#{active_donation_site.id}/deactivate'] .btn")
         expect(button1.text.strip).to eq("Deactivate")
         expect(button1.attr('class')).not_to match(/disabled/)
       end
@@ -39,12 +39,12 @@
           expect(response.body).to include("An Inactive Site")
 
           # Active donation site should have deactivate button
-          button1 = page.css(".btn[href='/donation_sites/#{active_donation_site.id}/deactivate']")
+          button1 = page.css("form[action='/donation_sites/#{active_donation_site.id}/deactivate'] .btn")
           expect(button1.text.strip).to eq("Deactivate")
           expect(button1.attr('class')).not_to match(/disabled/)
 
           # Inactive donation site should have reactivate button
-          button2 = page.css(".btn[href='/donation_sites/#{inactive_donation_site.id}/reactivate']")
+          button2 = page.css("form[action='/donation_sites/#{inactive_donation_site.id}/reactivate'] .btn")
           expect(button2.text.strip).to eq("Restore")
           expect(button2.attr('class')).not_to match(/disabled/)
         end
diff --git a/spec/requests/donations_requests_spec.rb b/spec/requests/donations_requests_spec.rb
@@ -269,7 +269,7 @@
           get donation_path(donation.id)
           page = Nokogiri::HTML(response.body)
           edit = page.at_css("a[href='#{edit_donation_path(donation.id)}']")
-          delete = page.at_css("a.btn-danger[href='#{donation_path(donation.id)}']")
+          delete = page.at_css("form[action='#{donation_path(donation.id)}'] .btn-danger")
           expect(edit.attr("class")).to match(/disabled/)
           expect(delete.attr("class")).to match(/disabled/)
           expect(response.body).to match(/please make the following items active: #{item.name}/)
diff --git a/spec/requests/items_requests_spec.rb b/spec/requests/items_requests_spec.rb
@@ -294,12 +294,12 @@
         expect(response.body).to include("NODEACTIVATE")
         expect(response.body).to include("NODELETE")
         expect(response.body).not_to include("NOSIR")
-        button1 = page.css(".btn.btn-danger[href='/items/#{item.id}']")
+        button1 = page.css("form[action='/items/#{item.id}'] .btn.btn-danger")
         expect(button1.text.strip).to eq("Delete")
-        button2 = page.css(".btn[href='/items/#{non_delete_item.id}/deactivate']")
+        button2 = page.css("form[action='/items/#{non_delete_item.id}/deactivate'] .btn")
         expect(button2.text.strip).to eq("Deactivate")
         expect(button2.attr('class')).not_to match(/disabled/)
-        button3 = page.css(".btn[href='/items/#{non_deactivate_item.id}/deactivate']")
+        button3 = page.css("form[action='/items/#{non_deactivate_item.id}/deactivate'] .btn")
         expect(button3.text.strip).to eq("Deactivate")
         expect(button3.attr('class')).to match(/disabled/)
       end
diff --git a/spec/requests/purchases_requests_spec.rb b/spec/requests/purchases_requests_spec.rb
@@ -429,7 +429,7 @@
           get purchase_path(purchase.id)
           page = Nokogiri::HTML(response.body)
           edit = page.at_css("a[href='#{edit_purchase_path(purchase.id)}']")
-          delete = page.at_css("a.btn-danger[href='#{purchase_path(purchase.id)}']")
+          delete = page.at_css("form[action='#{purchase_path(purchase.id)}'] .btn-danger")
           expect(edit.attr("class")).to match(/disabled/)
           expect(delete.attr("class")).to match(/disabled/)
           expect(response.body).to match(/please make the following items active: #{item.name}/)
diff --git a/spec/requests/storage_locations_requests_spec.rb b/spec/requests/storage_locations_requests_spec.rb
@@ -73,7 +73,7 @@
           it "shows a deactivate button" do
             get storage_locations_path(format: response_format)
             page = Nokogiri::HTML(response.body)
-            deactivate_link = page.at_css("a[href='#{storage_location_deactivate_path(storage_location)}']")
+            deactivate_link = page.at_css("form[action='#{storage_location_deactivate_path(storage_location)}'] button")
             expect(deactivate_link.attr("class")).not_to match(/disabled/)
           end
         end
@@ -87,7 +87,7 @@
           it "shows a disabled deactivate button" do
             get storage_locations_path(format: response_format)
             page = Nokogiri::HTML(response.body)
-            deactivate_link = page.at_css("a[href='#{storage_location_deactivate_path(storage_location)}']")
+            deactivate_link = page.at_css("form[action='#{storage_location_deactivate_path(storage_location)}'] button")
             expect(deactivate_link.attr("class")).to match(/disabled/)
           end
         end
diff --git a/spec/support/double_click_helper.rb b/spec/support/double_click_helper.rb
@@ -1,6 +1,11 @@
 # Simulates the user double clicking on an element specified by a css_selector
 # by using the API provided by Ferrum to manipulate the browser directly.
 #
+# Capybara's double click method and executing Javascript scripts were also tried,
+# but these were not able to reliably cause bugs observed when manually double
+# clicking an element. This method was the only one found to consistently produce
+# the buggy behavior and thus validate that the bug is fixed.
+#
 # @param css_selector [String] The CSS selector for the element to be double clicked.
 #
 # @example Usage
diff --git a/spec/system/audit_system_spec.rb b/spec/system/audit_system_spec.rb
@@ -87,7 +87,7 @@
           expect(page).to have_content(audit_quantity)
 
           accept_confirm do
-            click_link "Finalize Audit"
+            click_button "Finalize Audit"
           end
           expect(page.find(".alert-info")).to have_content "Audit is Finalized"
 
@@ -209,7 +209,7 @@
         expect(page).to have_content("Delete Audit")
         expect do
           accept_confirm do
-            click_link "Delete Audit"
+            click_button "Delete Audit"
           end
           expect(page).to have_content("Audit is successfully deleted.")
         end.to change { Audit.count }.by(-1)
@@ -257,7 +257,7 @@
         expect(page).to have_content("Delete Audit")
         expect do
           accept_confirm do
-            click_link "Delete Audit"
+            click_button "Delete Audit"
           end
           expect(page).to have_content("Audit is successfully deleted.")
         end.to change { Audit.count }.by(-1)
@@ -269,7 +269,7 @@
         expect(page).to have_content("Finalize Audit")
         expect do
           accept_confirm do
-            click_link "Finalize Audit"
+            click_button "Finalize Audit"
           end
           expect(page).to have_content("Audit is Finalized.")
         end.to change { Audit.finalized.count }.by(1)
@@ -284,7 +284,7 @@
           expect(page).to have_content("Finalize Audit")
           expect do
             accept_confirm do
-              click_link "Finalize Audit"
+              click_button "Finalize Audit"
             end
             expect(page).to have_content("Audit is Finalized.")
           end.to change { storage_location.size }.by(quantity - item_quantity)
@@ -294,7 +294,7 @@
           visit subject
           expect(page).to have_content("Finalize Audit")
           accept_confirm do
-            click_link "Finalize Audit"
+            click_button "Finalize Audit"
           end
           expect(page).not_to have_content("Resume Audit")
           expect(page).not_to have_content("Delete Audit")
@@ -308,7 +308,7 @@
           visit subject
           expect(page).to have_content("Finalize Audit")
           accept_confirm do
-            click_link "Finalize Audit"
+            click_button "Finalize Audit"
           end
           expect(page).not_to have_content("Delete Audit")
           # Actual Deletion(`delete :destroy`) Check is done in audits_controller_spec
@@ -331,7 +331,7 @@
             visit subject
             expect do
               accept_confirm do
-                click_link "Finalize Audit"
+                click_button "Finalize Audit"
               end
               expect(page).to have_content("Audit is Finalized.")
             end.to change { storage_location.size }.by(quantity - item_quantity)
diff --git a/spec/system/authorization_system_spec.rb b/spec/system/authorization_system_spec.rb
@@ -46,7 +46,7 @@
         visit new_distribution_path
         select "Test Partner", from: "Partner"
         select "Test Storage Location", from: "From storage location"
-        first('input[name="authenticity_token"]', visible: false).set("NOTAVALIDCSRFTOKEN")
+        first('form[action="/distributions"] input[name="authenticity_token"]', visible: false).set("NOTAVALIDCSRFTOKEN")
         page.execute_script("$(\"meta[name='csrf-token']\").attr('content', 'NOTAVALIDCSRFTOKEN');")
         click_button "Save"
         expect(current_path).to eql new_distribution_path
diff --git a/spec/system/barcode_item_system_spec.rb b/spec/system/barcode_item_system_spec.rb
@@ -69,6 +69,29 @@
 
       expect(page).to have_css("table tbody tr", count: 2)
     end
+
+    it "can delete a barcode item" do
+      item = create(:item, name: "Red 1T Diapers", base_item: base_item)
+      create(:barcode_item, organization: organization, barcodeable: item, value: "barcode_to_delete")
+
+      visit subject
+      expect(page).to have_content("barcode_to_delete")
+      click_button "Delete"
+      expect(page).to have_content("Barcode deleted!")
+      expect(page).not_to have_content("barcode_to_delete")
+    end
+
+    it "Double clicking the delete button does not result in the barcode attemping to be deleted twice" do
+      item = create(:item, name: "Red 1T Diapers", base_item: base_item)
+      b_item = create(:barcode_item, organization: organization, barcodeable: item, value: "barcode_to_delete")
+
+      visit subject
+      expect(page).to have_content(b_item.value)
+      ferrum_double_click('form[action*="/barcode_items/"] .btn.btn-danger.btn-xs')
+      expect(page).to have_content("Barcode deleted!")
+      expect(page).not_to have_content("barcode_to_delete")
+      expect(page).not_to have_content("Sorry, you don't have permission to delete this barcode.")
+    end
   end
 
   context "With organization-specific barcodes" do
diff --git a/spec/system/distribution_system_spec.rb b/spec/system/distribution_system_spec.rb
diff --git a/spec/system/donation_system_spec.rb b/spec/system/donation_system_spec.rb
diff --git a/spec/system/organization_system_spec.rb b/spec/system/organization_system_spec.rb
diff --git a/spec/system/partner_system_spec.rb b/spec/system/partner_system_spec.rb
diff --git a/spec/system/partners/approval_process_spec.rb b/spec/system/partners/approval_process_spec.rb
diff --git a/spec/system/purchase_system_spec.rb b/spec/system/purchase_system_spec.rb
diff --git a/spec/system/storage_location_system_spec.rb b/spec/system/storage_location_system_spec.rb
diff --git a/spec/system/transfer_system_spec.rb b/spec/system/transfer_system_spec.rb
diff --git a/spec/system/vendor_system_spec.rb b/spec/system/vendor_system_spec.rb
