Merge pull request #22 from richmolj/master

Improve error handling

Author: richmolj

src/util/validation-errors.ts

@@ -1,4 +1,5 @@
 import Model from '../model';
+import { camelize } from './string'
 
 export default class ValidationErrors {
   model: Model;
@@ -32,7 +33,13 @@ export default class ValidationErrors {
   }
 
   private _processResource(errorsAccumulator: object, meta: Object) {
-    errorsAccumulator[meta['attribute']] = meta['message'];
+    let attribute = meta['attribute']
+
+    if (this.model.klass.camelizeKeys) {
+      attribute = camelize(attribute)
+    }
+
+    errorsAccumulator[attribute] = meta['message'];
   }
 
   private _processRelationship(model: Model, meta: Object) {
@@ -48,8 +55,18 @@ export default class ValidationErrors {
     } else {
       let relatedAccumulator = {}
       this._processResource(relatedAccumulator, meta);
-      relatedObject.errors = relatedAccumulator
-    }
 
+      // make sure to assign a new error object, instead of mutating
+      // the existing one, otherwise js frameworks with object tracking
+      // won't be able to keep up. Validate vue.js when changing this code:
+      let newErrs = {}
+      Object.keys(relatedObject.errors).forEach((key) => {
+        newErrs[key] = relatedObject.errors[key]
+      });
+      Object.keys(relatedAccumulator).forEach((key) => {
+        newErrs[key] = relatedAccumulator[key]
+      });
+      relatedObject.errors = newErrs
+    }
   }
 }

test/integration/validations-test.ts

@@ -2,8 +2,6 @@ import { expect, sinon, fetchMock } from '../test-helper';
 import { Author, Book, Genre } from '../fixtures';
 import uuid from '../../src/util/uuid';
 
-let serverResponse;
-
 const resetMocks = function() {
   fetchMock.restore();
 
@@ -61,15 +59,31 @@ const resetMocks = function() {
                 }
               }
             }
+          },
+          {
+            code: 'unprocessable_entity',
+            status: '422',
+            title: 'Validation Error',
+            detail: 'base some error',
+            meta: {
+              relationship: {
+                name: 'books',
+                type: 'books',
+                ['temp-id']: 'abc1',
+                relationship: {
+                  name: 'genre',
+                  type: 'genres',
+                  id: '1',
+                  attribute: 'base',
+                  message: 'some error'
+                }
+              }
+            }
           }
         ]
       }
     }
   });
-
-  fetchMock.post('http://example.com/api/v1/people', function(url, payload) {
-    return serverResponse;
-  });
 }
 
 let instance;
@@ -97,19 +111,50 @@ describe('validations', function() {
     uuid.generate['restore']();
   });
 
-  // todo on next save, remove errs
   it('applies errors to the instance', function(done) {
     instance.save({ with: { books: 'genre' }}).then((success) => {
       expect(instance.isPersisted()).to.eq(false);
       expect(success).to.eq(false);
       expect(instance.errors).to.deep.equal({
-        first_name: 'cannot be blank',
-        last_name: 'cannot be blank'
+        firstName: 'cannot be blank',
+        lastName: 'cannot be blank'
       });
       done();
     });
   });
 
+  describe('when camelizeKeys is false', function() {
+    beforeEach(function() {
+      instance.klass.camelizeKeys = false
+    });
+
+    afterEach(function() {
+      instance.klass.camelizeKeys = true
+    });
+
+    it('does not camelize the error keys', function() {
+      instance.save({ with: { books: 'genre' }}).then((success) => {
+        expect(instance.errors).to.deep.equal({
+          first_name: 'cannot be blank',
+          last_name: 'cannot be blank'
+        });
+      });
+    });
+  });
+
+  it('clears errors on save', function(done) {
+    fetchMock.restore()
+    fetchMock.mock({
+      matcher: '*',
+      response: { data: { id: '1', type: 'employees'} }
+    });
+    instance.errors = { foo: 'bar' }
+    instance.save().then(() => {
+      expect(instance.errors).to.deep.eq({})
+      done()
+    });
+  });
+
   it('instantiates a new error object instance after save', function(done) {
     let originalErrors = instance.errors = {foo: 'bar'};
     let result = instance.save({ with: { books: 'genre' }});
@@ -151,8 +196,11 @@ describe('validations', function() {
     instance.save({ with: { books: 'genre' }}).then((success) => {
       expect(instance.isPersisted()).to.eq(false);
       expect(success).to.eq(false);
+
+      // note we're validating multiple properties
       expect(instance.books[0].genre.errors).to.deep.equal({
         name: 'cannot be blank',
+        base: 'some error'
       });
       done();
     });