Unsafe Actions: Replace link_to calls with button_to (#2516)

The `<a>` element is suitable for [Safe HTTP Methods][] (like `GET`) to
drive page navigations. Unsafe HTTP Methods (like `POST`, `PUT`, and
`DELETE`) are better initiated by `<form>` submissions.

This commit replaces generated calls to `link_to` with calls to
`button_to`.

The rest of the styling changes aim to preserve design decisions made
about preserving the appearance of elements that were once presented as
`<a>` elements that are now presented as `<input type="submit">`
elements nested within `<form>` elements.

While the design changes preserved backwards compatibility, it's worth
re-considering the choice to present them as "navigation" links instead
of "action" buttons.

[Safe HTTP Methods]: https://developer.mozilla.org/en-US/docs/Glossary/Safe/HTTP

app/assets/stylesheets/administrate/application.scss

@@ -26,5 +26,3 @@
 @import "components/navigation";
 @import "components/pagination";
 @import "components/search";
-
-@import "utilities/text-color";

app/assets/stylesheets/administrate/base/_layout.scss

@@ -13,6 +13,11 @@ figure {
   margin: 0;
 }
 
+/* stylelint-disable selector-no-qualifying-type, selector-class-pattern */
+form.button_to { // we don't control this class name
+  display: contents;
+}
+
 img,
 picture {
   margin: 0;

app/assets/stylesheets/administrate/base/_typography.scss

@@ -21,11 +21,22 @@ p {
   margin: 0 0 $small-spacing;
 }
 
-a {
+a,
+.link:is(
+  button,
+  [type="button"],
+  [type="reset"],
+  [type="submit"]
+) {
   color: $action-color;
   text-decoration-skip-ink: auto;
   transition: color $base-duration $base-timing;
 
+  /* stylelint-disable selector-no-qualifying-type */
+  &.link--danger {
+    color: $red;
+  }
+
   &:hover {
     color: mix($black, $action-color, 25%);
   }

app/assets/stylesheets/administrate/components/_buttons.scss

@@ -22,23 +22,23 @@ button,
   vertical-align: middle;
   white-space: nowrap;
 
-  &:hover {
+  &:disabled {
+    cursor: not-allowed;
+    opacity: 0.5;
+  }
+
+  &:not(.link):hover {
     background-color: mix($black, $action-color, 20%);
     color: $white;
   }
 
-  &:focus {
+  &:not(.link):focus {
     outline: $focus-outline;
     outline-offset: $focus-outline-offset;
   }
 
-  &:disabled {
-    cursor: not-allowed;
-    opacity: 0.5;
-
-    &:hover {
-      background-color: $action-color;
-    }
+  &:not(.link):disabled:hover {
+    background-color: $action-color;
   }
 }
 

app/assets/stylesheets/administrate/reset/_normalize.scss

@@ -100,7 +100,13 @@ pre {
  * 2. Remove gaps in links underline in iOS 8+ and Safari 8+.
  */
 
-a {
+a,
+.link:is(
+  button,
+  [type="button"],
+  [type="reset"],
+  [type="submit"]
+) {
   background-color: transparent; /* 1 */
   -webkit-text-decoration-skip: objects; /* 2 */
 }

app/views/administrate/application/_collection_item_actions.html.erb

@@ -7,10 +7,11 @@
 <% end %>
 
 <% if existing_action?(collection_presenter.resource_name, :destroy) %>
-  <td><%= link_to(
+  <td><%= button_to(
     t("administrate.actions.destroy"),
     [namespace, resource],
-    class: "text-color-red",
-    data: { turbo_method: :delete, turbo_confirm: t("administrate.actions.confirm") }
+    class: "link link--danger",
+    method: :delete,
+    data: { turbo_confirm: t("administrate.actions.confirm") }
   ) if accessible_action?(resource, :destroy) %></td>
 <% end %>

app/views/administrate/application/show.html.erb

@@ -30,11 +30,12 @@ as well as a link to its edit page.
       class: "button",
     ) if accessible_action?(page.resource, :edit) %>
 
-    <%= link_to(
+    <%= button_to(
       t("administrate.actions.destroy"),
       [namespace, page.resource],
       class: "button button--danger",
-      data: { turbo_method: :delete, turbo_confirm: t("administrate.actions.confirm") }
+      method: :delete,
+      data: { turbo_confirm: t("administrate.actions.confirm") }
     ) if accessible_action?(page.resource, :destroy) %>
   </div>
 </header>

spec/example_app/app/views/admin/customers/_collection_item_actions.html.erb

@@ -7,10 +7,10 @@
 <% end %>
 
 <% if existing_action?(collection_presenter.resource_name, :destroy) %>
-  <td><%= link_to(
+  <td><%= button_to(
     t("administrate.actions.destroy"),
     [namespace, resource],
-    class: "text-color-red",
+    class: "link link--danger",
     method: :delete,
     data: { turbo_confirm: t("administrate.actions.confirm") }
   ) if authorized_action?(resource, :destroy) %></td>

spec/features/index_page_spec.rb

@@ -81,9 +81,17 @@
     visit admin_customers_path
 
     within(".main-content") { click_on "Orders" }
-    expect(page).to have_content(/Cam.*1 order.*Ade.*2 orders.*Ben.*3 orders/)
+    within :table do
+      expect(page).to have_css("tbody tr:nth-child(1)", text: "Cam").and(have_text("1 order"))
+      expect(page).to have_css("tbody tr:nth-child(2)", text: "Ade").and(have_text("2 orders"))
+      expect(page).to have_css("tbody tr:nth-child(3)", text: "Ben").and(have_text("3 orders"))
+    end
 
     within(".main-content") { click_on "Orders" }
-    expect(page).to have_content(/Ben.*3 orders.*Ade.*2 orders.*Cam.*1 order/)
+    within :table do
+      expect(page).to have_css("tbody tr:nth-child(1)", text: "Ben").and(have_text("3 orders"))
+      expect(page).to have_css("tbody tr:nth-child(2)", text: "Ade").and(have_text("2 orders"))
+      expect(page).to have_css("tbody tr:nth-child(3)", text: "Cam").and(have_text("1 order"))
+    end
   end
 end

spec/features/orders_index_spec.rb

@@ -77,9 +77,17 @@
     visit admin_orders_path
 
     click_on "Customer"
-    expect(page).to have_content(/Alpha.*Bravo.*Charlie/)
+    within :table do
+      expect(page).to have_css("tbody tr:nth-child(1)", text: "Alpha")
+      expect(page).to have_css("tbody tr:nth-child(2)", text: "Bravo")
+      expect(page).to have_css("tbody tr:nth-child(3)", text: "Charlie")
+    end
 
     click_on "Customer"
-    expect(page).to have_content(/Charlie.*Bravo.*Alpha/)
+    within :table do
+      expect(page).to have_css("tbody tr:nth-child(1)", text: "Charlie")
+      expect(page).to have_css("tbody tr:nth-child(2)", text: "Bravo")
+      expect(page).to have_css("tbody tr:nth-child(3)", text: "Alpha")
+    end
   end
 end

spec/features/products_index_spec.rb

@@ -53,12 +53,25 @@
     )
 
     visit admin_products_path
-    expect(page).to have_content(/Gamma.*Alpha.*Beta/)
+
+    within :table do
+      expect(page).to have_css("tbody tr:nth-child(1)", text: "Gamma")
+      expect(page).to have_css("tbody tr:nth-child(2)", text: "Alpha")
+      expect(page).to have_css("tbody tr:nth-child(3)", text: "Beta")
+    end
 
     click_on "Product Meta Tag"
-    expect(page).to have_content(/Alpha.*Beta.*Gamma/)
+    within :table do
+      expect(page).to have_css("tbody tr:nth-child(1)", text: "Alpha")
+      expect(page).to have_css("tbody tr:nth-child(2)", text: "Beta")
+      expect(page).to have_css("tbody tr:nth-child(3)", text: "Gamma")
+    end
 
     click_on "Product Meta Tag"
-    expect(page).to have_content(/Gamma.*Beta.*Alpha/)
+    within :table do
+      expect(page).to have_css("tbody tr:nth-child(1)", text: "Gamma")
+      expect(page).to have_css("tbody tr:nth-child(2)", text: "Beta")
+      expect(page).to have_css("tbody tr:nth-child(3)", text: "Alpha")
+    end
   end
 end