Repository: parse-community/Parse-SDK-JS
Pull request #910 (2019-09-05 23:07:09)

Add master-key-only option when saving config parameter
This allows to save a config parameter that can be read with master key only using `Parse.Config.save(parameters, masterKeyOnly)`.

An additional argument can be added to specify which of the parameters should be retrievable with master key only, for example:

```
Parse.Config.save(
    { internal: "i", public: "p" },
    { internal: true }
 );
```

A config parameter is retrievable without master key if it is not added in the `masterKeyOnly` argument or is added with value `false`.


codecov[bot]: # [Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/910?src=pr&el=h1) Report
> Merging [#910](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/910?src=pr&el=desc) into [master](https://codecov.io/gh/parse-community/Parse-SDK-JS/commit/2fc8da088b53a4bfde36bb0b6cfab8aebc7b868e?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `83.33%`.

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

```diff
@@            Coverage Diff             @@
##           master     #910      +/-   ##
==========================================
- Coverage   91.95%   91.93%   -0.02%     
==========================================
  Files          54       54              
  Lines        5045     5048       +3     
  Branches     1133     1133              
==========================================
+ Hits         4639     4641       +2     
- Misses        406      407       +1
```


| [Impacted Files](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/910?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/ParseConfig.js](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/910/diff?src=pr&el=tree#diff-c3JjL1BhcnNlQ29uZmlnLmpz) | `87.2% <83.33%> (-0.75%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/910?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/910?src=pr&el=footer). Last update [2fc8da0...aaa34db](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/910?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

mtrezza: The code coverage diff seems to be a false positive? The integration tests seem to cover all the changed code.
dplewis: @mtrezza The code coverage here refers to the unit tests and not the integration tests.
mtrezza: What should I do about the decreased coverage? Should I add unit test even though the integration tests cover it already?
dplewis: Even tho the coverage is a 100% now. A few unit test is recommended

Add tests here https://github.com/parse-community/Parse-SDK-JS/blob/master/src/__tests__/ParseConfig-test.js#L103

You can check out the ParseObject-test.js

Since you are passing in parameters, you want to check that those parameters get passed into the restController
mtrezza: > Since you are passing in parameters, you want to check that those parameters get passed into the restController

How would I do this, like this?

```
it('can save a config object with a parameter that be retrieved with masterkey only', async () => {
    ParseConfig.save(
      {internal: 'i', number: 12},
      {internal: true});
    expect(CoreManager.getRESTController().request.mock.calls[0]).toEqual([
      'PUT',
      'config',
      { params: {internal: 'i', number: 12}, masterKeyOnly: {internal: true} },
      { useMasterKey: true }]);
  });
```
dplewis: There are a few ways, you can do it

```
fit('can get a config object with master key', async () => {
    CoreManager.setRESTController({
      request(method, path, body, options) {
        expect(method).toBe('GET');
        expect(path).toBe('config');
        expect(body).toEqual({});
        expect(options).toEqual({ useMasterKey: true });
        return Promise.resolve({
          params: {
            str: 'hello',
            num: 45
          }
        });
      },
      ajax() {}
    });
    const config = await ParseConfig.get({ useMasterKey: true });
    expect(config.get('str')).toBe('hello');
    expect(config.get('num')).toBe(45);
    const path = Storage.generatePath('currentConfig');
    expect(JSON.parse(Storage.getItem(path))).toEqual({
      str: 'hello',
      num: 45,
    });
  });
```
mtrezza: The unit test fails I added fails because `encode` results in `undefined` for all values: https://github.com/parse-community/Parse-SDK-JS/blob/2fc8da088b53a4bfde36bb0b6cfab8aebc7b868e/src/ParseConfig.js#L190-L192

Strange, because that logic was not changed in this PR.
mtrezza: Amazing! Thanks @dplewis 

Patch:
diff --git a/package-lock.json b/package-lock.json
index d23ce5f..fd39a7e 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -6201,7 +6201,6 @@
       "version": "14.5.4",
       "resolved": "https://registry.npmjs.org/graphql/-/graphql-14.5.4.tgz",
       "integrity": "sha512-dPLvHoxy5m9FrkqWczPPRnH0X80CyvRE6e7Fa5AWEqEAzg9LpxHvKh24po/482E6VWHigOkAmb4xCp6P9yT9gw==",
-      "dev": true,
       "requires": {
         "iterall": "^1.2.2"
       }
@@ -7543,8 +7542,7 @@
     "iterall": {
       "version": "1.2.2",
       "resolved": "https://registry.npmjs.org/iterall/-/iterall-1.2.2.tgz",
-      "integrity": "sha512-yynBb1g+RFUPY64fTrFv7nsjRrENBQJaX2UL+2Szc9REFrSNm1rpSXHGzhmAy7a9uv3vlvgBlXnf9RqmPH1/DA==",
-      "dev": true
+      "integrity": "sha512-yynBb1g+RFUPY64fTrFv7nsjRrENBQJaX2UL+2Szc9REFrSNm1rpSXHGzhmAy7a9uv3vlvgBlXnf9RqmPH1/DA=="
     },
     "jasmine": {
       "version": "3.4.0",
@@ -10376,8 +10374,7 @@
     "pluralize": {
       "version": "8.0.0",
       "resolved": "https://registry.npmjs.org/pluralize/-/pluralize-8.0.0.tgz",
-      "integrity": "sha512-Nc3IT5yHzflTfbjgqWcCPpo7DaKy4FnpB0l/zCAW0Tc7jxAiuqSxHasntB3D7887LSrA93kDJ9IXovxJYxyLCA==",
-      "dev": true
+      "integrity": "sha512-Nc3IT5yHzflTfbjgqWcCPpo7DaKy4FnpB0l/zCAW0Tc7jxAiuqSxHasntB3D7887LSrA93kDJ9IXovxJYxyLCA=="
     },
     "pn": {
       "version": "1.1.0",
diff --git a/src/ParseConfig.js b/src/ParseConfig.js
index fcfccd4..0960578 100644
--- a/src/ParseConfig.js
+++ b/src/ParseConfig.js
@@ -92,14 +92,20 @@ class ParseConfig {
   /**
    * Save value keys to the server.
    * @static
+   * @param {Object} attrs The config parameters and values.
+   * @param {Object} masterKeyOnlyFlags The flags that define whether config parameters listed
+   * in `attrs` should be retrievable only by using the master key.
+   * For example: `param1: true` makes `param1` only retrievable by using the master key.
+   * If a parameter is not provided or set to `false`, it can be retrieved without
+   * using the master key.
    * @return {Promise} A promise that is resolved with a newly-created
    *     configuration object or with the current with the update.
    */
-  static save(attrs: { [key: string]: any }) {
+  static save(attrs: { [key: string]: any }, masterKeyOnlyFlags: { [key: string]: any }) {
     const controller = CoreManager.getConfigController();
     //To avoid a mismatch with the local and the cloud config we get a new version
-    return controller.save(attrs).then(() => {
-      return controller.get();
+    return controller.save(attrs, masterKeyOnlyFlags).then(() => {
+      return controller.get({ useMasterKey: true });
     },(error) => {
       return Promise.reject(error);
     });
@@ -184,7 +190,7 @@ const DefaultController = {
     });
   },
 
-  save(attrs: { [key: string]: any }) {
+  save(attrs: { [key: string]: any }, masterKeyOnlyFlags: { [key: string]: any }) {
     const RESTController = CoreManager.getRESTController();
     const encodedAttrs = {};
     for(const key in attrs){
@@ -193,7 +199,7 @@ const DefaultController = {
     return RESTController.request(
       'PUT',
       'config',
-      { params: encodedAttrs },
+      { params: encodedAttrs, masterKeyOnly: masterKeyOnlyFlags },
       { useMasterKey: true }
     ).then(response => {
       if(response && response.result){


Test patch:
diff --git a/integration/test/ParseConfigTest.js b/integration/test/ParseConfigTest.js
index 3448972..a6b1a79 100644
--- a/integration/test/ParseConfigTest.js
+++ b/integration/test/ParseConfigTest.js
@@ -5,15 +5,9 @@ const clear = require('./clear');
 const Parse = require('../../node');
 
 function testConfig() {
-  const data = {
-    params: { internal: 'i', public: 'p' },
-    masterKeyOnly: { internal: true },
-  };
-  return Parse.CoreManager.getRESTController().request(
-    'PUT',
-    'config',
-    data,
-    { useMasterKey: true }
+  return Parse.Config.save(
+    { internal: "i", string: "s", number: 12 },
+    { internal: true }
   );
 }
 
@@ -28,22 +22,21 @@ describe('Parse Config', () => {
   });
 
   it('can create a config', async () => {
-    const config = await Parse.Config.save({
-      str: 'hello',
-      num: 42
-    });
-    assert.equal(config.get('str'), 'hello');
-    assert.equal(config.get('num'), 42);
+    const config = await testConfig();
+
+    assert.notStrictEqual(config, undefined);
+    assert.strictEqual(config.get('string'), 's');
+    assert.strictEqual(config.get('internal'), 'i');
+    assert.strictEqual(config.get('number'), 12);
   });
 
   it('can get a config', async () => {
-    await Parse.Config.save({
-      str: 'hello',
-      num: 42
-    });
+    await testConfig();
+
     const config = await Parse.Config.get();
-    assert.equal(config.get('str'), 'hello');
-    assert.equal(config.get('num'), 42);
+    assert.notStrictEqual(config, undefined);
+    assert.strictEqual(config.get('string'), 's');
+    assert.strictEqual(config.get('number'), 12);
   });
 
   it('can get internal config parameter with masterkey', async () => {
@@ -51,7 +44,7 @@ describe('Parse Config', () => {
 
     const config = await Parse.Config.get({ useMasterKey: true });
     assert.equal(config.get('internal'), 'i');
-    assert.equal(config.get('public'), 'p');
+    assert.equal(config.get('string'), 's');
   });
 
   it('cannot get internal config parameter without masterkey', async () => {
@@ -59,6 +52,6 @@ describe('Parse Config', () => {
 
     const config = await Parse.Config.get();
     assert.equal(config.get('internal'), undefined);
-    assert.equal(config.get('public'), 'p');
+    assert.equal(config.get('string'), 's');
   });
 });
diff --git a/src/__tests__/ParseConfig-test.js b/src/__tests__/ParseConfig-test.js
index be88361..a3fab0a 100644
--- a/src/__tests__/ParseConfig-test.js
+++ b/src/__tests__/ParseConfig-test.js
@@ -9,6 +9,7 @@
 
 jest.dontMock('../CoreManager');
 jest.dontMock('../decode');
+jest.dontMock('../encode');
 jest.dontMock('../escape');
 jest.dontMock('../ParseConfig');
 jest.dontMock('../ParseError');
@@ -126,6 +127,74 @@ describe('ParseConfig', () => {
     });
   });
 
+  it('can save a config object that be retrieved with masterkey only', async () => {
+    CoreManager.setRESTController({
+      request(method, path, body, options) {
+        console.log(method, path, body, options);
+        if (method === 'PUT') {
+          expect(method).toBe('PUT');
+          expect(path).toBe('config');
+          expect(body).toEqual({
+            params: { internal: 'i', number: 12 },
+            masterKeyOnly: { internal: true },
+          });
+          expect(options).toEqual({ useMasterKey: true });
+          return Promise.resolve({
+            params: {
+              internal: 'i',
+              number: 12
+            },
+            result: true,
+          });
+        } else if (method === 'GET') {
+          expect(method).toBe('GET');
+          expect(path).toBe('config');
+          expect(body).toEqual({});
+          expect(options).toEqual({ useMasterKey: true });
+          return Promise.resolve({
+            params: {
+              internal: 'i',
+              number: 12
+            },
+          });
+        }
+      },
+      ajax() {}
+    });
+    const config = await ParseConfig.save(
+      { internal: 'i', number: 12 },
+      { internal: true }
+    );
+    expect(config.get('internal')).toBe('i');
+    expect(config.get('number')).toBe(12);
+  });
+
+  it('can get a config object with master key', async () => {
+    CoreManager.setRESTController({
+      request(method, path, body, options) {
+        expect(method).toBe('GET');
+        expect(path).toBe('config');
+        expect(body).toEqual({});
+        expect(options).toEqual({ useMasterKey: true });
+        return Promise.resolve({
+          params: {
+            str: 'hello',
+            num: 45
+          }
+        });
+      },
+      ajax() {}
+    });
+    const config = await ParseConfig.get({ useMasterKey: true });
+    expect(config.get('str')).toBe('hello');
+    expect(config.get('num')).toBe(45);
+    const path = Storage.generatePath('currentConfig');
+    expect(JSON.parse(Storage.getItem(path))).toEqual({
+      str: 'hello',
+      num: 45,
+    });
+  });
+
   it('rejects save on invalid response', (done) => {
     CoreManager.setRESTController({
       request() {