Validating with DS.Errors

[mapohjola]

We are currently using DS.Errors to have a coherent way of showing record errors to a user. The issue #3853 changes the behavior of add and clear methods on DS.Errors.

Currently we are doing something like this with our validation.

• Trigger validation
• If validation error found use errors.add(attribute, message)
• If record has flag isInvalid, show message to user

See twiddle: https://ember-twiddle.com/f62581bdad4fa2005b74976758243445?fileTreeShown=false&numColumns=2&openFiles=routes.application.js%2Ctemplates.application.hbs

The pull request will remove this behavior in 3.x and replace with _add and _clear methods.

I mostly agree with

I don't think the error class should be responsible for updating the model state. From #3853

but my concern is that we will have no public API for adding or clearing errors on a record.

[bmac]

@tchak I believe you have/had the same use case. What did you end up doing?

[tchak]

We are still doing it like @mapohjola... I think the only change is the fact that with new behaviour one will need to check isValid and errors.isEmpty, no?

[mapohjola]

The new methods will not flag the record isValid / isInvalid, that is something you can handle on your own with sending becameValid/ becameInvalid. This is atleast how I understand it. You could also check errors.isEmpty if you don't want to mark the record invalid yourself.

I just feel that there should be a public way to work with errors.

[cibernox]

♻️ ♻️ ♻️ Consider the environment before printing this message ♻️ ♻️ ♻️

After conversation with @pangratz in slack, I ping @tchak with a use case that can be relevant for many people.

Note: Examples will use AMS payloads

Save one model along with the models in a hasMany relationship (N levels deep) in one request.

Imagine that you have a complex form like this:

On this form you are saving a SpellingTest that contains N Trays (each one is a tab). Each Tray contains N Tasks (Each "row" within the tab).

Logically all this information should be saved to the server transactionally in a single request, so when saving to the backend, the serializer uses DS.EmbeddedRecordsMixin to send the entire tree of models.

import BaseSerializer from 'my-app/serializers/application';
import DS from 'ember-data';

export default BaseSerializer.extend(DS.EmbeddedRecordsMixin, {
  attrs: {
    trays: { serialize: 'records', deserialize: 'ids' }
  }
});

The payload sent to the server looks like this, for an update operation where I intentionally cleared the title of the spelling test and the description of one of the tasks of the first tray to force a validation error.

{
  "created_at": "2015-11-24T00:00:00.000Z",
  "updated_at": null,
  "due_on": "2016-04-28T19:28:34",
  "description": "Lorem Ipsum is simply dummy......",
  "original_class_task_id": null,
  "issued_on": "2015-04-04T01:00:00",
  "published_at": "2015-04-04T00:00:00.000Z",
  "title": "",                                                    // Intentionally blank
  "hand_in_online": false,
  "purpose": "2",
  "subject": "English",
  "class_year": "1",
  "language": "en_uk",
  "school_id": "1",
  "teacher_id": "9991",
  "class_group_id": "1",
  "marking_scheme_id": null,
  "trays": [
    {
      "id": "1",
      "title": "Easy",
      "tasks": [
        {
          "id": "1",
          "text": "",                                             // Intentionally blank
          "sentence": "Water can be found in the ocean",
          "definition": "Basically you fill a glass and BAM sorted!",
          "file_url": "/audio/speak.mp3"
        },
        {
          "id": "2",
          "text": "Addition",
          "sentence": null,
          "definition": "You go to the ocean with the glass and then u fill it",
          "file_url": "/audio/speak.mp3"
        }
      ]
    },
    {
      "id": "2",
      "title": "Hard",
      "tasks": [
        {
          "id": "4",
          "text": "Basket",
          "sentence": null,
          "definition": "Over 1 year, the average Vim user saves 11 minutes in productivity.\n However, they loose 27 hours through evangelising Vim to non-users.",
          "file_url": "/audio/speak.mp3"
        },
        {
          "id": "5",
          "text": "Cookie",
          "sentence": "Figure out why everything's broken",
          "definition": "A dessert made with chocolate chips",
          "file_url": "/audio/speak.mp3"
        }
      ]
    }
  ]
}

The errors object returned by the server (although I'd be glad to change it if there is a better standard response that works out of the box) is this one:

// Status code is 422
{
  "errors":{
    "title": ["blank"],                // There is errors on the title of the SpellingTest
    "trays":{                         // There is errors in the trays relationship of the SpellingTest...
      "0":{                             // ... in the first model of the collection...
        "tasks":{                     // ... the error is in the tasks relationship ...
          "0":{                         // ... in the first task of the collection...
            "description": ["blank"]  // ... in the description
          }
        }
      }
    }
  }
}

Why did I use one object with numbers as keys to express what object had errors instead of an array? Because maybe only the 1st and the 4th elements of the collection have errors, and using an arrays means I would have had to insert empty objects when some records were correct. But that would be too bad, I could do that too.

How do I transfer those errors to the proper models? I include this mixing on the models that can be saved along with their hasMany relationships.

import Ember from 'ember';

const { camelize } = Ember.String;

export default Ember.Mixin.create({
  becameInvalid(instance) {
    this._super(...arguments);
    let collectionErrors = [];

    instance.eachRelationship((name, descriptor) => {
      if (descriptor.kind === 'hasMany') {
        collectionErrors.push(this.get(`errors.${name}.firstObject`));
        this.get('errors').remove(name);
      }
    });

    collectionErrors.compact().forEach(errorObject => {
      const attribute = camelize(errorObject.attribute);
      const message   = errorObject.message;

      this.get(attribute).toArray().forEach((element, index) => {
        let localErrors = message[index];
        if (localErrors) {
          for (let key in localErrors) {
            if (localErrors.hasOwnProperty(key)) {
              element.get('errors').add(camelize(key), localErrors[key]);
            }
          }
          element.becameInvalid(element);
        }
      });
    });
  },
});

Given that the errors were just copied in the top-most record, I use the becameInvalid callback to copy the errors to the proper model in the relationship. The I call invoke becameInvalid on those records too so the process is recursive.

This works when nested records are being created, but not when records are being updated. The error message I get is: Attempted to handle event becameInvalid on <smhw-frontend@model:spelling-test-tray::ember1982:1> while in state root.loaded.saved.`

I'd swear that this worked at some point in time but I can be sure because this code is ~9m old.

Summary

We need some API format that populates errors on records being saved with DS.EmbeddedRecordsMixin.
But given that JSON-API haven't (afaik, correct me if I'm wrong) settled on any payload format for this kind of nested error, in the interim we need:

1. A public API to manage errors on models
2. I think that records being saved nested into another records with DS.EmbeddedRecordsMixin are not marked as inFlight and that is probably the cause of the error I'm facing. Seems that they should.

[tchak]

JSON-API error source.pointer is what you looking for. The error object part is actually all figured out. Problem is ED do not handle deep pointers right now because there is no consensus on batching format for JSON-API. It is pretty clear that embedding is not going to be supported. Current line of thoughts seems to be "JSON-API operations". I know that @dgeb is working hard on some RFC right now, so hopefully we are going to have something to talk about soon.

We do have a public api to manage errors. I agree we should settle on some public api for marking records as invalid. The two seems to be independent.

[runspired]

I vaguely remember us deciding to preserve the behavior of changing the valid/invalid state when using add/clear. We should confirm/deny this.

Regardless, the CUSTOM_MODEL_CLASS and RECORD_DATA_ERRORS and RECORD_DATA_STATE RFCs now allow for addons to provide the precise error handling behavior desired and it is unlikely we make changes to @ember-data/model in this area.

Closing since we won't be pursuing a change.