Keystone-classic: Field validation error when using object reference

[ghost]

Expected behavior

Create item (document) using __ref, when model defined with required: true and _without_ many: true.

Actual behavior

Error occurs when applying an update.

{ [ValidationError: Post validation failed]
  message: 'Post validation failed',
  name: 'ValidationError',
  errors: 
   { category: 
      { [ValidatorError: Path `category` is required.]
        message: 'Path `category` is required.',
        name: 'ValidatorError',
        properties: [Object],
        kind: 'required',
        path: 'category',
        value: undefined } },
  model: 'Post',
  data: 
   { name: 'A draft post',
     category: 'keystone',
     __doc: { _id: 574e9e74a7f5369c84ac372c, name: 'A draft post' } } }

Steps to reproduce the behavior

Apply update using models and update script below.

// models/Post.js

var keystone = require('keystone');
var Types = keystone.Field.Types;

var Post = new keystone.List('Post', {
    autokey: { path: 'slug', from: 'name', unique: true }
});

Post.add({
    name: { type: String, required: true },
    category: {
        type: Types.Relationship,
        ref: 'PostCategory',
        initial: true,
        required: true
    }
});

Post.register();

// models/PostCategory.js

var keystone = require('keystone');

var PostCategory = new keystone.List('PostCategory', {
    autokey: { from: 'name', path: 'key', unique: true }
});

PostCategory.add({
    name: { type: String, required: true }
});

PostCategory.relationship({ ref: 'Post', path: 'category' });
PostCategory.register();

// updates/0.0.1.js

exports.create = {

    PostCategory: [{
        name: 'Keystone JS',
        __ref: 'keystone'
    }],

    Post: [{
        name: 'A draft post',
        category: 'keystone'
    }]

};

[r3wt]

It should be noted that if the object referenced by the relationship field is deleted, then updating the record fails in Mongo DB and spits out an error. This can become quite problematic. In a non rdbms, fault tolerance is not expected. It should be possible to catch the error, silently drop the reference and continue the update.

IIRC, mysql updates any nullable reference to null when removed. Since we can't have this behavior in Mongo it would make sense to handle it by silently deleting on update.

[macroramesh6]

same issue with mongodb

[kazazes]

I can confirm that this bug is still quite real. __ref does not work as advertised.

[kazazes]

Further testing shows that only top level references are populated, all nested refs are ignored.

[romecode]

this bug is still affecting 0.4beta5, just spent about 2 hours pulling my hair trying to figure out wth was going on

[Noviny]

Just verified this as a bug for items that have a relationship of required: true in update scripts. Don't have an immediate solution, but looking in to how we can solve this.

Just want to clarify that the PostCategory.relationship won't work as intended int eh above code sample, but that is not the issue. Below is the intended implementation.

PostCategory.relationship({ ref: 'Post', path: 'category', refPath: 'category' });

[Noviny]

The root cause is likely to be non-trivial to solve, so for anyone needing to press forward, a possible workaround is to have two update scripts, the first of which adds items to be depended upon, the second as a function that retrieves the items by their reference, before saving the new items.

[export-mike]

I've been looking into this, we need to capture more information of the data structure being imported ahead of 'saving' in createItems. To do this will require a significant refactoring and testing to be sure we do not break existing behaviour. I'm going to keep this in mind and figure out a clear path.

For now, as @Noviny suggested you can structure your input into keystone ahead of import. I suggest something like the following to have total control over your import logic when using required: true.

const keystone = require('keystone');
const PostCategory = keystone.list('PostCategory');
const Post = keystone.list('Post');

const importData = [
    { name: 'A draft post', category: 'Keystone JS' },
    ...
];

exports = function (done) {
    const importPromise = importData.map(({ name, category }) => createPost({ name, category }));

    importPromise.then(() => done()).catch(done);
};

const categories = {};

const createPost = ({ name, category }) => {
    let postCategory = new PostCategory.model({ category });
    if (categories[category]) {
        postCategory = categories[category];
    }
    categories[category] = postCategory;
    const post = new Post.model({ name });
    post.category = postCategory._id.toString();
    return Promise.all([
        post.save(),
        postCategory.save()
    ]);
}

[thekevinbrown]

Unfortunately it looks like addressing this fully isn't going to make the v4 release unless someone can contribute a PR. I went ahead and made a note about this in the docs here: https://github.com/keystonejs/keystone/commit/1d2830132bab183340dc34fdc36fe1a659d1e613

I'll leave this issue open so others know that it's a thing, but unfortunately addressing it fully isn't on our roadmap at this stage.

[stevenkaspar]

Here is one way to handle it, but it requires changing data a little bit - it isn't very dynamic so it won't handle child refs but it could probably be improved easy enough

const keystone = require('keystone')
const moment = require('moment')

const DATA = {
  User: [
    { 'name.first': 'Admin', 'name.last': 'User', 'email': '[email protected]', 'password': 'password', 'is_admin': true },
    { 'name.first': 'Jack', 'name.last': 'Smith', 'email': '[email protected]', 'password': 'password', 'is_admin': false, },
  ],
  Visit: [
    {
      'REF__user': {ref: 'User', find: {email: '[email protected]'}},
      'checkin_date': moment().subtract(10, 'days').toDate(),
    }
  ],
  //
  Invoice: [
    {
      'REF__visit':    {ref: 'Visit', find: async () => {
        let user = keystone.list('User').model.findOne({email: '[email protected]'})
        return {
          user: user._id
        }
      }},
      'date': moment().subtract(9, 'days').toDate(),
      'amount': 1000.00,
    }
  ],
};

async function create (keystone_list, data) {
  let List = keystone.list(keystone_list)
  let created_items = []
  for(let item of data[keystone_list]){
    for(let key in item){
      // use 'REF__' to define fields
      if(/^REF__/i.test(key)){
        // remove 'REF__' and use as mongoose field name
        let keystone_field = key.replace(/^REF__/i, '')
        // allows for many: true
        if(Array.isArray(item[key])){
          item[keystone_field] = []
          for(let many_item of item[key]){
            let ref_item = await keystone.list(many_item.ref).model.findOne(await many_item.find)
            if(ref_item === null) throw new Error(`${many_item.ref} find \n${JSON.stringify(many_item.find, null, '  ')}\n is null`)
            item[keystone_field].push(ref_item._id)
          }
        }
        else {
          let ref_item = await keystone.list(item[key].ref).model.findOne(await item[key].find)
          if(ref_item === null) throw new Error(`${item[key].ref} find \n${JSON.stringify(item[key].find, null, '  ')}\n is null`)
          item[keystone_field] = ref_item._id
        }
        // delete the 'REF__xx' key so we don't keep that on the document
        delete item[key]
      }
    }
    let new_item = new List.model(item)
    await new_item.save()
    created_items.push(new_item)
  }
  return created_items
}

exports = module.exports = async function (done) {
  for(let model in DATA){
    try {
      await create(model, DATA)
    }
    catch(e){
      done(e)
    }
  }
  done()
};