Repository: parse-community/Parse-SDK-JS
Pull request #729 (2019-02-06 21:01:15)

Improve dot notation for updating nested objects

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

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

```diff
@@           Coverage Diff           @@
##           master     #729   +/-   ##
=======================================
  Coverage   88.59%   88.59%           
=======================================
  Files          53       53           
  Lines        4541     4541           
  Branches     1054     1054           
=======================================
  Hits         4023     4023           
  Misses        518      518
```


| [Impacted Files](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/729?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/ParseObject.js](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/729/diff?src=pr&el=tree#diff-c3JjL1BhcnNlT2JqZWN0Lmpz) | `89.47% <100%> (ø)` | :arrow_up: |

------

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

dplewis: @flovilmart This test case works for PG also
flovilmart: Well, there is to test also all the other cases: setting a value on ‘key.path’ and increment when it’s not an object stored behind, unset(‘key.path’) etc...

<sub>Sent with <a href="http://githawk.com">GitHawk</a></sub>
flovilmart: I am very worry bad things will happen with that much decoupling but if you beliebe this is ‘needed’ go ahead 

<sub>Sent with <a href="http://githawk.com">GitHawk</a></sub>
dplewis: @flovilmart I did run into two issues with this.

You can't set or update nested documents on new objects.

```
const obj = new TestObject();
obj.set('objectField.number', 20); // Throws internal server error because field doesn't exist
```

If field exist the local data doesn't update until saved

```
const obj = new TestObject({ objectField: { number: 5 } });
await obj.save();
// obj.get('objectField').number is 5
obj.set('objectField.number', 20);
// obj.get('objectField').number is still 5
await obj.save();
// obj.get('objectField').number is now 20
```
flovilmart: Do what you feel is best 

<sub>Sent with <a href="http://githawk.com">GitHawk</a></sub>
dplewis: @flovilmart I resolved both issues I stated on the client side. Can you look this over?

I found an existing bug but it’s not important.
flovilmart: I still have bad feelings about this, but go ahead.

<sub>Sent with <a href="http://githawk.com">GitHawk</a></sub>

Patch:
diff --git a/src/ObjectStateMutations.js b/src/ObjectStateMutations.js
index 4cba356..4827449 100644
--- a/src/ObjectStateMutations.js
+++ b/src/ObjectStateMutations.js
@@ -123,7 +123,18 @@ export function estimateAttributes(serverData: AttributeMap, pendingOps: Array<O
           );
         }
       } else {
-        data[attr] = pendingOps[i][attr].applyTo(data[attr]);
+        if (attr.includes('.')) {
+          // convert a.b.c into { a: { b: { c: value } } }
+          const fields = attr.split('.');
+          const last = fields[fields.length - 1];
+          let object = Object.assign({}, data);
+          for (let i = 0; i < fields.length - 1; i++) {
+            object = object[fields[i]];
+          }
+          object[last] = pendingOps[i][attr].applyTo(object[last]);
+        } else {
+          data[attr] = pendingOps[i][attr].applyTo(data[attr]);
+        }
       }
     }
   }
