From a53c94d33f8b9ffd8b1a95c6066a0253213a4fe3 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 30 Mar 2015 14:43:26 -0500 Subject: [PATCH 1/2] Closes softlayer/sl-ember-store#122 Closes softlayer/sl-ember-store#122 --- README.md | 17 +++-- addon/adapters/ajax.js | 47 ++++++++----- addon/model.js | 7 +- tests/unit/adapters/ajax-test.js | 108 ++++++++++++++++++++++++++---- tests/unit/model-test.js | 111 +++++++++++++++++++++++++------ 5 files changed, 233 insertions(+), 57 deletions(-) diff --git a/README.md b/README.md index 4734622..9a8fa36 100755 --- a/README.md +++ b/README.md @@ -103,7 +103,7 @@ Models have `ajax` specified as default, so you don't need to do this unless you sl-ember-store adapters always return [Ember Promise Proxies](http://emberjs.com/api/classes/Ember.PromiseProxyMixin.html). If you request a single object then you will get an `Ember.ObjectProxy` with the promise proxy mixin applied. Requests for -Multiple records will return an `Ember.ArrayProxy` with the promise proxiy mixin applied. +Multiple records will return an `Ember.ArrayProxy` with the promise proxy mixin applied. ### Ajax adapter The `ajax` adapter uses [ic-ajax](https://github.com/instructure/ic-ajax) to make `xhr` requests to your remote api. @@ -135,25 +135,34 @@ Foo.reopenClass({ return response.result; } }, - post: '/superFooPost' + post: '/superFooPost', + put: '/superFooPut', + delete: 'superFooDelete' }, 'boringFoo': { url: '/boringFoo', serializer: someSerializer }, - 'superBoringFoo': '/superBoringFoo', + 'superBoringFoo': '/superBoringFoo' } }); export default Foo; ``` -In the example above, the `superFoo:post` endpoint will use the default serializer. +In the example above, the `superFoo:post` endpoint will use the default serializer. Please note that specific endpoint +actions ( get, post, put, and delete ) MUST be in lowercase. All HTTP verbs on the `boringfoo` endpoint will use the `someSerializer` function as their serializer. All HTTP verbs on the `superBoringFoo` endpoint will use the default serializer. Models should always have a `url` specified. Further urls can be specified in the `endpoints` object. Urls and Serializers can be specified on a per endpoint/action basis and will default to the top level url and serializer. +The creation of a new record, one in which an id has not been assigned by your API, will result in a POST action. +Updating a record, one in which an id has been assigned by your API, will result in a PUT action. The model's save() +method is responsible for both the creation and update of a record. In both cases, the request body payload wil be passed through. +The difference in whether a POST or a PUT is sent is dependent on whether the record already has an id assigned to it from the API. +The deletion of a record requires that a record has an id already assigned to it from the API. + If you find you need an `inflection` service to support your api, we recommend you use [Ember-Inflector](https://github.com/stefanpenner/ember-inflector). You can then use `Ember.Inflector` in your serializers and models. diff --git a/addon/adapters/ajax.js b/addon/adapters/ajax.js index effeccc..c8be62d 100755 --- a/addon/adapters/ajax.js +++ b/addon/adapters/ajax.js @@ -91,21 +91,24 @@ export default Adapter.extend({ * Delete record * * @function deleteRecord - * @param {string} url - The URL to send the DELETE command to + * @param {string} url - The URL to send the DELETE command to * @param {integer} id - The model record's ID * @throws {Ember.assert} * @returns {Ember.RSVP} Promise */ deleteRecord: function( url, id ) { - var queryObj = { - url : url, + var _self = this, + queryObj; + + Ember.assert( 'A url is required to delete a model', url ); + Ember.assert( 'An id property is required to delete a model', id ); + + queryObj = { + url : url + '/' + id, type : 'DELETE', data : JSON.stringify({ id: id }), context : this - }, - _self = this; - - Ember.assert( 'A url is required to delete a model', url ); + }; this.runPreQueryHooks( queryObj ); @@ -115,26 +118,36 @@ export default Adapter.extend({ } , 'sl-ember-store.ajaxAdapter:deleteRecord' ); }, - /** + /** * Save record * * @function save - * @param {string} url - The URL to send the POST command to + * @param {string} url - The URL to send the POST/PUT command to (depends on whether an id exists or not). * @param {object} content - Data to save * @throws {Ember.assert} * @returns {Ember.RSVP} Promise */ save: function( url, content ) { - var promise, - queryObj = { - url : url, - type : 'POST', - data : JSON.stringify( content ), - context : this - }, - _self = this; + var _self = this, + promise, + queryObj; Ember.assert( 'A url property is required to save a model', url ); + Ember.assert( + 'save() expects parameter to be an Object', + 'object' === typeof content && !Array.isArray( content ) + ); + + if ( Ember.get( content, 'id' ) ) { + url = url + '/' + Ember.get( content, 'id' ); + } + + queryObj = { + url : url, + type : ( content.id && !Ember.isEmpty( content.id ) ) ? 'PUT' : 'POST', + data : JSON.stringify( content ), + context : this + }; this.runPreQueryHooks( queryObj ); diff --git a/addon/model.js b/addon/model.js index a37f28c..ac99a24 100755 --- a/addon/model.js +++ b/addon/model.js @@ -17,11 +17,13 @@ var Model = Ember.ObjectProxy.extend({ */ save: function( options ) { var data, + type, endpoint; options = options || {}; - endpoint = this.constructor.getUrlForEndpointAction( options.endpoint, 'post' ); data = this.get( 'content' ); + type = ( !Ember.isEmpty( data.id ) ) ? 'put' : 'post'; + endpoint = this.constructor.getUrlForEndpointAction( options.endpoint, type ); Ember.assert( 'Endpoint must be configured on ' + this.toString() + ' before calling save.', endpoint ); @@ -47,8 +49,9 @@ var Model = Ember.ObjectProxy.extend({ endpoint = this.constructor.getUrlForEndpointAction( options.endpoint, 'delete' ); Ember.assert( 'Enpoint must be configured on ' + this.toString() + ' before calling deleteRecord.', endpoint ); + Ember.assert( 'deleteRecord() requires an id', this.get( 'id' ) ); - return this.container.lookup( 'adapter:'+this.constructor.adapter ).deleteRecord( endpoint, this.get( 'id' ) ) + return this.container.lookup( 'adapter:' + this.constructor.adapter ).deleteRecord( endpoint, this.get( 'id' ) ) .then( Ember.run.bind( this, function() { Ember.run( this, 'destroy' ); }), null, 'sl-ember-store.model:deleteRecord' ); diff --git a/tests/unit/adapters/ajax-test.js b/tests/unit/adapters/ajax-test.js index 59a0ef6..e14a571 100755 --- a/tests/unit/adapters/ajax-test.js +++ b/tests/unit/adapters/ajax-test.js @@ -175,32 +175,114 @@ asyncTest( 'find should not throw error if response is empty', function( assert }); }); -test( 'save', function( assert ){ - var foo = Foo.create({ test: 'foo', 'bar': { id: 1, quiz: 'bar' } }); +test( 'save() should send PUT since an id exists', function( assert ){ + var foo = Foo.create({ id: 3 }); + response = ajaxAdapter.save( '/foo', foo ); + assert.ok( requestSpy.calledOnce, 'should call icAjax request once' ); + assert.equal( requestSpy.args[0][0].url, '/foo/3', 'should call icAjax with correct url'); + assert.equal( requestSpy.args[0][0].type, 'PUT', 'should call icAjax with PUT method'); + assert.equal( typeof requestSpy.args[0][0].data, 'string', 'icAjax should return a string'); +}); + +test( 'save() should send POST since NO id exists', function( assert ){ + var foo = Foo.create({ label: 'My name' }); response = ajaxAdapter.save( '/foo', foo ); assert.ok( requestSpy.calledOnce, 'should call icAjax request once' ); assert.equal( requestSpy.args[0][0].url, '/foo', 'should call icAjax with correct url'); - assert.equal( requestSpy.args[0][0].type, 'POST', 'should call icAjax with correct method'); + assert.equal( requestSpy.args[0][0].type, 'POST', 'should call icAjax with POST method'); assert.equal( typeof requestSpy.args[0][0].data, 'string', 'icAjax should return a string'); }); -test( 'save, should call $.ajax with the correct arguments', function( assert ){ - var foo = Foo.create({ test: 'foo', 'bar': { id: 1, quiz: 'bar' } }); +test( 'save() should call $.ajax with the correct arguments', function( assert ){ + var foo = Foo.create({ id: 3 }); response = ajaxAdapter.save( '/foo', foo ); assert.ok( requestSpy.calledOnce, 'request called once' ); - assert.equal( requestSpy.args[0][0].url, '/foo' ); - assert.equal( requestSpy.args[0][0].type, 'POST' ); - assert.equal( typeof requestSpy.args[0][0].data, 'string' ); + assert.equal( typeof requestSpy.args[0][0].data, 'string' ); // update to check json_decode also in 184 and 193 assert.ok( response.then, 'response is a promise' ); }); -test( 'delete, should call icAjax.request once', function( assert ){ - var foo = Foo.create({ id: 1, test: 'foo', 'bar': { id: 1, quiz: 'bar' } }); - response = ajaxAdapter.deleteRecord( '/foo', 1 ); +test( 'save() requires an Object to be provided as the second parameter', function( assert ) { + + // Empty parameter + + var assertionThrown = false; + + try { + ajaxAdapter.save(); + } catch( error ) { + assertionThrown = true; + } + + assert.ok( assertionThrown, 'Parameter was empty' ); + + // Number parameter + + assertionThrown = false; + + try { + ajaxAdapter.save( 'test/', 4 ); + } catch( error ) { + assertionThrown = true; + } + + assert.ok( assertionThrown, 'Parameter was a Number' ); + + // Array Parameter + + assertionThrown = false; + + try { + ajaxAdapter.save( 'test/', [] ); + } catch( error ) { + assertionThrown = true; + } + + assert.ok( assertionThrown, 'Parameter was an Array' ); + + // Function + + assertionThrown = false; + + try { + ajaxAdapter.save( 'test/', function(){} ); + } catch( error ) { + assertionThrown = true; + } + + assert.ok( assertionThrown, 'Parameter was a Function' ); + + // String Parameter + + assertionThrown = false; + + try { + ajaxAdapter.save( 'test/', 'test' ); + } catch( error ) { + assertionThrown = true; + } + + assert.ok( assertionThrown, 'Parameter was a String' ); + + // Object Parameter + + assertionThrown = false; + + try { + ajaxAdapter.save( 'test/', {} ); + } catch( error ) { + assertionThrown = true; + } + + assert.ok( !assertionThrown, 'Parameter was an Object' ); +}); + +test( 'deleteRecord() should call icAjax.request once', function( assert ){ + var foo = Foo.create({ id: 3 }); + response = ajaxAdapter.deleteRecord( '/foo', 3 ); assert.ok( requestSpy.calledOnce ); - assert.equal( requestSpy.args[0][0].url, '/foo', 'should call icAjax with correct url'); + assert.equal( requestSpy.args[0][0].url, '/foo/3', 'should call icAjax with correct url'); assert.equal( requestSpy.args[0][0].type, 'DELETE', 'should call icAjax with correct url'); assert.equal( typeof requestSpy.args[0][0].data, 'string', 'icAjax should return a string'); assert.ok( response.then, 'response is a proxy' ); -}); +}); \ No newline at end of file diff --git a/tests/unit/model-test.js b/tests/unit/model-test.js index fdf983b..f66030d 100755 --- a/tests/unit/model-test.js +++ b/tests/unit/model-test.js @@ -57,12 +57,17 @@ module( 'Unit - sl-ember-store/model', { url : '/bar', endpoints : { default: { - post: '/barUpdate', + post: '/barCreate', + put: '/barUpdate', delete: '/barDelete', serializer: serializer1 }, car: { post: { + url: '/carCreate', + serializer: serializer2, + }, + put: { url: '/carUpdate', serializer: serializer2, }, @@ -102,8 +107,7 @@ module( 'Unit - sl-ember-store/model', { foo = Foo.create({ content: { - test: 'foo', - 'bar': { id: 1, quiz: 'bar' }, + test: 'foo' }, container: container }); @@ -111,8 +115,7 @@ module( 'Unit - sl-ember-store/model', { bar = Bar.create({ content: { - test: 'bar', - 'car': { id: 1, quiz: 'car' }, + test: 'bar' }, container: container }); @@ -122,15 +125,13 @@ module( 'Unit - sl-ember-store/model', { adapter.deleteRecord.reset(); foo = Foo.create({ content: { - test: 'foo', - 'bar': { id: 1, quiz: 'bar' }, + test: 'foo' }, container: container }); bar = Bar.create({ content: { - test: 'bar', - 'car': { id: 1, quiz: 'car' }, + test: 'bar' }, container: container }); @@ -141,8 +142,12 @@ test( 'getUrlForEndpointAction:should return /bar for ( null, `get` )', function assert.equal( Bar.getUrlForEndpointAction( null, 'get' ), '/bar' ); }); -test( 'getUrlForEndpointAction:should return /barUpdate for ( null, `post` )', function( assert ) { - assert.equal( Bar.getUrlForEndpointAction( null, 'post' ), '/barUpdate' ); +test( 'getUrlForEndpointAction:should return /barCreate for ( null, `post` )', function( assert ) { + assert.equal( Bar.getUrlForEndpointAction( null, 'post' ), '/barCreate' ); +}); + +test( 'getUrlForEndpointAction:should return /barUpdate for ( null, `put` )', function( assert ) { + assert.equal( Bar.getUrlForEndpointAction( null, 'put' ), '/barUpdate' ); }); test( 'getUrlForEndpointAction:should return /barDelete for ( null, `delete` )', function( assert ) { @@ -153,8 +158,12 @@ test( 'getUrlForEndpointAction:should return /bar for ( `default`, `get` )', fun assert.equal( Bar.getUrlForEndpointAction( 'default', 'get' ), '/bar' ); }); -test( 'getUrlForEndpointAction:should return /barUpdate for ( `default`, `post` )', function( assert ) { - assert.equal( Bar.getUrlForEndpointAction( 'default', 'post' ), '/barUpdate' ); +test( 'getUrlForEndpointAction:should return /barCreate for ( `default`, `post` )', function( assert ) { + assert.equal( Bar.getUrlForEndpointAction( 'default', 'post' ), '/barCreate' ); +}); + +test( 'getUrlForEndpointAction:should return /barUpdate for ( `default`, `put` )', function( assert ) { + assert.equal( Bar.getUrlForEndpointAction( 'default', 'put' ), '/barUpdate' ); }); test( 'getUrlForEndpointAction:should return /barDelete for ( `default`, `delete` )', function( assert ) { @@ -165,8 +174,12 @@ test( 'getUrlForEndpointAction:should return /bar for ( `car`, `get` )', functio assert.equal( Bar.getUrlForEndpointAction( 'car', 'get' ), '/bar' ); }); -test( 'getUrlForEndpointAction:should return /carUpdate for ( `car`, `post` )', function( assert ) { - assert.equal( Bar.getUrlForEndpointAction( 'car', 'post' ), '/carUpdate' ); +test( 'getUrlForEndpointAction:should return /carCreate for ( `car`, `post` )', function( assert ) { + assert.equal( Bar.getUrlForEndpointAction( 'car', 'post' ), '/carCreate' ); +}); + +test( 'getUrlForEndpointAction:should return /carUpdate for ( `car`, `put` )', function( assert ) { + assert.equal( Bar.getUrlForEndpointAction( 'car', 'put' ), '/carUpdate' ); }); test( 'getUrlForEndpointAction:should return /carDelete for ( `car`, `delete` )', function( assert ) { @@ -201,10 +214,11 @@ test( 'save-default:should update its content with fooResponse', function( asser test( 'save-endpoint:should call adapter.save with correct arguments', function( assert ) { assert.expect(2); bar.save().then( function() { - assert.equal( adapter.save.args[0][0], '/barUpdate' ); + assert.equal( adapter.save.args[0][0], '/barCreate' ); assert.equal( adapter.save.args[0][1].test, 'bar' ); }); }); + test( 'save-endpoint:should update its content with fooResponse', function( assert ) { assert.expect(1); bar.save().then( function() { @@ -212,12 +226,26 @@ test( 'save-endpoint:should update its content with fooResponse', function( asse }); }); -test( 'save-endpoint:car: should call adapter.save with correct arguments', function( assert ) { +test( 'save-endpoint:car: should call adapter.save with correct arguments - NO id (POST)', function( assert ) { + assert.expect(2); + bar = Bar.create({ + content: { + test: 'bar' + }, + container: container + }); + bar.save({endpoint:'car'}).then( function() { + assert.equal( adapter.save.args[0][0], '/carCreate' ); + assert.equal( adapter.save.args[0][1].test, 'bar' ); + }); +}); + +test( 'save-endpoint:car: should call adapter.save with correct arguments - WITH id (PUT)', function( assert ) { assert.expect(2); bar = Bar.create({ content: { - test: 'bar', - 'car': { id: 1, quiz: 'car' }, + id : 3, + test: 'bar' }, container: container }); @@ -230,8 +258,7 @@ test( 'save-endpoint:car: should call adapter.save with correct arguments', func test( 'save-endpoint:car: should update its content with fooResponse', function( assert ) { bar = Bar.create({ content: { - test: 'bar', - 'car': { id: 1, quiz: 'car' }, + test: 'bar' }, container: container }); @@ -241,6 +268,13 @@ test( 'save-endpoint:car: should update its content with fooResponse', function( }); test( 'delete: should call adapter.deleteRecord with correct arguments', function( assert ) { + var foo = Foo.create({ + id: 3, + content: { + test: 'foo' + }, + container: container + }); assert.expect(1); foo.deleteRecord().then( function() { assert.ok( adapter.deleteRecord.calledWith( '/foo' ) ); @@ -248,6 +282,13 @@ test( 'delete: should call adapter.deleteRecord with correct arguments', functio }); test( 'delete: should destroy foo', function( assert ) { + var foo = Foo.create({ + id: 3, + content: { + test: 'foo' + }, + container: container + }); assert.expect(1); foo.deleteRecord().then( function() { assert.ok( foo.isDestroyed ); @@ -256,6 +297,13 @@ test( 'delete: should destroy foo', function( assert ) { }); test( 'delete-endpoint: should call adapter.delete with correct arguments', function( assert ) { + var bar = Bar.create({ + id: 3, + content: { + test: 'bar' + }, + container: container + }); assert.expect(1); bar.deleteRecord().then( function() { assert.ok( adapter.deleteRecord.calledWith( '/barDelete' ) ); @@ -263,6 +311,13 @@ test( 'delete-endpoint: should call adapter.delete with correct arguments', func }); test( 'delete-endpoint: should destroy bar', function( assert ) { + var bar = Bar.create({ + id: 3, + content: { + test: 'bar' + }, + container: container + }); assert.expect(1); bar.deleteRecord().then( function() { assert.ok( bar.isDestroyed ); @@ -270,6 +325,13 @@ test( 'delete-endpoint: should destroy bar', function( assert ) { }); test( 'delete-endpoint:car: should call adapter.delete with correct arguments', function( assert ) { + var bar = Bar.create({ + id: 3, + content: { + test: 'bar' + }, + container: container + }); assert.expect(1); bar.deleteRecord({endpoint:'car'}).then( function() { assert.ok( adapter.deleteRecord.calledWith( '/carDelete' ) ); @@ -277,6 +339,13 @@ test( 'delete-endpoint:car: should call adapter.delete with correct arguments', }); test( 'delete-endpoint:car: should destroy bar', function( assert ) { + var bar = Bar.create({ + id: 3, + content: { + test: 'bar' + }, + container: container + }); assert.expect(1); bar.deleteRecord({endpoint:'car'}).then( function() { assert.ok( bar.isDestroyed ); From fad25855fe6380e48e9c5ebcc51ab9d23a9b2744 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 27 Apr 2015 14:36:28 -0500 Subject: [PATCH 2/2] Fixed two typos in README file and modified one test. --- README.md | 4 ++-- tests/unit/adapters/ajax-test.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 9a8fa36..38c9e82 100755 --- a/README.md +++ b/README.md @@ -157,9 +157,9 @@ All HTTP verbs on the `superBoringFoo` endpoint will use the default serializer. Models should always have a `url` specified. Further urls can be specified in the `endpoints` object. Urls and Serializers can be specified on a per endpoint/action basis and will default to the top level url and serializer. -The creation of a new record, one in which an id has not been assigned by your API, will result in a POST action. +The creation of a new record, one in which an id has not yet been assigned by your API, will result in a POST action. Updating a record, one in which an id has been assigned by your API, will result in a PUT action. The model's save() -method is responsible for both the creation and update of a record. In both cases, the request body payload wil be passed through. +method is responsible for both the creation and update of a record. In both cases, the request body payload will be passed through. The difference in whether a POST or a PUT is sent is dependent on whether the record already has an id assigned to it from the API. The deletion of a record requires that a record has an id already assigned to it from the API. diff --git a/tests/unit/adapters/ajax-test.js b/tests/unit/adapters/ajax-test.js index e14a571..7c7e375 100755 --- a/tests/unit/adapters/ajax-test.js +++ b/tests/unit/adapters/ajax-test.js @@ -208,7 +208,7 @@ test( 'save() requires an Object to be provided as the second parameter', functi var assertionThrown = false; try { - ajaxAdapter.save(); + ajaxAdapter.save( 'test/', null ); } catch( error ) { assertionThrown = true; }