Repository: parse-community/Parse-SDK-JS
Pull request #1133 (2020-04-02 12:53:10)

Fix file upload progress
Based on the discussion from [#1096](https://github.com/parse-community/Parse-SDK-JS/issues/1096)

Thanks to @ladigitale for the advice. :+1:

codecov[bot]: # [Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1133?src=pr&el=h1) Report
> Merging [#1133](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1133?src=pr&el=desc) into [master](https://codecov.io/gh/parse-community/Parse-SDK-JS/commit/2081dc25ed76adf72a062b2e76714407b2febff4?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1133/graphs/tree.svg?width=650&token=8nCENM0QHj&height=150&src=pr)](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1133?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##           master   #1133      +/-   ##
=========================================
+ Coverage   92.29%   92.3%   +<.01%     
=========================================
  Files          54      54              
  Lines        5230    5235       +5     
  Branches     1168    1169       +1     
=========================================
+ Hits         4827    4832       +5     
  Misses        403     403
```


| [Impacted Files](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1133?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/RESTController.js](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1133/diff?src=pr&el=tree#diff-c3JjL1JFU1RDb250cm9sbGVyLmpz) | `84.51% <100%> (+0.51%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1133?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1133?src=pr&el=footer). Last update [2081dc2...7056135](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1133?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

JeromeDeLeon: I tested it without `type` and worked just fine. No duplication. Maybe the browser handles the calling of progress behind the scene if it should be calling progress for upload or progress for download.
JeromeDeLeon: I was thinking the same thing like putting `loaded` and `total` into an object allowing the future values to be wrapped into an object or putting `type` as an object at the end of the parameter but then again, the `type` could possibly remove if we could determine about the collision of `upload progress` and `download progress`. If you could test it on your end, that would be great help :+1: 

Patch:
diff --git a/package-lock.json b/package-lock.json
index 27c16cc..0609545 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -3240,6 +3240,18 @@
         "util": "~0.10.1",
         "vm-browserify": "^1.0.0",
         "xtend": "^4.0.0"
+      },
+      "dependencies": {
+        "buffer": {
+          "version": "5.2.1",
+          "resolved": "https://registry.npmjs.org/buffer/-/buffer-5.2.1.tgz",
+          "integrity": "sha512-c+Ko0loDaFfuPWiL02ls9Xd3GO3cPVmUobQ6t3rXNUk304u6hGq+8N/kFi+QEIKhzK3uwolVhLzszmfLmMLnqg==",
+          "dev": true,
+          "requires": {
+            "base64-js": "^1.0.2",
+            "ieee754": "^1.1.4"
+          }
+        }
       }
     },
     "browserify-aes": {
@@ -3340,16 +3352,6 @@
       "integrity": "sha512-S/yKGU1syOMzO86+dGpg2qGoDL0zvzcb262G+gqEy6TgP6rt6z6qxSFX/8X6vLC91P7G7C3nLs0+bvDzmvBA3Q==",
       "dev": true
     },
-    "buffer": {
-      "version": "5.5.0",
-      "resolved": "https://registry.npmjs.org/buffer/-/buffer-5.5.0.tgz",
-      "integrity": "sha512-9FTEDjLjwoAkEwyMGDjYJQN2gfRgOKBKRfiglhvibGbpeeU/pQn1bJxQqm32OD/AIeEuHxU9roxXxg34Byp/Ww==",
-      "dev": true,
-      "requires": {
-        "base64-js": "^1.0.2",
-        "ieee754": "^1.1.4"
-      }
-    },
     "buffer-equal": {
       "version": "1.0.0",
       "resolved": "https://registry.npmjs.org/buffer-equal/-/buffer-equal-1.0.0.tgz",
@@ -10898,7 +10900,7 @@
         "cors": "2.8.5",
         "deepcopy": "2.0.0",
         "express": "4.17.1",
-        "follow-redirects": "1.10.0",
+        "follow-redirects": "1.11.0",
         "graphql": "14.6.0",
         "graphql-list-fields": "2.0.2",
         "graphql-relay": "^0.6.0",
@@ -10912,7 +10914,7 @@
         "lru-cache": "5.1.1",
         "mime": "2.4.4",
         "mongodb": "3.5.5",
-        "parse": "2.11.0",
+        "parse": "2.12.0",
         "pg-promise": "10.4.4",
         "pluralize": "^8.0.0",
         "redis": "3.0.2",
diff --git a/src/RESTController.js b/src/RESTController.js
index 392c8d4..d24226b 100644
--- a/src/RESTController.js
+++ b/src/RESTController.js
@@ -158,15 +158,31 @@ const RESTController = {
       for (const key in customHeaders) {
         headers[key] = customHeaders[key];
       }
-      xhr.onprogress = (event) => {
-        if(options && typeof options.progress === 'function') {
+
+      function handleProgress(event) {
+        if (options && typeof options.progress === 'function') {
           if (event.lengthComputable) {
-            options.progress(event.loaded / event.total, event.loaded, event.total);
+            options.progress(
+              event.loaded / event.total,
+              event.loaded,
+              event.total
+            );
           } else {
             options.progress(null);
           }
         }
+      }
+
+      xhr.onprogress = (event) => {
+        handleProgress(event);
       };
+
+      if (xhr.upload) {
+        xhr.upload.onprogress = (event) => {
+          handleProgress(event);
+        }
+      }
+
       xhr.open(method, url, true);
 
       for (const h in headers) {


Test patch:
diff --git a/src/__tests__/ParseFile-test.js b/src/__tests__/ParseFile-test.js
index d04e042..4c14d50 100644
--- a/src/__tests__/ParseFile-test.js
+++ b/src/__tests__/ParseFile-test.js
@@ -30,7 +30,7 @@ jest.setMock('../LocalDatastore', mockLocalDatastore);
 function generateSaveMock(prefix) {
   return function(name, payload, options) {
     if (options && typeof options.progress === 'function') {
-      options.progress(0.5);
+      options.progress(0.5, 5, 10);
     }
     return Promise.resolve({
       name: name,
@@ -236,7 +236,7 @@ describe('ParseFile', () => {
     jest.spyOn(options, 'progress');
 
     return file.save(options).then(function(f) {
-      expect(options.progress).toHaveBeenCalledWith(0.5);
+      expect(options.progress).toHaveBeenCalledWith(0.5, 5, 10);
       expect(f).toBe(file);
       expect(f.name()).toBe('progress.txt');
       expect(f.url()).toBe('http://files.parsetfss.com/a/progress.txt');
diff --git a/src/__tests__/RESTController-test.js b/src/__tests__/RESTController-test.js
index 1b5fa9c..7859a12 100644
--- a/src/__tests__/RESTController-test.js
+++ b/src/__tests__/RESTController-test.js
@@ -428,7 +428,7 @@ describe('RESTController', () => {
     CoreManager.set('SERVER_AUTH_TOKEN', null);
   });
 
-  it('reports upload progress of the AJAX request when callback is provided', (done) => {
+  it('reports upload/download progress of the AJAX request when callback is provided', (done) => {
     const xhr = mockXHR([{ status: 200, response: { success: true } }], {
       progress: {
         lengthComputable: true,
@@ -445,6 +445,7 @@ describe('RESTController', () => {
 
     RESTController.ajax('POST', 'files/upload.txt', {}, {}, options).then(({ response, status }) => {
       expect(options.progress).toHaveBeenCalledWith(0.5, 5, 10);
+      expect(options.progress).toHaveBeenCalledTimes(2);
       expect(response).toEqual({ success: true });
       expect(status).toBe(200);
       done();
diff --git a/src/__tests__/test_helpers/mockXHR.js b/src/__tests__/test_helpers/mockXHR.js
index 75ee661..cd23a6c 100644
--- a/src/__tests__/test_helpers/mockXHR.js
+++ b/src/__tests__/test_helpers/mockXHR.js
@@ -22,6 +22,7 @@ function mockXHR(results, options = {}) {
   XHR.prototype = {
     open: function() { },
     setRequestHeader: function() { },
+    upload: function() { },
     send: function() {
       this.status = results[attempts].status;
       this.responseText = JSON.stringify(results[attempts].response || {});
@@ -29,6 +30,7 @@ function mockXHR(results, options = {}) {
       attempts++;
       this.onreadystatechange();
       this.onprogress(options.progress);
+      this.upload.onprogress(options.progress);
     }
   };
   return XHR;