From 0b301416467dffe8437ec5251c3177d4c9edf3b8 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Thu, 12 Jan 2023 12:03:08 -0700
Subject: [PATCH 1/4] Move MR Widget state polling into Apollo

---
 .../vue_merge_request_widget/constants.js     |  5 ++
 .../mr_widget_options.vue                     | 72 ++++++++++++-------
 2 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/app/assets/javascripts/vue_merge_request_widget/constants.js b/app/assets/javascripts/vue_merge_request_widget/constants.js
index dbfbd35b9b68a0..185037208149af 100644
--- a/app/assets/javascripts/vue_merge_request_widget/constants.js
+++ b/app/assets/javascripts/vue_merge_request_widget/constants.js
@@ -2,6 +2,11 @@ import { s__ } from '~/locale';
 import { helpPagePath } from '~/helpers/help_page_helper';
 import { stateToComponentMap as classStateMap, stateKey } from './stores/state_maps';
 
+export const FOUR_MINUTES_IN_MS = 1000 * 60 * 4;
+
+export const STATE_QUERY_POLLING_INTERVAL_DEFAULT = 5000;
+export const STATE_QUERY_POLLING_INTERVAL_BACKOFF = 2;
+
 export const SUCCESS = 'success';
 export const WARNING = 'warning';
 export const INFO = 'info';
diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
index 8867478654ace4..ce437760101cb3 100644
--- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
@@ -1,5 +1,5 @@
 <script>
