From 4646eb8e98a59d368c5bfaa1496cdea107ad0736 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Wed, 16 Nov 2022 16:15:34 -0700
Subject: [PATCH] Properly change the URL when changing MR tabs

The fix for the problem is just in moving the
`.setCurrentAction` call into the `tabShown` function,
which should guarantee we're updating the URL every
time the tab changes.

However, the back button is still an issue here, because
the `popstate` handler is doing special logic outside of
the `tabShown` logic. By EXPECTING the popped entry to
have a `state` object (which we would have assigned).
For some requests (especially first loads), this is not true.

To "standardize" everything, the `popstate` handler now
just defers to the `tabShown` function with a simple
"extraction" of the action from the URL.

Changelog: fixed
---
 app/assets/javascripts/merge_request_tabs.js | 32 +++++++++++++-------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js
index 0ddf5def8ee571..fc2857cbcb4c83 100644
--- a/app/assets/javascripts/merge_request_tabs.js
+++ b/app/assets/javascripts/merge_request_tabs.js
@@ -161,6 +161,18 @@ function toggleLoader(state) {
   $('.mr-loading-status .loading').toggleClass('hide', !state);
 }
 
+function getActionFromHref(href) {
+  let action = new URL(href).pathname.match(/\/(commits|diffs|pipelines).*$/);
+
+  if (action) {
+    action = action[0].replace(/(^\/|\.html)/g, '');
+  } else {
+    action = 'show';
+  }
+
+  return action;
+}
+
 export default class MergeRequestTabs {
   constructor({ action, setUrl, stubLocation } = {}) {
     this.mergeRequestTabs = document.querySelector('.merge-request-tabs-container');
@@ -206,12 +218,11 @@ export default class MergeRequestTabs {
 
   bindEvents() {
     $('.merge-request-tabs a[data-toggle="tabvue"]').on('click', this.clickTab);
-    window.addEventListener('popstate', (event) => {
-      if (event.state && event.state.action) {
-        this.tabShown(event.state.action, event.target.location);
-        this.currentAction = event.state.action;
-        this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction());
-      }
+    window.addEventListener('popstate', () => {
+      const action = getActionFromHref(location.href);
+
+      this.tabShown(action, location.href);
+      this.eventHub.$emit('MergeRequestTabChange', action);
     });
   }
 
@@ -252,10 +263,6 @@ export default class MergeRequestTabs {
       } else if (action) {
         const href = e.currentTarget.getAttribute('href');
         this.tabShown(action, href);
-
-        if (this.setUrl) {
-          this.setCurrentAction(action);
-        }
       }
     }
   }
@@ -263,6 +270,9 @@ export default class MergeRequestTabs {
   tabShown(action, href, shouldScroll = true) {
     if (action !== this.currentTab && this.mergeRequestTabs) {
       this.currentTab = action;
+      if (this.setUrl) {
+        this.setCurrentAction(action);
+      }
 
       if (this.mergeRequestTabPanesAll) {
         this.mergeRequestTabPanesAll.forEach((el) => {
@@ -398,7 +408,7 @@ export default class MergeRequestTabs {
     // Ensure parameters and hash come along for the ride
     newState += location.search + location.hash;
 
-    if (window.history.state && window.history.state.url && window.location.pathname !== newState) {
+    if (window.location.pathname !== newState) {
       window.history.pushState(
         {
           url: newState,
-- 
GitLab