improve: deeplinking behavior (#4960)

* add passing tests for fragment rewriting

* add failing fragment retention tests

* fire legacy `show` actions before standard ones

* skip failing tests

* remove extra line

Author: shockey

src/core/plugins/deep-linking/layout.js

@@ -77,22 +77,21 @@ export const parseDeepLinkHash = (rawHash) => ({ layoutActions, layoutSelectors,
 
     const isShownKey = layoutSelectors.isShownKeyFromUrlHashArray(hashArray)
 
-    const [type, tagId, maybeOperationId] = isShownKey
+    const [type, tagId = "", maybeOperationId = ""] = isShownKey
 
     if(type === "operations") {
       // we're going to show an operation, so we need to expand the tag as well
       const tagIsShownKey = layoutSelectors.isShownKeyFromUrlHashArray([tagId])
-      layoutActions.show(tagIsShownKey)
 
       // If an `_` is present, trigger the legacy escaping behavior to be safe
       // TODO: remove this in v4.0, it is deprecated
       if(tagId.indexOf("_") > -1) {
         console.warn("Warning: escaping deep link whitespace with `_` will be unsupported in v4.0, use `%20` instead.")
         layoutActions.show(tagIsShownKey.map(val => val.replace(/_/g, " ")), true)
       }
-    }
 
-    layoutActions.show(isShownKey, true)
+      layoutActions.show(tagIsShownKey, true)
+    }
 
     // If an `_` is present, trigger the legacy escaping behavior to be safe
     // TODO: remove this in v4.0, it is deprecated
@@ -101,6 +100,8 @@ export const parseDeepLinkHash = (rawHash) => ({ layoutActions, layoutSelectors,
       layoutActions.show(isShownKey.map(val => val.replace(/_/g, " ")), true)
     }
 
+    layoutActions.show(isShownKey, true)
+
     // Scroll to the newly expanded entity
     layoutActions.scrollTo(isShownKey)
   }

test/e2e-cypress/tests/deep-linking.js

@@ -14,12 +14,13 @@ describe("Deep linking feature", () => {
 
     describe("Operation with whitespace in tag+id", () => {
       const elementToGet = ".opblock-post"
+      const correctFragment = "#/my%20Tag/my%20Operation"
 
       BaseDeeplinkTestFactory({
         baseUrl: swagger2BaseUrl,
         elementToGet,
         correctElementId: "operations-my_Tag-my_Operation",
-        correctFragment: "#/my%20Tag/my%20Operation",
+        correctFragment,
         correctHref: "#/my%20Tag/my%20Operation"
       })
 
@@ -31,6 +32,13 @@ describe("Deep linking feature", () => {
           .get(`${elementToGet}.is-open`)
           .should("exist")
       })
+
+      it.skip("should rewrite to the correct fragment when provided the legacy fragment", () => {
+        cy.visit(`${swagger2BaseUrl}${legacyFragment}`)
+          .reload()
+          .window()
+          .should("have.deep.property", "location.hash", correctFragment)
+      })
     })
 
     describe("Operation with underscores in tag+id", () => {
@@ -91,14 +99,13 @@ describe("Deep linking feature", () => {
 
     describe("Operation with whitespace in tag+id", () => {
       const elementToGet = ".opblock-post"
-      const correctElementId = "operations-my_Tag-my_Operation"
       const correctFragment = "#/my%20Tag/my%20Operation"
 
       BaseDeeplinkTestFactory({
         baseUrl: openAPI3BaseUrl,
         elementToGet: ".opblock-post",
         correctElementId: "operations-my_Tag-my_Operation",
-        correctFragment: "#/my%20Tag/my%20Operation",
+        correctFragment,
         correctHref: "#/my%20Tag/my%20Operation"
       })
 
@@ -110,6 +117,14 @@ describe("Deep linking feature", () => {
           .get(`${elementToGet}.is-open`)
           .should("exist")
       })
+
+
+      it.skip("should rewrite to the correct fragment when provided the legacy fragment", () => {
+        cy.visit(`${openAPI3BaseUrl}${legacyFragment}`)
+          .reload()
+          .window()
+          .should("have.deep.property", "location.hash", correctFragment)
+      })
     })
 
     describe("Operation with underscores in tag+id", () => {
@@ -187,4 +202,12 @@ function BaseDeeplinkTestFactory({ baseUrl, elementToGet, correctElementId, corr
       .get(`${elementToGet}.is-open`)
       .should("exist")
   })
+
+  it("should retain the correct fragment when reloaded", () => {
+    cy.visit(`${baseUrl}${correctFragment}`)
+      .reload()
+      .should("exist")
+      .window()
+      .should("have.deep.property", "location.hash", correctFragment)
+  })
 }