Skip to content

becameError / becameInvalid / errors hash changed #1313

Closed
gerry3 opened this Issue Sep 16, 2013 · 8 comments

4 participants

@gerry3
gerry3 commented Sep 16, 2013

We are continuing our upgrade from 0.13 to // Last commit: 8e8ed7e (2013-09-15 17:24:13 -0700). The transition guide doesn't mention errors at all yet.

We are returning 422 & JSON with an errors root node (of ActiveRecord errors) e.g. render json: { errors: @post.errors }, status: :unprocessable_entity

Old behavior:
An errors property was set on the model (to a parsed object, but not an ember object) & becameInvalid was called on the model.

Current behavior:

return promise.then(function(payload) {
    payload = serializer.extract(store, type, payload, get(record, 'id'), operation);
    store.didSaveRecord(record, payload);
    return record;
  }, function(reason) {
    if (reason instanceof DS.InvalidError) {
      store.recordWasInvalid(record, reason.errors);
    } else {
      store.recordWasError(record, reason);
    }

    throw reason;
  }).then(resolver.resolve, resolver.reject);

reason is not a DS.InvalidError (we couldn't figure out if DS.InvalidError was instantiated anywhere), so becameError is called in our model and there are no parsed errors from the server.

We have been hacking this code a bit, but not having much luck. Next we are going to go back to the old code & see if we can steal the parsing logic and stick it in here.

In any case, is there a plan to support something similar to what was done before? Should we doing something in a custom serializer or using a different standard serializer or some other approach?

@gerry3
gerry3 commented Sep 16, 2013

Ok, this _commit hack would work except for our underscored keys:

return promise.then(function(payload) {
  ...
  }, function(reason) {
    if (reason.status === 422) {
      reason = new DS.InvalidError(JSON.parse(reason.responseText).errors);
    }

    if (reason instanceof DS.InvalidError) {
      store.recordWasInvalid(record, reason.errors);
    } else {
      store.recordWasError(record, reason);
    }

    throw reason;
  }).then(resolver.resolve, resolver.reject);

So we just added the old extractValidationErrors & keyForAttributeName methods from 0.13 to our serializer and do this:

return promise.then(function(payload) {
    ...
  }, function(reason) {
    if (reason.status === 422) {
      var json = JSON.parse(reason.responseText),
          errors = serializer.extractValidationErrors(type, json);
      store.recordWasInvalid(record, errors);
    } else {
      store.recordWasError(record, reason);
    }

    throw reason;
  }).then(resolver.resolve, resolver.reject);

The next issue is that we are unable to recover from the error state (root.loaded.created.invalid) the way we did before (in the model): @get('stateManager').transitionTo('loaded.created.uncommitted') since stateManager is now undefined.

@cyclomarc

You can find some more info about recovering from an error state in this article: http://discuss.emberjs.com/t/migrating-from-ember-data-0-13-to-1-0-0-beta-1-my-findings/2368

Good luck !

@gerry3
gerry3 commented Sep 16, 2013

Ok, our hack is working now.

Instead of transitioning the stateManager, we transition the record itself:

Before:

  • update: stateManager > loaded.updated
  • create: stateManager > loaded.created.uncommitted

After:

  • update: this > saved
  • create: this > uncommitted
@decasia
decasia commented Sep 16, 2013

thanks for posting all this, @gerry3, since you're not the only one with these issues. I've just spent part of the afternoon trying to figure out what happened to the isInvalid state in ember-data beta, and I'm glad to find this post.

@cyclomarc, your guide is a start but I think @gerry3's code gives a better developed example of how to sort out the state management question.

For what it's worth, I'm working on aggregating errors from a bunch of associated records that get saved together, and I've ended with something like this:

successes = 0
failures = 0
generalError = false
recordWasSaved = =>
  if successes + failures == dirtyRecords.length
    if failures == 0 then @afterSuccess() else @afterFailure(generalError)

dirtyRecords.forEach (record) =>
  record.save().then (record) =>
    successes += 1
    recordWasSaved()
  , (response) =>
    failures += 1
    if response.status == 422
      record.set 'errors', App.NormalizeErrors response.responseJSON
    else
      generalError = true
    recordWasSaved()

Similar logic to yours, except I should be using the state management hooks you are using...

@gerry3
gerry3 commented Sep 22, 2013

@wycats For us, just some way to execute the old error logic in a real hook rather than a hack to _commit would work or is there a plan for errors? (seems like there might be as it doesn't look like DS.InvalidError is being instantiated anywhere at the moment)

I would love to help in any way that I can--just let me know.

@wycats
Ember.js member
wycats commented Sep 22, 2013

@gerry3 what kind of hook are you thinking of? Can you show me an example?

@gerry3
gerry3 commented Sep 24, 2013

@wycats exactly like this one from less than a day ago: 756bf2c

However, there is a bug in the ActiveModelAdapter's implementation of this ajaxError hook: it does not pass the errors hash keys through the ActiveModelSerializer's keyForAttribute method.

I am happy to work on a pull request for this, but I need some help. Like how can I get a reference to the serializer in ajaxError? Would I have to pass the store to ajax so I can pass the store & type to ajaxError and use store.serializerFor(type)? Is there an IRC or something I can join?

@gerry3
gerry3 commented Sep 24, 2013

I am closing this in favor of the new issue #1359

@gerry3 gerry3 closed this Sep 24, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.