diff --git a/src/ParseObject.js b/src/ParseObject.js
index e2ce642..faf0b2a 100644
--- a/src/ParseObject.js
+++ b/src/ParseObject.js
@@ -282,8 +282,24 @@ class ParseObject {
     const dirtyObjects = this._getDirtyObjectAttributes();
     const json = {};
     let attr;
+
     for (attr in dirtyObjects) {
-      json[attr] = new SetOp(dirtyObjects[attr]).toJSON();
+      let isDotNotation = false;
+      for (let i = 0; i < pending.length; i += 1) {
+        for (const field in pending[i]) {
+          // Dot notation operations are handled later
+          if (field.includes('.')) {
+            const fieldName = field.split('.')[0];
+            if (fieldName === attr) {
+              isDotNotation = true;
+              break;
+            }
+          }
+        }
+      }
+      if (!isDotNotation) {
+        json[attr] = new SetOp(dirtyObjects[attr]).toJSON();
+      }
     }
     for (attr in pending[0]) {
       json[attr] = pending[0][attr].toJSON();
@@ -582,8 +598,8 @@ class ParseObject {
   /**
    * Sets a hash of model attributes on the object.
    *
-   * <p>You can call it with an object containing keys and values, or with one
-   * key and value.  For example:<pre>
+   * <p>You can call it with an object containing keys and values, with one
+   * key and value, or dot notation.  For example:<pre>
    *   gameTurn.set({
    *     player: player1,
    *     diceRoll: 2
@@ -601,6 +617,8 @@ class ParseObject {
    *
    *   game.set("finished", true);</pre></p>
    *
+   *   game.set("player.score", 10);</pre></p>
+   *
    * @param {String} key The key to set.
    * @param {} value The value to give it.
    * @param {Object} options A set of options for the set.
@@ -661,8 +679,18 @@ class ParseObject {
       }
     }
 
-    // Calculate new values
     const currentAttributes = this.attributes;
+
+    // Only set nested fields if exists
+    const serverData = this._getServerData();
+    if (typeof key === 'string' && key.includes('.')) {
+      const field = key.split('.')[0];
+      if (!serverData[field]) {
+        return this;
+      }
+    }
+
+    // Calculate new values
     const newValues = {};
     for (const attr in newOps) {
       if (newOps[attr] instanceof RelationOp) {
@@ -910,7 +938,7 @@ class ParseObject {
       );
     }
     for (const key in attrs) {
-      if (!(/^[A-Za-z][0-9A-Za-z_]*$/).test(key)) {
+      if (!(/^[A-Za-z][0-9A-Za-z_.]*$/).test(key)) {
         return new ParseError(ParseError.INVALID_KEY_NAME);
       }
     }


Test patch:
diff --git a/integration/test/ParseObjectTest.js b/integration/test/ParseObjectTest.js
index edde7e2..7877a36 100644
--- a/integration/test/ParseObjectTest.js
+++ b/integration/test/ParseObjectTest.js
@@ -229,6 +229,181 @@ describe('Parse Object', () => {
     });
   });
 
+  it('can increment nested fields', async () => {
+    const obj = new TestObject();
+    obj.set('objectField', { number: 5 });
+    assert.equal(obj.get('objectField').number, 5);
+    await obj.save();
+
+    obj.increment('objectField.number', 15);
+    assert.equal(obj.get('objectField').number, 20);
+    await obj.save();
+
+    assert.equal(obj.get('objectField').number, 20);
+
+    const query = new Parse.Query(TestObject);
+    const result = await query.get(obj.id);
+    assert.equal(result.get('objectField').number, 20);
+  });
+
+  it('can increment non existing field', async () => {
+    const obj = new TestObject();
+    obj.set('objectField', { number: 5 });
+    await obj.save();
+
+    obj.increment('objectField.unknown', 15);
+    assert.deepEqual(obj.get('objectField'), {
+      number: 5,
+      unknown: 15,
+    });
+    await obj.save();
+
+    const query = new Parse.Query(TestObject);
+    const result = await query.get(obj.id);
+    assert.equal(result.get('objectField').number, 5);
+    assert.equal(result.get('objectField').unknown, 15);
+  });
+
+  it('can increment nested fields two levels', async () => {
+    const obj = new TestObject();
+    obj.set('objectField', { foo: { bar: 5 } });
+    assert.equal(obj.get('objectField').foo.bar, 5);
+    await obj.save();
+
+    obj.increment('objectField.foo.bar', 15);
+    assert.equal(obj.get('objectField').foo.bar, 20);
+    await obj.save();
+
+    assert.equal(obj.get('objectField').foo.bar, 20);
+
+    const query = new Parse.Query(TestObject);
+    const result = await query.get(obj.id);
+    assert.equal(result.get('objectField').foo.bar, 20);
+  });
+
+  it('can increment nested fields without object', async () => {
+    const obj = new TestObject();
+    obj.set('hello', 'world');
+    await obj.save();
+
+    obj.increment('hello.dot', 15);
+    try  {
+      await obj.save();
+      assert.equal(false, true);
+    } catch(error) {
+      assert.equal(error.message, "Cannot create property 'dot' on string 'world'");
+    }
+  });
+
+  it('can set nested fields', async () => {
+    const obj = new TestObject({ objectField: { number: 5 } });
+    assert.equal(obj.get('objectField').number, 5);
+    await obj.save();
+
+    assert.equal(obj.get('objectField').number, 5);
+    obj.set('objectField.number', 20);
+    assert.equal(obj.get('objectField').number, 20);
+    await obj.save();
+
+    const query = new Parse.Query(TestObject);
+    const result = await query.get(obj.id);
+    assert.equal(result.get('objectField').number, 20);
+  });
+
+  it('can set non existing fields', async () => {
+    const obj = new TestObject();
+    obj.set('objectField', { number: 5 });
+    await obj.save();
+
+    obj.set('objectField.unknown', 20);
+    await obj.save();
+    const query = new Parse.Query(TestObject);
+    const result = await query.get(obj.id);
+    assert.equal(result.get('objectField').number, 5);
+    assert.equal(result.get('objectField').unknown, 20);
+  });
+
+  it('ignore set nested fields on new object', async () => {
+    const obj = new TestObject();
+    obj.set('objectField.number', 5);
+    assert.deepEqual(obj._getPendingOps()[0], {});
+    assert.equal(obj.get('objectField'), undefined);
+
+    await obj.save();
+    assert.equal(obj.get('objectField'), undefined);
+  });
+
+  it('can set nested fields two levels', async () => {
+    const obj = new TestObject({ objectField: { foo: { bar: 5 } } });
+    assert.equal(obj.get('objectField').foo.bar, 5);
+    await obj.save();
+
+    assert.equal(obj.get('objectField').foo.bar, 5);
+    obj.set('objectField.foo.bar', 20);
+    assert.equal(obj.get('objectField').foo.bar, 20);
+    await obj.save();
+
+    const query = new Parse.Query(TestObject);
+    const result = await query.get(obj.id);
+    assert.equal(result.get('objectField').foo.bar, 20);
+  });
+
+  it('can unset nested fields', async () => {
+    const obj = new TestObject({
+      objectField: {
+        number: 5,
+        string: 'hello',
+      }
+    });
+    await obj.save();
+
+    obj.unset('objectField.number');
+    assert.equal(obj.get('objectField').number, undefined);
+    assert.equal(obj.get('objectField').string, 'hello');
+    await obj.save();
+
+    const query = new Parse.Query(TestObject);
+    const result = await query.get(obj.id);
+    assert.equal(result.get('objectField').number, undefined);
+    assert.equal(result.get('objectField').string, 'hello');
+  });
+
+  it('can unset nested fields two levels', async () => {
+    const obj = new TestObject({
+      objectField: {
+        foo: {
+          bar: 5,
+        },
+        string: 'hello',
+      }
+    });
+    await obj.save();
+
+    obj.unset('objectField.foo.bar');
+    assert.equal(obj.get('objectField').foo.bar, undefined);
+    assert.equal(obj.get('objectField').string, 'hello');
+    await obj.save();
+
+    const query = new Parse.Query(TestObject);
+    const result = await query.get(obj.id);
+    assert.equal(result.get('objectField').foo.bar, undefined);
+    assert.equal(result.get('objectField').string, 'hello');
+  });
+
+  it('can unset non existing fields', async () => {
+    const obj = new TestObject();
+    obj.set('objectField', { number: 5 });
+    await obj.save();
+
+    obj.unset('objectField.unknown');
+    await obj.save();
+
+    const query = new Parse.Query(TestObject);
+    const result = await query.get(obj.id);
+    assert.equal(result.get('objectField').number, 5);
+    assert.equal(result.get('objectField').unknown, undefined);
+  });
+
   it('can set keys to null', (done) => {
     const obj = new TestObject();
     obj.set('foo', null);
diff --git a/src/__tests__/ObjectStateMutations-test.js b/src/__tests__/ObjectStateMutations-test.js
index e95f181..9f972d2 100644
--- a/src/__tests__/ObjectStateMutations-test.js
+++ b/src/__tests__/ObjectStateMutations-test.js
@@ -119,6 +119,22 @@ describe('ObjectStateMutations', () => {
     expect(attributes.likes.key).toBe('likes');
   });
 
+  it('can estimate attributes for nested documents', () => {
+    const serverData = { objectField: { counter: 10 } };
+    let pendingOps = [{ 'objectField.counter': new ParseOps.IncrementOp(2) }];
+    expect(ObjectStateMutations.estimateAttributes(serverData, pendingOps, 'someClass', 'someId')).toEqual({
+      objectField: {
+        counter: 12
+      },
+    });
+    pendingOps = [{ 'objectField.counter': new ParseOps.SetOp(20) }];
+    expect(ObjectStateMutations.estimateAttributes(serverData, pendingOps, 'someClass', 'someId')).toEqual({
+      objectField: {
+        counter: 20
+      },
+    });
+  });
+
   it('can commit changes from the server', () => {
     const serverData = {};
     const objectCache = {};
diff --git a/src/__tests__/ParseObject-test.js b/src/__tests__/ParseObject-test.js
index 8b02401..6c95b17 100644
--- a/src/__tests__/ParseObject-test.js
+++ b/src/__tests__/ParseObject-test.js
@@ -474,6 +474,45 @@ describe('ParseObject', () => {
     expect(o2.attributes).toEqual({ age: 41 });
   });
 
+  it('can set nested field', () => {
+    const o = new ParseObject('Person');
+    o._finishFetch({
+      objectId: 'setNested',
+      objectField: {
+        number: 5
+      },
+      otherField: {},
+    });
+
+    expect(o.attributes).toEqual({
+      objectField: { number: 5 },
+      otherField: {},
+    });
+    o.set('otherField', { hello: 'world' });
+    o.set('objectField.number', 20);
+
+    expect(o.attributes).toEqual({
+      objectField: { number: 20 },
+      otherField: { hello: 'world' },
+    });
+    expect(o.op('objectField.number') instanceof SetOp).toBe(true);
+    expect(o.dirtyKeys()).toEqual(['otherField', 'objectField.number', 'objectField']);
+    expect(o._getSaveJSON()).toEqual({
+      'objectField.number': 20,
+      otherField: { hello: 'world' },
+    });
+  });
+
+  it('ignore set nested field on new object', () => {
+    const o = new ParseObject('Person');
+    o.set('objectField.number', 20);
+
+    expect(o.attributes).toEqual({});
+    expect(o.op('objectField.number') instanceof SetOp).toBe(false);
+    expect(o.dirtyKeys()).toEqual([]);
+    expect(o._getSaveJSON()).toEqual({});
+  });
+
   it('can add elements to an array field', () => {
     const o = new ParseObject('Schedule');
     o.add('available', 'Monday');
@@ -650,6 +689,10 @@ describe('ParseObject', () => {
     expect(o.validate({
       noProblem: 'here'
     })).toBe(false);
+
+    expect(o.validate({
+      'dot.field': 'here'
+    })).toBe(false);
   });
 
   it('validates attributes on set()', () => {