Repository: js-data/js-data
Pull request #446 (2017-02-20 21:18:24)

fix(Collection): respect noValidate override passed in to find/findAll when adding records
In our project we've come into a problem to skip validation of records returned by adapter. Eg. all records received from API should be treated as valid records. Passing `noValidate: true` to `find/findAll` was not respected cause property setter gets `noValidate` property value from `Settable`. `Settable` instance get's this property from schema definition, which is `false` by default. Hence no matter what value for `noValidate` we pass to `find/findAll` it does not affect validation.


codecov-io: # [Codecov](https://codecov.io/gh/js-data/js-data/pull/446?src=pr&el=h1) Report
> Merging [#446](https://codecov.io/gh/js-data/js-data/pull/446?src=pr&el=desc) into [master](https://codecov.io/gh/js-data/js-data/commit/edeceebc6fbde37e954369a3ba0cb30c72a842d3?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.


```diff
@@            Coverage Diff             @@
##           master     #446      +/-   ##
==========================================
+ Coverage   95.95%   95.96%   +<.01%     
==========================================
  Files          21       21              
  Lines        2498     2504       +6     
==========================================
+ Hits         2397     2403       +6     
  Misses        101      101
```


| [Impacted Files](https://codecov.io/gh/js-data/js-data/pull/446?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/Collection.js](https://codecov.io/gh/js-data/js-data/compare/edeceebc6fbde37e954369a3ba0cb30c72a842d3...de914adc8761f7f168c68bdb3912520f1c28a4d2?src=pr&el=tree#diff-c3JjL0NvbGxlY3Rpb24uanM=) | `100% <100%> (ø)` | :white_check_mark: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/js-data/js-data/pull/446?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/js-data/js-data/pull/446?src=pr&el=footer). Last update [edeceeb...de914ad](https://codecov.io/gh/js-data/js-data/compare/edeceebc6fbde37e954369a3ba0cb30c72a842d3...de914adc8761f7f168c68bdb3912520f1c28a4d2?el=footer&src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Patch:
diff --git a/src/Collection.js b/src/Collection.js
index d9f7cbe..1bcf4bc 100644
--- a/src/Collection.js
+++ b/src/Collection.js
@@ -212,6 +212,7 @@ export default Component.extend({
    * @param {(Object|Object[]|Record|Record[])} data The record or records to insert.
    * @param {Object} [opts] Configuration options.
    * @param {boolean} [opts.commitOnMerge=true] See {@link Collection#commitOnMerge}.
+   * @param {boolean} [opts.noValidate] See {@link Mapper#noValidate}.
    * @param {string} [opts.onConflict] See {@link Collection#onConflict}.
    * @returns {(Object|Object[]|Record|Record[])} The added record or records.
    */
@@ -253,6 +254,12 @@ export default Component.extend({
         // Here, the currently visited record corresponds to a record already
         // in the collection, so we need to merge them
         const onConflict = opts.onConflict || this.onConflict
+        let restoreNoValidate
+        // `noValidate` might have been overriden in find/findAll. respect that
+        if (opts.noValidate !== undefined) {
+          restoreNoValidate = existing._set.bind(existing, 'noValidate', existing._get('noValidate'))
+          existing._set('noValidate', opts.noValidate)
+        }
         if (onConflict === 'merge') {
           utils.deepMixIn(existing, record)
         } else if (onConflict === 'replace') {
@@ -263,8 +270,12 @@ export default Component.extend({
           })
           existing.set(record)
         } else {
+          // restore prev `noValidate` before throwing if it was overriden for the record
+          utils.isFunction(restoreNoValidate) && restoreNoValidate()
           throw utils.err(`${DOMAIN}#add`, 'opts.onConflict')(400, 'one of (merge, replace)', onConflict, true)
         }
+        // restore prev `noValidate` value if it was overriden for the record
+        utils.isFunction(restoreNoValidate) && restoreNoValidate()
         record = existing
         if (opts.commitOnMerge && utils.isFunction(record.commit)) {
           record.commit()


Test patch:
diff --git a/test/unit/collection/add.test.js b/test/unit/collection/add.test.js
index 57f2d3e..dcad1f9 100644
--- a/test/unit/collection/add.test.js
+++ b/test/unit/collection/add.test.js
@@ -115,6 +115,27 @@ describe('Collection#add', function () {
       collection.add({ id: 1 }, { onConflict: 'invalid_choice' })
     }, Error, `[Collection#add:opts.onConflict] expected: one of (merge, replace), found: invalid_choice\nhttp://www.js-data.io/v3.0/docs/errors#400`)
   })
+  it('should respect opts.noValidate', function () {
+    const mapper = new JSData.Mapper({
+      name: 'user',
+      noValidate: false,
+      schema: {
+        type: 'object',
+        properties: {
+          id: { type: 'string' },
+          name: { type: 'string' }
+        }
+      }
+    })
+    const collection = new JSData.Collection({ mapper })
+    const userData = { id: Math.random().toString(), name: Math.random().toString() }
+    const user = collection.add(userData)
+    assert.doesNotThrow(() => {
+      collection.add(Object.assign({}, userData, { name: null }), { noValidate: true })
+    })
+    // original noValidate prop value should be restored
+    assert.equal(user._get('noValidate'), false)
+  })
   it('should required an argument', function () {
     const collection = new JSData.Collection()
     TYPES_EXCEPT_OBJECT_OR_ARRAY.forEach((value) => {