From ec5bbd27f5b334a6e338574ebafe6b0cd39461bc Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 23 Feb 2019 11:00:38 -0800
Subject: [PATCH] Remove duplicate XHR request when requesting new pipeline
 page

When a user clicked on another page in the pipeline page, the following
would happen:

1. PipelinesService.getPipelines() would be called to fetch the new page
2. In the success handler, Poll.restart() would be called
3. This would invoke Poll.makeRequest(), which would fire off a new
request.

To fix this, we introduce a enable(), which will:

1. Update the request data accordingly
2. Clear the old timeout if necessary and start a new timer

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/58095
---
 app/assets/javascripts/lib/utils/poll.js      | 20 ++++++++++--
 .../javascripts/pipelines/mixins/pipelines.js |  3 +-
 .../sh-fix-double-xhr-pipelines.yml           |  5 +++
 spec/javascripts/lib/utils/poll_spec.js       | 32 +++++++++++++++++++
 4 files changed, 55 insertions(+), 5 deletions(-)
 create mode 100644 changelogs/unreleased/sh-fix-double-xhr-pipelines.yml

diff --git a/app/assets/javascripts/lib/utils/poll.js b/app/assets/javascripts/lib/utils/poll.js
index 198711cf427..a900ff34bf5 100644
--- a/app/assets/javascripts/lib/utils/poll.js
+++ b/app/assets/javascripts/lib/utils/poll.js
@@ -63,6 +63,10 @@ export default class Poll {
     const headers = normalizeHeaders(response.headers);
     const pollInterval = parseInt(headers[this.intervalHeader], 10);
     if (pollInterval > 0 && successCodes.indexOf(response.status) !== -1 && this.canPoll) {
+      if (this.timeoutID) {
+        clearTimeout(this.timeoutID);
+      }
+
       this.timeoutID = setTimeout(() => {
         this.makeRequest();
       }, pollInterval);
@@ -101,15 +105,25 @@ export default class Poll {
   }
 
   /**
-   * Restarts polling after it has been stoped
+   * Enables polling after it has been stopped
    */
-  restart(options) {
-    // update data
+  enable(options) {
     if (options && options.data) {
       this.options.data = options.data;
     }
 
     this.canPoll = true;
+
+    if (options && options.response) {
+      this.checkConditions(options.response);
+    }
+  }
+
+  /**
+   * Restarts polling after it has been stopped and makes a request
+   */
+  restart(options) {
+    this.enable(options);
     this.makeRequest();
   }
 }
diff --git a/app/assets/javascripts/pipelines/mixins/pipelines.js b/app/assets/javascripts/pipelines/mixins/pipelines.js
index 32bfa47e5f2..74ca3071364 100644
--- a/app/assets/javascripts/pipelines/mixins/pipelines.js
+++ b/app/assets/javascripts/pipelines/mixins/pipelines.js
@@ -94,8 +94,7 @@ export default {
           this.isLoading = false;
           this.successCallback(response);
 
-          // restart polling
-          this.poll.restart({ data: this.requestData });
+          this.poll.enable({ data: this.requestData, response });
         })
         .catch(() => {
           this.isLoading = false;
diff --git a/changelogs/unreleased/sh-fix-double-xhr-pipelines.yml b/changelogs/unreleased/sh-fix-double-xhr-pipelines.yml
new file mode 100644
index 00000000000..e6c762f1d47
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-double-xhr-pipelines.yml
@@ -0,0 +1,5 @@
+---
+title: Remove duplicate XHR request when requesting new pipeline page
+merge_request: 25506
+author:
+type: fixed
diff --git a/spec/javascripts/lib/utils/poll_spec.js b/spec/javascripts/lib/utils/poll_spec.js
index d0da659c3d7..138041a349f 100644
--- a/spec/javascripts/lib/utils/poll_spec.js
+++ b/spec/javascripts/lib/utils/poll_spec.js
@@ -153,6 +153,36 @@ describe('Poll', () => {
     });
   });
 
+  describe('enable', () => {
+    it('should enable polling upon a response', done => {
+      jasmine.clock().install();
+
+      const Polling = new Poll({
+        resource: service,
+        method: 'fetch',
+        data: { page: 1 },
+        successCallback: () => {},
+      });
+
+      Polling.enable({
+        data: { page: 4 },
+        response: { status: 200, headers: { 'poll-interval': 1 } },
+      });
+
+      jasmine.clock().tick(1);
+      jasmine.clock().uninstall();
+
+      waitForAllCallsToFinish(service, 1, () => {
+        Polling.stop();
+
+        expect(service.fetch.calls.count()).toEqual(1);
+        expect(service.fetch).toHaveBeenCalledWith({ page: 4 });
+        expect(Polling.options.data).toEqual({ page: 4 });
+        done();
+      });
+    });
+  });
+
   describe('restart', () => {
     it('should restart polling when its called', done => {
       mockServiceCall(service, { status: 200, headers: { 'poll-interval': 1 } });
@@ -171,6 +201,7 @@ describe('Poll', () => {
       });
 
       spyOn(Polling, 'stop').and.callThrough();
+      spyOn(Polling, 'enable').and.callThrough();
       spyOn(Polling, 'restart').and.callThrough();
 
       Polling.makeRequest();
@@ -181,6 +212,7 @@ describe('Poll', () => {
         expect(service.fetch.calls.count()).toEqual(2);
         expect(service.fetch).toHaveBeenCalledWith({ page: 4 });
         expect(Polling.stop).toHaveBeenCalled();
+        expect(Polling.enable).toHaveBeenCalled();
         expect(Polling.restart).toHaveBeenCalled();
         expect(Polling.options.data).toEqual({ page: 4 });
         done();