-import { isEmpty } from 'lodash';
+import { isEmpty, clamp } from 'lodash';
 import {
   registerExtension,
   registeredExtensions,
@@ -10,7 +10,6 @@ import MRWidgetService from 'ee_else_ce/vue_merge_request_widget/services/mr_wid
 import MRWidgetStore from 'ee_else_ce/vue_merge_request_widget/stores/mr_widget_store';
 import { stateToComponentMap as classState } from 'ee_else_ce/vue_merge_request_widget/stores/state_maps';
 import { createAlert } from '~/alert';
-import { secondsToMilliseconds } from '~/lib/utils/datetime_utility';
 import notify from '~/lib/utils/notify';
 import { sprintf, s__, __ } from '~/locale';
 import Project from '~/pages/projects/project';
@@ -44,7 +43,13 @@ import UnresolvedDiscussionsState from './components/states/unresolved_discussio
 import WorkInProgressState from './components/states/work_in_progress.vue';
 import ExtensionsContainer from './components/extensions/container';
 import WidgetContainer from './components/widget/app.vue';
-import { STATE_MACHINE, stateToComponentMap } from './constants';
+import {
+  STATE_MACHINE,
+  stateToComponentMap,
+  STATE_QUERY_POLLING_INTERVAL_DEFAULT,
+  STATE_QUERY_POLLING_INTERVAL_BACKOFF,
+  FOUR_MINUTES_IN_MS,
+} from './constants';
 import eventHub from './event_hub';
 import mergeRequestQueryVariablesMixin from './mixins/merge_request_query_variables';
 import getStateQuery from './queries/get_state.query.graphql';
@@ -99,6 +104,7 @@ export default {
   apollo: {
     state: {
       query: getStateQuery,
+      notifyOnNetworkStatusChange: true,
       manual: true,
       skip() {
         return !this.mr;
@@ -106,10 +112,18 @@ export default {
       variables() {
         return this.mergeRequestQueryVariables;
       },
-      result({ data: { project } }) {
-        if (project) {
-          this.mr.setGraphqlData(project);
+      pollInterval() {
+        return this.pollInterval;
+      },
+      result(response) {
+        if (!response.loading) {
+          this.pollInterval = this.apolloStateQueryPollingInterval;
+        }
+
+        if (response.data?.project) {
+          this.mr.setGraphqlData(response.data.project);
           this.loading = false;
+          this.checkStatus(undefined, undefined, false);
         }
       },
       subscribeToMore: {
@@ -158,9 +172,27 @@ export default {
       loading: true,
       recomputeComponentName: 0,
       issuableId: false,
+      startingPollInterval: STATE_QUERY_POLLING_INTERVAL_DEFAULT,
+      pollInterval: STATE_QUERY_POLLING_INTERVAL_DEFAULT,
     };
   },
   computed: {
+    apolloStateQueryMaxPollingInterval() {
+      return this.startingPollInterval + FOUR_MINUTES_IN_MS;
+    },
+    apolloStateQueryPollingInterval() {
+      if (this.startingPollInterval < 0) {
+        return 0;
+      }
+
+      const unboundedInterval = STATE_QUERY_POLLING_INTERVAL_BACKOFF * this.pollInterval;
+
+      return clamp(
+        unboundedInterval,
+        this.startingPollInterval,
+        this.apolloStateQueryMaxPollingInterval,
+      );
+    },
     shouldRenderApprovals() {
       return this.mr.state !== 'nothingToMerge';
     },
@@ -284,7 +316,8 @@ export default {
   mounted() {
     MRWidgetService.fetchInitialData()
       .then(({ data, headers }) => {
-        this.startingPollInterval = Number(headers['POLL-INTERVAL']);
+        this.startingPollInterval =
+          Number(headers['POLL-INTERVAL']) || STATE_QUERY_POLLING_INTERVAL_DEFAULT;
         this.initWidget(data);
       })
       .catch(() =>
@@ -295,9 +328,6 @@ export default {
   },
   beforeDestroy() {
     eventHub.$off('mr.discussion.updated', this.checkStatus);
-    if (this.pollingInterval) {
-      this.pollingInterval.destroy();
-    }
 
     if (this.deploymentsInterval) {
       this.deploymentsInterval.destroy();
@@ -332,7 +362,6 @@ export default {
         this.initPostMergeDeploymentsPolling();
       }
 
-      this.initPolling();
       this.bindEventHubListeners();
       eventHub.$on('mr.discussion.updated', this.checkStatus);
 
@@ -363,8 +392,10 @@ export default {
     createService(store) {
       return new MRWidgetService(this.getServiceEndpoints(store));
     },
-    checkStatus(cb, isRebased) {
-      this.$apollo.queries.state.refetch();
+    checkStatus(cb, isRebased, refetch = true) {
+      if (refetch) {
+        this.$apollo.queries.state.refetch();
+      }
 
       return this.service
         .checkStatus()
@@ -389,17 +420,6 @@ export default {
       }
       return Promise.resolve();
     },
-    initPolling() {
-      if (this.startingPollInterval <= 0) return;
-
-      this.pollingInterval = new SmartInterval({
-        callback: this.checkStatus,
-        startingInterval: this.startingPollInterval,
-        maxInterval: this.startingPollInterval + secondsToMilliseconds(4 * 60),
-        hiddenInterval: secondsToMilliseconds(6 * 60),
-        incrementByFactorOf: 2,
-      });
-    },
     initDeploymentsPolling() {
       this.deploymentsInterval = this.deploymentsPoll(this.fetchPreMergeDeployments);
     },
@@ -476,10 +496,10 @@ export default {
       notify.notifyMe(title, message, this.mr.gitlabLogo);
     },
     resumePolling() {
-      this.pollingInterval?.resume();
+      this.$apollo.queries.state.startPolling(this.pollInterval);
     },
     stopPolling() {
-      this.pollingInterval?.stopTimer();
+      this.$apollo.queries.state.stopPolling();
     },
     bindEventHubListeners() {
       eventHub.$on('MRWidgetUpdateRequested', (cb) => {
-- 
GitLab


From 3628265acb0c850b4bfc00adf6b01a849a8823cd Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Thu, 12 Jan 2023 17:27:19 -0700
Subject: [PATCH 2/4] Update MR widget options tests with removed polling code

---
 .../mr_widget_options_spec.js                 | 130 +++++++++++-------
 1 file changed, 81 insertions(+), 49 deletions(-)

diff --git a/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js b/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js
index 171ebbe8279186..f73cccabb6b775 100644
--- a/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js
+++ b/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js
@@ -1,5 +1,5 @@
 import { GlBadge, GlLink, GlIcon, GlButton, GlDropdown } from '@gitlab/ui';
-import { mount } from '@vue/test-utils';
+import { mount, shallowMount } from '@vue/test-utils';
 import MockAdapter from 'axios-mock-adapter';
 import Vue, { nextTick } from 'vue';
 import VueApollo from 'vue-apollo';
@@ -21,6 +21,7 @@ import {
   registerExtension,
   registeredExtensions,
 } from '~/vue_merge_request_widget/components/extensions';
+import { STATE_QUERY_POLLING_INTERVAL_BACKOFF } from '~/vue_merge_request_widget/constants';
 import { SUCCESS } from '~/vue_merge_request_widget/components/deployment/constants';
 import eventHub from '~/vue_merge_request_widget/event_hub';
 import MrWidgetOptions from '~/vue_merge_request_widget/mr_widget_options.vue';
@@ -62,6 +63,8 @@ jest.mock('@sentry/browser', () => ({
 Vue.use(VueApollo);
 
 describe('MrWidgetOptions', () => {
+  let stateQueryHandler;
+  let queryResponse;
   let wrapper;
   let mock;
 
@@ -91,32 +94,36 @@ describe('MrWidgetOptions', () => {
     gon.features = {};
   });
 
-  const createComponent = (mrData = mockData, options = {}) => {
-    wrapper = mount(MrWidgetOptions, {
+  const createComponent = (mrData = mockData, options = {}, data = {}, fullMount = true) => {
+    const mounting = fullMount ? mount : shallowMount;
+
+    queryResponse = {
+      data: {
+        project: {
+          ...getStateQueryResponse.data.project,
+          mergeRequest: {
+            ...getStateQueryResponse.data.project.mergeRequest,
+            mergeError: mrData.mergeError || null,
+          },
+        },
+      },
+    };
+    stateQueryHandler = jest.fn().mockResolvedValue(queryResponse);
+    wrapper = mounting(MrWidgetOptions, {
       propsData: {
         mrData: { ...mrData },
       },
       data() {
-        return { loading: false };
+        return {
+          loading: false,
+          ...data,
+        };
       },
 
       ...options,
       apolloProvider: createMockApollo([
         [approvalsQuery, jest.fn().mockResolvedValue(approvedByCurrentUser)],
-        [
-          getStateQuery,
-          jest.fn().mockResolvedValue({
-            data: {
-              project: {
-                ...getStateQueryResponse.data.project,
-                mergeRequest: {
-                  ...getStateQueryResponse.data.project.mergeRequest,
-                  mergeError: mrData.mergeError || null,
-                },
-              },
-            },
-          }),
-        ],
+        [getStateQuery, stateQueryHandler],
         [readyToMergeQuery, jest.fn().mockResolvedValue(readyToMergeResponse)],
         [
           userPermissionsQuery,
@@ -354,18 +361,6 @@ describe('MrWidgetOptions', () => {
         });
       });
 
-      describe('initPolling', () => {
-        it('should call SmartInterval', () => {
-          wrapper.vm.initPolling();
-
-          expect(SmartInterval).toHaveBeenCalledWith(
-            expect.objectContaining({
-              callback: wrapper.vm.checkStatus,
-            }),
-          );
-        });
-      });
-
       describe('initDeploymentsPolling', () => {
         it('should call SmartInterval', () => {
           wrapper.vm.initDeploymentsPolling();
@@ -532,23 +527,68 @@ describe('MrWidgetOptions', () => {
         });
       });
 
-      describe('resumePolling', () => {
-        it('should call stopTimer on pollingInterval', () => {
-          jest.spyOn(wrapper.vm.pollingInterval, 'resume').mockImplementation(() => {});
+      describe('Apollo query', () => {
+        const interval = 5;
+        const data = 'foo';
+        const mockCheckStatus = jest.fn().mockResolvedValue({ data });
+        const mockSetGraphqlData = jest.fn();
+        const mockSetData = jest.fn();
 
-          wrapper.vm.resumePolling();
+        beforeEach(() => {
+          wrapper.destroy();
+
+          return createComponent(
+            mockData,
+            {},
+            {
+              pollInterval: interval,
+              startingPollInterval: interval,
+              mr: {
+                setData: mockSetData,
+                setGraphqlData: mockSetGraphqlData,
+              },
+              service: {
+                checkStatus: mockCheckStatus,
+              },
+            },
+            false,
+          );
+        });
 
-          expect(wrapper.vm.pollingInterval.resume).toHaveBeenCalled();
+        describe('normal polling behavior', () => {
+          it('responds to the GraphQL query finishing', () => {
+            expect(mockSetGraphqlData).toHaveBeenCalledWith(queryResponse.data.project);
+            expect(mockCheckStatus).toHaveBeenCalled();
+            expect(mockSetData).toHaveBeenCalledWith(data, undefined);
+            expect(stateQueryHandler).toHaveBeenCalledTimes(1);
+          });
         });
-      });
 
-      describe('stopPolling', () => {
-        it('should call stopTimer on pollingInterval', () => {
-          jest.spyOn(wrapper.vm.pollingInterval, 'stopTimer').mockImplementation(() => {});
+        describe('resumePolling', () => {
+          it('should start the Apollo query polling', async () => {
+            wrapper.vm.resumePolling();
 
-          wrapper.vm.stopPolling();
+            expect(stateQueryHandler).toHaveBeenCalled();
+            jest.advanceTimersByTime(interval * STATE_QUERY_POLLING_INTERVAL_BACKOFF);
+            expect(stateQueryHandler).toHaveBeenCalledTimes(2);
+          });
+        });
 
-          expect(wrapper.vm.pollingInterval.stopTimer).toHaveBeenCalled();
+        describe('stopPolling', () => {
+          it('should stop the Apollo query from polling', () => {
+            expect(stateQueryHandler).toHaveBeenCalledTimes(1);
+            // After the query is run once, the interval is advanced by the backoff amount
+            const newInterval = interval * STATE_QUERY_POLLING_INTERVAL_BACKOFF;
+            expect(wrapper.vm.pollInterval).toBe(newInterval);
+
+            wrapper.vm.stopPolling();
+
+            jest.advanceTimersByTime(newInterval);
+
+            // Because polling was stopped, the pollinterval never advances by another backoff factor
+            expect(wrapper.vm.pollInterval).toBe(newInterval);
+            expect(stateQueryHandler).toHaveBeenCalledTimes(1); // no additional polling after a real interval timeout
+          });
         });
       });
     });
@@ -893,11 +933,7 @@ describe('MrWidgetOptions', () => {
   });
 
   describe('mock extension', () => {
-    let pollRequest;
-
     beforeEach(() => {
-      pollRequest = jest.spyOn(Poll.prototype, 'makeRequest');
-
       registerExtension(workingExtension());
 
       createComponent();
@@ -948,10 +984,6 @@ describe('MrWidgetOptions', () => {
       expect(collapsedSection.findComponent(GlButton).exists()).toBe(true);
       expect(collapsedSection.findComponent(GlButton).text()).toBe('Full report');
     });
-
-    it('extension polling is not called if enablePolling flag is not passed', () => {
-      expect(pollRequest).toHaveBeenCalledTimes(0);
-    });
   });
 
   describe('expansion', () => {
-- 
GitLab


From 64836ca48ba28cdd5f7a4dbe2277c30c112bf7e1 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Tue, 14 Mar 2023 18:29:17 -0600
Subject: [PATCH 3/4] Split up checking the widget status to only when the
 query isn't running

---
 .../vue_merge_request_widget/mr_widget_options.vue       | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
index ce437760101cb3..bbad2c1322031b 100644
--- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
@@ -118,11 +118,12 @@ export default {
       result(response) {
         if (!response.loading) {
           this.pollInterval = this.apolloStateQueryPollingInterval;
-        }
 
-        if (response.data?.project) {
-          this.mr.setGraphqlData(response.data.project);
-          this.loading = false;
+          if (response.data?.project) {
+            this.mr.setGraphqlData(response.data.project);
+            this.loading = false;
+          }
+        } else {
           this.checkStatus(undefined, undefined, false);
         }
       },
-- 
GitLab


From 318400021c2baaa9e6fdead61e9c5b199f411a92 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Wed, 15 Mar 2023 12:17:35 -0600
Subject: [PATCH 4/4] Remove `wrapper.vm` from Apollo query tests

---
 .../mr_widget_options_spec.js                 | 36 +++++++++----------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js b/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js
index f73cccabb6b775..43ce9b77cd33a8 100644
--- a/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js
+++ b/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js
@@ -564,30 +564,26 @@ describe('MrWidgetOptions', () => {
           });
         });
 
-        describe('resumePolling', () => {
-          it('should start the Apollo query polling', async () => {
-            wrapper.vm.resumePolling();
-
-            expect(stateQueryHandler).toHaveBeenCalled();
-            jest.advanceTimersByTime(interval * STATE_QUERY_POLLING_INTERVAL_BACKOFF);
-            expect(stateQueryHandler).toHaveBeenCalledTimes(2);
+        describe('external event control', () => {
+          describe('enablePolling', () => {
+            it('enables the Apollo query polling using the event hub', () => {
+              eventHub.$emit('EnablePolling');
+
+              expect(stateQueryHandler).toHaveBeenCalled();
+              jest.advanceTimersByTime(interval * STATE_QUERY_POLLING_INTERVAL_BACKOFF);
+              expect(stateQueryHandler).toHaveBeenCalledTimes(2);
+            });
           });
-        });
 
-        describe('stopPolling', () => {
-          it('should stop the Apollo query from polling', () => {
-            expect(stateQueryHandler).toHaveBeenCalledTimes(1);
-            // After the query is run once, the interval is advanced by the backoff amount
-            const newInterval = interval * STATE_QUERY_POLLING_INTERVAL_BACKOFF;
-            expect(wrapper.vm.pollInterval).toBe(newInterval);
-
-            wrapper.vm.stopPolling();
+          describe('disablePolling', () => {
+            it('disables the Apollo query polling using the event hub', () => {
+              expect(stateQueryHandler).toHaveBeenCalledTimes(1);
 
-            jest.advanceTimersByTime(newInterval);
+              eventHub.$emit('DisablePolling');
+              jest.advanceTimersByTime(interval * STATE_QUERY_POLLING_INTERVAL_BACKOFF);
 
-            // Because polling was stopped, the pollinterval never advances by another backoff factor
-            expect(wrapper.vm.pollInterval).toBe(newInterval);
-            expect(stateQueryHandler).toHaveBeenCalledTimes(1); // no additional polling after a real interval timeout
+              expect(stateQueryHandler).toHaveBeenCalledTimes(1); // no additional polling after a real interval timeout
+            });
           });
         });
       });
-- 
GitLab