Merge branch '49747-update-poll-2xx' into 'master'

Resolve "Update poll.js to keep polling if the status code is 2xx"

Closes #49747

See merge request gitlab-org/gitlab-ce!20904

app/assets/javascripts/lib/utils/http_status.js

@@ -2,11 +2,34 @@
  * exports HTTP status codes
  */
 
-export default {
+const httpStatusCodes = {
   ABORTED: 0,
-  NO_CONTENT: 204,
   OK: 200,
+  CREATED: 201,
+  ACCEPTED: 202,
+  NON_AUTHORITATIVE_INFORMATION: 203,
+  NO_CONTENT: 204,
+  RESET_CONTENT: 205,
+  PARTIAL_CONTENT: 206,
+  MULTI_STATUS: 207,
+  ALREADY_REPORTED: 208,
+  IM_USED: 226,
   MULTIPLE_CHOICES: 300,
   BAD_REQUEST: 400,
   NOT_FOUND: 404,
 };
+
+export const successCodes = [
+  httpStatusCodes.OK,
+  httpStatusCodes.CREATED,
+  httpStatusCodes.ACCEPTED,
+  httpStatusCodes.NON_AUTHORITATIVE_INFORMATION,
+  httpStatusCodes.NO_CONTENT,
+  httpStatusCodes.RESET_CONTENT,
+  httpStatusCodes.PARTIAL_CONTENT,
+  httpStatusCodes.MULTI_STATUS,
+  httpStatusCodes.ALREADY_REPORTED,
+  httpStatusCodes.IM_USED,
+];
+
+export default httpStatusCodes;

app/assets/javascripts/lib/utils/poll.js

-import httpStatusCodes from './http_status';
+import httpStatusCodes, { successCodes } from './http_status';
 import { normalizeHeaders } from './common_utils';
 
 /**
@@ -62,7 +62,7 @@ export default class Poll {
   checkConditions(response) {
     const headers = normalizeHeaders(response.headers);
     const pollInterval = parseInt(headers[this.intervalHeader], 10);
-    if (pollInterval > 0 && response.status === httpStatusCodes.OK && this.canPoll) {
+    if (pollInterval > 0 && successCodes.indexOf(response.status) !== -1 && this.canPoll) {
       clearTimeout(this.timeoutID);
       this.timeoutID = setTimeout(() => {
         this.makeRequest();

changelogs/unreleased/49747-update-poll-2xx.yml

+---
+title: Changes poll.js to keep polling on any 2xx http status code
+merge_request: 20904
+author:
+type: other

doc/development/fe_guide/performance.md

@@ -15,7 +15,7 @@ Use the following rules when creating realtime solutions.
 Use that as your polling interval. This way it is [easy for system administrators to change the
 polling rate](../../administration/polling.md).
 A `Poll-Interval: -1` means you should disable polling, and this must be implemented.
-1. A response with HTTP status `4XX` or `5XX` should disable polling as well.
+1. A response with HTTP status different from 2XX should disable polling as well.
 1. Use a common library for polling.
 1. Poll on active tabs only. Please use [Visibility](https://github.com/ai/visibilityjs).
 1. Use regular polling intervals, do not use backoff polling, or jitter, as the interval will be
@@ -25,15 +25,15 @@ controlled by the server.
 
 ### Lazy Loading Images
 
-To improve the time to first render we are using lazy loading for images. This works by setting 
-the actual image source on the `data-src` attribute. After the HTML is rendered and JavaScript is loaded, 
+To improve the time to first render we are using lazy loading for images. This works by setting
+the actual image source on the `data-src` attribute. After the HTML is rendered and JavaScript is loaded,
 the value of `data-src` will be moved to `src` automatically if the image is in the current viewport.
 
 *  Prepare images in HTML for lazy loading by renaming the `src` attribute to `data-src` AND adding the class `lazy`
 *  If you are using the Rails `image_tag` helper, all images will be lazy-loaded by default unless `lazy: false` is provided.
 
 If you are asynchronously adding content which contains lazy images then you need to call the function
-`gl.lazyLoader.searchLazyImages()` which will search for lazy images and load them if needed. 
+`gl.lazyLoader.searchLazyImages()` which will search for lazy images and load them if needed.
 But in general it should be handled automatically through a `MutationObserver` in the lazy loading function.
 
 ### Animations
@@ -97,19 +97,19 @@ bundle and included on the page.
 
     ```javascript
     import initMyWidget from './my_widget';
-  
+
     document.addEventListener('DOMContentLoaded', () => {
       initMyWidget();
     });
     ```
 
-- **Supporting Module Placement:**  
+- **Supporting Module Placement:**
     - If a class or a module is _specific to a particular route_, try to locate
       it close to the entry point it will be used. For instance, if
       `my_widget.js` is only imported within `pages/widget/show/index.js`, you
       should place the module at `pages/widget/show/my_widget.js` and import it
       with a relative path (e.g. `import initMyWidget from './my_widget';`).
-      
+
     - If a class or module is _used by multiple routes_, place it within a
       shared directory at the closest common parent directory for the entry
       points that import it.  For example, if `my_widget.js` is imported within

spec/javascripts/lib/utils/poll_spec.js

 import Poll from '~/lib/utils/poll';
+import { successCodes } from '~/lib/utils/http_status';
 
 const waitForAllCallsToFinish = (service, waitForCount, successCallback) => {
   const timer = () => {
@@ -91,28 +92,32 @@ describe('Poll', () => {
     }).catch(done.fail);
   });
 
-  it('starts polling when http status is 200 and interval header is provided', (done) => {
-    mockServiceCall(service, { status: 200, headers: { 'poll-interval': 1 } });
+  describe('for 2xx status code', () => {
+    successCodes.forEach(httpCode => {
+      it(`starts polling when http status is ${httpCode} and interval header is provided`, (done) => {
+        mockServiceCall(service, { status: httpCode, headers: { 'poll-interval': 1 } });
 
-    const Polling = new Poll({
-      resource: service,
-      method: 'fetch',
-      data: { page: 1 },
-      successCallback: callbacks.success,
-      errorCallback: callbacks.error,
-    });
+        const Polling = new Poll({
+          resource: service,
+          method: 'fetch',
+          data: { page: 1 },
+          successCallback: callbacks.success,
+          errorCallback: callbacks.error,
+        });
 
-    Polling.makeRequest();
+        Polling.makeRequest();
 
-    waitForAllCallsToFinish(service, 2, () => {
-      Polling.stop();
+        waitForAllCallsToFinish(service, 2, () => {
+          Polling.stop();
 
-      expect(service.fetch.calls.count()).toEqual(2);
-      expect(service.fetch).toHaveBeenCalledWith({ page: 1 });
-      expect(callbacks.success).toHaveBeenCalled();
-      expect(callbacks.error).not.toHaveBeenCalled();
+          expect(service.fetch.calls.count()).toEqual(2);
+          expect(service.fetch).toHaveBeenCalledWith({ page: 1 });
+          expect(callbacks.success).toHaveBeenCalled();
+          expect(callbacks.error).not.toHaveBeenCalled();
 
-      done();
+          done();
+        });
+      });
     });
   });