New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle some create and update errors in the REST adapter. #476

Merged
merged 1 commit into from Nov 20, 2012

Conversation

Projects
None yet
8 participants
@pivotal-medici
Contributor

pivotal-medici commented Nov 14, 2012

  • 422 Unprocessable Entity

    Transitions the record to the invalid state. Assumes
    the server has responded with {errors: {...}} and
    sets them on the record.

  • All other status codes

    Transitions the record to the error state.

This is limited to only single, non-bulk creates/updates.

@pivotal-medici

This comment has been minimized.

Show comment
Hide comment
@pivotal-medici

pivotal-medici Nov 14, 2012

Contributor

Not sure why Travis failed there. The whole test suite works for us in the browser and from the command line.

Contributor

pivotal-medici commented Nov 14, 2012

Not sure why Travis failed there. The whole test suite works for us in the browser and from the command line.

@mspisars

This comment has been minimized.

Show comment
Hide comment
@mspisars

mspisars Nov 15, 2012

Contributor

It is actually failing jshint because of another commit: 90c253d
https://github.com/emberjs/data/blob/master/packages/ember-data/lib/system/serializer.js#L601
key is not defined for both the _addBelongsTo and _addHasMany

Contributor

mspisars commented Nov 15, 2012

It is actually failing jshint because of another commit: 90c253d
https://github.com/emberjs/data/blob/master/packages/ember-data/lib/system/serializer.js#L601
key is not defined for both the _addBelongsTo and _addHasMany

@wagenet

This comment has been minimized.

Show comment
Hide comment
@wagenet

wagenet Nov 15, 2012

Member

This seems fine but I think we were planning for something more comprehensive to do error handling. Nevertheless, this might be ok in the short term. @wycats?

Also, the JSHint failure has been fixed.

Member

wagenet commented Nov 15, 2012

This seems fine but I think we were planning for something more comprehensive to do error handling. Nevertheless, this might be ok in the short term. @wycats?

Also, the JSHint failure has been fixed.

@wycats

This comment has been minimized.

Show comment
Hide comment
@wycats

wycats Nov 19, 2012

Member

@elliterate if you DRY up the error callback, this looks good to go.

Member

wycats commented Nov 19, 2012

@elliterate if you DRY up the error callback, this looks good to go.

Evan Farrar & Ian Lesperance Ian Lesperance
Handle some create and update errors in the REST adapter.
* 422 Unprocessable Entity

  Transitions the record to the invalid state. Assumes
  the server has responded with `{errors: {...}}` and
  sets them on the record.

* All other status codes

  Transitions the record to the error state.

This is limited to only single, non-bulk creates/updates.
@pivotal-medici

This comment has been minimized.

Show comment
Hide comment
@pivotal-medici

pivotal-medici Nov 19, 2012

Contributor

The error callback is now DRY.

Contributor

pivotal-medici commented Nov 19, 2012

The error callback is now DRY.

@tomdale

This comment has been minimized.

Show comment
Hide comment
@tomdale
Member

tomdale commented Nov 20, 2012

programming

tomdale added a commit that referenced this pull request Nov 20, 2012

Merge pull request #476 from mhelabs/server_error_handling
Handle some create and update errors in the REST adapter.

@tomdale tomdale merged commit 36a70c9 into emberjs:master Nov 20, 2012

1 check passed

default The Travis build passed
Details
@florent-blanvillain

This comment has been minimized.

Show comment
Hide comment
@florent-blanvillain
@florent-blanvillain

This comment has been minimized.

Show comment
Hide comment
@florent-blanvillain
@ryanjm

This comment has been minimized.

Show comment
Hide comment
@ryanjm

ryanjm Apr 25, 2013

Is there plans (soon) to add support for 422 (?) on deletes? How should it be handled if an object shouldn't be allowed to be deleted?

ryanjm commented Apr 25, 2013

Is there plans (soon) to add support for 422 (?) on deletes? How should it be handled if an object shouldn't be allowed to be deleted?

@madadam

This comment has been minimized.

Show comment
Hide comment
@madadam

madadam Apr 25, 2013

Contributor

If the object is not allowed to be deleted, wouldn't 403 be more appropriate response?

Contributor

madadam commented Apr 25, 2013

If the object is not allowed to be deleted, wouldn't 403 be more appropriate response?

@ryanjm

This comment has been minimized.

Show comment
Hide comment
@ryanjm

ryanjm Apr 25, 2013

You are right. 403 would be better.

On Apr 25, 2013, at 3:12 PM, "Adam Cigánek" notifications@github.com
wrote:

If the object is not allowed to be deleted, wouldn't 403 be more
appropriate response?


Reply to this email directly or view it on
GitHubhttps://github.com//pull/476#issuecomment-17039201
.

ryanjm commented Apr 25, 2013

You are right. 403 would be better.

On Apr 25, 2013, at 3:12 PM, "Adam Cigánek" notifications@github.com
wrote:

If the object is not allowed to be deleted, wouldn't 403 be more
appropriate response?


Reply to this email directly or view it on
GitHubhttps://github.com//pull/476#issuecomment-17039201
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment