Keystone-classic: In admin UI virtuals can't access field values not included in `defaultColumns`

[creynders]

If you want to include a virtual to your defaultColumns the fields it accesses _must_ be included in defaultColumns as well.

E.g.:

//not working:
Foo.schema.virtual( 'foo' ).get( function(){
  return this.baz;
} );
Foo.defaultColumns = "name, foo";

//works:
Foo.schema.virtual( 'foo' ).get( function(){
  return this.baz;
} );
Foo.defaultColumns = "name, baz, foo";

I assume some filtering of the document happens for speed reasons, which makes sense, but is there some way we can force the document to have specific fields inside our virtual getters?

[ianbjorndilling]

I recreated your problem and looked through the code. Looks like you're right, the document gets filtered and thus the field the virtual is accessing is undefined. For it to work, the entire document would have to be retrieved, and then each field in defaultColumns would have have to be filtered out of the result.

[morenoh149]

solution proposal?

[creynders]

@morenoh149 I suppose the easiest solution would be to allow the explicit exclusion of fields from filtering or another option would be to allow adding fields to defaultColumns yet hiding them. Something like: MyModel.defaultColumns = "name, someVirtualField, usedByVirtual:hidden"

[pricco]

I'm using the following patch:

var keystone = require('keystone');

keystone.List.prototype.defaultSelectColumns = keystone.List.prototype.selectColumns;
keystone.List.prototype.selectColumns = function(query, columns) {
  var list = this;
  var allColumns = columns;
  columns.forEach(function(col){
    var virtual = list.model.schema.virtuals[col.path];
    if(virtual && virtual.depends) {
      // TODO: Check if the column was already added
      allColumns = allColumns.concat(list.expandColumns(virtual.depends));
    }
  });
  return this.defaultSelectColumns(query, allColumns);
};

Example:

Ticket.schema.virtual('total').get(function() {
  return this.limit || '∞';
}).depends = 'limit';

[creynders]

@pricco aw man, that's a very cool solution! Maybe turn it into a PR?

[morenoh149]

closing as elemental UI merge makes this irrelevant.

[sebastiancarlsson]

+1 this bug is still present in 0.3.19