Skip to content
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

Can no longer access model errors after failed save #564

Open
lindyhopchris opened this issue Dec 3, 2020 · 5 comments
Open

Can no longer access model errors after failed save #564

lindyhopchris opened this issue Dec 3, 2020 · 5 comments

Comments

@lindyhopchris
Copy link

lindyhopchris commented Dec 3, 2020

Version

[email protected]
[email protected]

Test Case

None

Steps to reproduce

Set up a scenario where the server rejects a save with validation errors, as per this example in the readme:

changeset
  .save()
  .then(() => {
    /* ... */
  })
  .catch(() => {
    get(this, 'model.errors').forEach(({ attribute, message }) => {
      changeset.addError(attribute, message);
    });
  });

Expected Behavior

model.errors contains errors

Actual Behavior

model.errors has no errors as original value has been restored on the model, wiping the errors out.

I believe this has been caused by this change:
validated-changeset/validated-changeset#80

When the original value of an attribute is restored onto the model, the model clears the errors for that attribute. That means by the time the error is caught, model.errors contains no error messages.

It is therefore no longer possible to add model errors to a changeset.

The exception that Ember Data throws does not have the parsed errors on it - it has the raw JSON:API response. So there is no way to access the parsed Ember Data errors and push them into the changeset.

@snewcomer
Copy link
Collaborator

👋 @lindyhopchris Do you happen to have a reproduction? I added a test here and all seems ok. We only rollback changes made through the changeset. Does DS.errors get set on the changeset as a result of a failed save? I would have assumed it would only exist on the model (content as referred in our codebase).

@lindyhopchris
Copy link
Author

@snewcomer thanks for getting back to me!

your test actually shows what the problem is. this line:

expect(dummyModel.name).toEqual('previous');

that shows that the value on the dummy model has been set back to the value it was before the changeset was saved. I.e. the sequence is:

  • changeset.save() sets dummyModel.name from previous to new
  • save rejects.
  • something sets dummyModel.name from new back to previous.
  • then the assertion in the finally block passes because dummyModel.name has been set back to previous.

However, that's the problem - setting the name back to previous on a real Ember Data model removes the error message for the name property.

I.e. for the Ember Data model to still have the error message for name, the assertion in the finally block needs to be:

expect(dummyModel.name).toEqual('new');

@lindyhopchris
Copy link
Author

I can't find any reference to it in the guides or the API docs, but basically when you set an attribute on an Ember Data model, if there is an error message for that attribute, the error message gets cleared. That's why setting the name value back to its original value clears the error message for the name attribute. Hope I've explained that ok!

@snewcomer
Copy link
Collaborator

Oh you are right. I naively assumed the errors from the API would be set and persist regardless of the DS.Model value. So it must clear out the errors when the value is set to the previous as you have described.

We are a little at odds because the underlying model is only expected to only update the underlying model upon a verified "proper" state on the changeset. I'll have to think about this!

@lindyhopchris
Copy link
Author

Yeah totally get why you're reverting the model after the save fails... does kind of make sense. Not sure what to suggest so interested to hear your thoughts once you've had a chance to think about it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants