From c471974dfaf546c071924935ec74c6c419dbd6ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez-Capel?= <alberto@37signals.com>
Date: Mon, 20 Nov 2023 17:22:35 +0000
Subject: [PATCH] Change default action for form submissions that redirect to
 the current location (#1072)

* Change default action for form submissions that redirect to the current location

This changes the default action for form submission that redirect to the
current location from "advance" to "replace". This makes more sense since
we're re-rendering the current page, there should be no new entry in the
history.

We are also changing the morphing behaviour to only kick in when the
visit action is "replace". This is to avoid morphing the page when, for example,
we are follwing a link, unless we opt in to morphing by setting the
data-turbo-action attribute to replace.

The main use case for morphing remains the same: you submit a form that
changes some state and refirect to the same location. Turbo handles the
redirect and morphs the page to reflect the new state.

* Add parentheses to clarify conditional
---
 src/core/drive/navigator.js              | 12 +++++++++---
 src/core/drive/page_view.js              |  2 +-
 src/tests/fixtures/navigation.html       | 11 +++++++++++
 src/tests/functional/navigation_tests.js | 12 ++++++++++++
 4 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/src/core/drive/navigator.js b/src/core/drive/navigator.js
index 54b01e8ee..8d8d3e0be 100644
--- a/src/core/drive/navigator.js
+++ b/src/core/drive/navigator.js
@@ -78,7 +78,7 @@ export class Navigator {
         }
 
         const { statusCode, redirected } = fetchResponse
-        const action = this.getActionForFormSubmission(formSubmission)
+        const action = this.#getActionForFormSubmission(formSubmission, fetchResponse)
         const visitOptions = {
           action,
           shouldCacheSnapshot,
@@ -153,7 +153,13 @@ export class Navigator {
     return this.history.restorationIdentifier
   }
 
-  getActionForFormSubmission({ submitter, formElement }) {
-    return getVisitAction(submitter, formElement) || "advance"
+  #getActionForFormSubmission(formSubmission, fetchResponse) {
+    const { submitter, formElement } = formSubmission
+    return getVisitAction(submitter, formElement) || this.#getDefaultAction(fetchResponse)
+  }
+
+  #getDefaultAction(fetchResponse) {
+    const sameLocationRedirect = fetchResponse.redirected && fetchResponse.location.href === this.location?.href
+    return sameLocationRedirect ? "replace" : "advance"
   }
 }
diff --git a/src/core/drive/page_view.js b/src/core/drive/page_view.js
index 0508d0960..19d8a244b 100644
--- a/src/core/drive/page_view.js
+++ b/src/core/drive/page_view.js
@@ -56,7 +56,7 @@ export class PageView extends View {
   }
 
   isPageRefresh(visit) {
-    return !visit || this.lastRenderedLocation.href === visit.location.href
+    return !visit || (this.lastRenderedLocation.href === visit.location.href && visit.action === "replace")
   }
 
   get snapshot() {
diff --git a/src/tests/fixtures/navigation.html b/src/tests/fixtures/navigation.html
index 12b0bc537..7b698a613 100644
--- a/src/tests/fixtures/navigation.html
+++ b/src/tests/fixtures/navigation.html
@@ -79,6 +79,17 @@ <h1>Navigation</h1>
           </custom-link-element>
         </turbo-toggle>
       </p>
+      <p>
+        <form action="/__turbo/redirect" method="post" class="redirect" data-turbo-frame="_top">
+          <input type="hidden" name="path" value="/src/tests/fixtures/one.html">
+          <button id="redirect-submit">Redirect to the another location</button>
+        </form>
+      </p>
+      <p>
+        <form action="/__turbo/refresh" method="post" class="redirect">
+          <button id="refresh-submit">Redirect to the current location</button>
+        </form>
+      </p>
       <p><a id="delayed-link" href="/__turbo/delayed_response">Delayed link</a></p>
       <p><a id="delayed-failure-link" href="/__turbo/delayed_response?status=500">Delayed failure link</a></p>
       <p><a id="link-target-iframe" href="/src/tests/fixtures/one.html" target="iframe">Targets iframe[name="iframe"]</a></p>
diff --git a/src/tests/functional/navigation_tests.js b/src/tests/functional/navigation_tests.js
index 73871e1c2..cdbc90e22 100644
--- a/src/tests/functional/navigation_tests.js
+++ b/src/tests/functional/navigation_tests.js
@@ -307,6 +307,18 @@ test("clicking the forward button", async ({ page }) => {
   assert.equal(await visitAction(page), "restore")
 })
 
+test("form submissions that redirect to a different location have a default advance action", async ({ page }) => {
+  await page.click("#redirect-submit")
+  await nextBody(page)
+  assert.equal(await visitAction(page), "advance")
+})
+
+test("form submissions that redirect to the current location have a default replace action", async ({ page }) => {
+  await page.click("#refresh-submit")
+  await nextBody(page)
+  assert.equal(await visitAction(page), "replace")
+})
+
 test("link targeting a disabled turbo-frame navigates the page", async ({ page }) => {
   await page.click("#link-to-disabled-frame")
   await nextBody(page)