Referenced field shows id (not populated) after migration to v8

Feature(s) impacted

all collections with referenced fields

Observed behavior

when loaded referenced fields are not populated, only shows id

Expected behavior

referenced field should show the name/referenced value

Context

  • Project name: WescoverContentAdmin
  • Team name: Content Admin
  • Environment name: NIMROD - LOCAL DEV
  • Agent type & version: forest-express-mongoose 8.7.1
  • Recent changes made on your end if any: migrating to v8

there seems to be an issue with referenced fields in v8, when loaded they display the referenced field id and not the value (not being populated)

i saw a post on this forum with the same issue which was resolved by changing the version of liana to the same version as the production, is this the case?

thanks in advance

Hi @nmz and welcom in our community :champagne: !

I’m trying to reproduce and look what could go wrong. I’ll try to reply asap

I can’t reproduce your issue while I’m on v8. Plus looking at your environment I see it on 7.9.2 version so not v8 :thinking:

hi @vince ,

thanks for replying.
the version on production is v7 because of this bug.
on dev environment its v8.
furthermore, when downgrading to v7 the problem disappears.
ill try to find a way to reproduce.

thanks.

Could you please link the blog post you think might be linked :thinking: ?

1 Like

Could you please share the config you have on the field maker and on your collection Maker (to see the reference field)

could u be more specific? i didnt understand…

maker: { type: mongoose.Schema.Types.ObjectId, ref: ‘Maker’, index: true },

Mongoose: items.aggregate([ { ‘$sort’: { _id: -1 } }, { ‘$skip’: 0 }, { ‘$limit’: 10 } ], {})
GET /forest/Item?timezone=Asia%2FJerusalem&fields%5BItem%5D=_id%2Cname%2Ccategory%2Cmaker%2CcreatedAt%2CmodifiedAt%2Cproduct&fields%5Bcategory%5D=categoryDisplayName&fields%5Bmaker%5D=name&fields%5Bproduct%5D=name&page%5Bnumber%5D=1&page%5Bsize%5D=10&sort=-_id 304 - - 26.764 ms
Mongoose: items.aggregate([ { ‘$group’: { _id: null, count: { ‘$sum’: 1 } } }], {})
GET /forest/Item/count?fields%5BItem%5D=_id%2Cname%2Ccategory%2Cmaker%2CcreatedAt%2CmodifiedAt%2Cproduct&fields%5Bcategory%5D=categoryDisplayName&fields%5Bmaker%5D=name&fields%5Bproduct%5D=name&timezone=Asia%2FJerusalem 304 - - 26.290 ms

thank u!

Hey @nmz,

Sorry, I will try to detail a bit more :sweat_smile:
I need the response in your Network Tab of your browser of the GET /forest/Item
And it could be nice also that you activate the layout editor and access the collection settings of Maker and send me a screen of the General tab :pray:

Hi @vince, I am working with @nmz so just want to move the conv. along by providing the info you requested.

The network response to /Forest/Item payload is quite big so I will quote the interesting parts:
Here is one item with the maker relationship exploded:

And here is an “included” example for a maker exploded:

Here is the Maker collection layout editor screen:

One other piece of info is that the actual only query sent to the DB for this call will look like this:

items.aggregate([ { '$sort': { _id: -1 } }, { '$skip': 0 }, { '$limit': 10 } ], {})

Btw, clicking into one specific Item will indeed show the correct “display field” for a maker - here is a snippet of that response showing this data exists on the included model.
image

Fo this call the queries looks like this:

Mongoose: items.findOne({ _id: ObjectId("619b76bfa2584147d92a982e") }, { projection: {} })
Mongoose: makers.find({ _id: { '$in': [ ObjectId("5dda5716b420c6ef0f831fbc") ] } }, { projection: {} })
... other relationships ...

I feel @nmz referenced blog post is relevant, could it be that the fact our production is still on Liana 7 causes the dev layout to be broken?
Also - we did not migrate yet to the new “environments” dev flow. We wait to do that after successfully upgrading to V8. Could that be related?

Thanks!

1 Like

Hi @Yoad_Snapir , @nmz ,

I cannot reproduce on my side. Can I have your model with your relations ?

Best,

Shohan

Sure,
This would be the relevant parts of the Item model:

const schema = mongoose.Schema(
  {
    name: { type: String, required: true, index: true },
    maker: { type: mongoose.Schema.Types.ObjectId, ref: 'Maker', index: true },
  },
  {
    timestamps: { updatedAt: 'modifiedAt' },
  }
);

And this is the relevant parts from the Maker model:

const schema = mongoose.Schema(
  {
    name: { type: String, required: true, index: true },
  },
  {
    timestamps: { updatedAt: 'modifiedAt' },
  }
);

We create the models using something like this:

 conn.model('Item', require('./items').default, 'items'),
 conn.model('Maker', require('./makers').default, 'makers'),

All very standard (and worked without modification for over a year).
Hopefully this is what you wanted to see?

In the schema JSON file the Item looks kinda like this:

 {
    "name": "Item",
    "nameOld": "items",
    "icon": null,
    "integration": null,
    "isReadOnly": false,
    "isSearchable": true,
    "isVirtual": false,
    "onlyForRelationships": false,
    "paginationType": "page",
    "fields": [{
      "field": "maker",
      "type": "String",
      "defaultValue": null,
      "enums": null,
      "integration": null,
      "isFilterable": true,
      "isPrimaryKey": false,
      "isReadOnly": false,
      "isRequired": false,
      "isSortable": true,
      "isVirtual": false,
      "reference": "Maker._id",
      "inverseOf": null,
      "validations": []
    },
    .... other fields ...
    ]
}

And the Maker looks like this:

{
    "name": "Maker",
    "nameOld": "makers",
    "icon": null,
    "integration": null,
    "isReadOnly": false,
    "isSearchable": true,
    "isVirtual": false,
    "onlyForRelationships": false,
    "paginationType": "page",
    "fields": [{
      "field": "name",
      "type": "String",
      "defaultValue": null,
      "enums": null,
      "integration": null,
      "isFilterable": true,
      "isPrimaryKey": false,
      "isReadOnly": false,
      "isRequired": true,
      "isSortable": true,
      "isVirtual": false,
      "reference": null,
      "inverseOf": null,
      "validations": [{
        "message": null,
        "type": "is present",
        "value": null
      }]
    },
    .... other fields ...
    ]
}
    

I think I have a handle on this bug.

Let me try and walk you through the path where I see the problem.

We have defined our “Item” model this way:
conn.model('Item', require('./items').default, 'items'),
So, the Item is the model name, and items is the collection name.

Keep that in mind.

When the Forest client makes a request to load the collection it would issue this call:
/forest/Item
So the model name is the path to reach the data.
And within the HTTP GET query the fields for this call look like this (shortened):
&fields[Item]=_id,name,maker&

Now on the server, directly on the route for this action - this code:

const { query, user } = request;
const recordsGetter = new RecordsGetter(Item, user, query);

Looks at a query with this structure:

{
  fields: {
    Item: "_id,name,category,maker,createdAt,modifiedAt,product",
    ... more
  },
  ... more
}

And RecordsGetter (mongoose) initialized with this query:

constructor(model, opts, params, user) {
    this._model = model;
    this._opts = { Mongoose: this._model.base, connections: this._model.base.connections };
    this._params = params;
    this._user = user;
  }

Now calls this query params - keep that in mind.

Moving on on the call chain - those params are passed down (higlighting intersting parts only in the pipeline):

RecordsGetter.perform()
-> new QueryBuilder(this._model, params, this._opts); // Params goes into query builder under private member this._params
-> await queryBuilder.joinAllReferences(jsonQuery); // query builder called to join references
--> let fieldNames = await this.getFieldNamesRequested(); // getFieldNamesRequested called
---> if (!this._params.fields || !this._params.fields[this._model.collection.name]) { return null; } // Ensure the target collection has any fields defined on the query

Look at that last condition:
!this._params.fields[this._model.collection.name]
This turns out to be true since:
this._model.collection.name == 'items'
and:
!this._params.fields["items"] == undefined

If I debug and manually set right before that line:
this._params.fields.items = this._params.fields.Item
I am getting all the joins applied to the DB query and the reference fields are sent to the client correctly.

Tracing the code changes - seems like this is a pretty old condition. What has changed since Liana 7 that could cause this?

This is how the code for QueryBuilder.joinAllReferences Looked at Liana 7.9.2:

async joinAllReferences(jsonQuery, alreadyJoinedQuery) {
    const fieldNames = await this.getFieldNamesRequested();
    this._schema.fields.forEach((field) => {
      if ((fieldNames && !fieldNames.includes(field.field))
          || QueryBuilder._joinAlreadyExists(field, alreadyJoinedQuery)) {
        return;
      }
      this.addJoinToQuery(field, jsonQuery);
    });
    return this;
  }

And here it is for Liana 8.7.2:

async joinAllReferences(jsonQuery, alreadyJoinedQuery) {
    let fieldNames = await this.getFieldNamesRequested();
    const flattenReferenceNames = Flattener
      .getFlattenedReferenceFieldsFromParams(this._model.collection.name, this._params.fields);

    fieldNames = flattenReferenceNames.concat(fieldNames);

    this._schema.fields.forEach((field) => {
      if ((fieldNames && !fieldNames.includes(field.field))
          || QueryBuilder._joinAlreadyExists(field, alreadyJoinedQuery)) {
        return;
      }
      this.addJoinToQuery(field, jsonQuery);
    });
    return this;
  }

See how on the old version the null coming back from getFieldNamesRequested() is not preventing the joins to be added.
Since
(fieldNames && !fieldNames.includes(field.field)) is false.
But on the new version - the fieldNames is produced by this concat:

const flattenReferenceNames = Flattener
      .getFlattenedReferenceFieldsFromParams(this._model.collection.name, this._params.fields);
fieldNames = flattenReferenceNames.concat(fieldNames);

And Flattener.getFlattenedReferenceFieldsFromParams() will always return an array.
So in fact fieldNames will not be null and thus no field join would be added since that short-circuit condition would not occur.

Patching 8.7.2 with something like this indeed solves the problem:

async joinAllReferences(jsonQuery, alreadyJoinedQuery) {
    let fieldNames = await this.getFieldNamesRequested();
    const flattenReferenceNames = Flattener
      .getFlattenedReferenceFieldsFromParams(this._model.collection.name, this._params.fields);

    fieldNames = fieldNames ? flattenReferenceNames.concat(fieldNames) : flattenReferenceNames;

    this._schema.fields.forEach((field) => {
      if ((fieldNames.length && !fieldNames.includes(field.field))
          || QueryBuilder._joinAlreadyExists(field, alreadyJoinedQuery)) {
        return;
      }
      this.addJoinToQuery(field, jsonQuery);
    });
    return this;
  }

Now, Is the problem for us the way the Model name is not the same as the collection name? Or the introduction of the flattener?

To answer to that I am trying to reproduce the same issue.

In which file do you have this implementation conn.model('Item', require('./items').default, 'items'), ?
I don’t have it on my end and this might be a difference between V7 and V8.

Indeed,
We have a multi-DB setup, we have built the connecting+creating models path on our own (since this was a pre7 setup to begin with).

This is how we have it setup:

server.js is the entry point
It calls a setupDbConnections() async function that connects to all databases and produces a “map” of connections.
It looks something like this (names are made up):

{
  content: <mongoose conn from mongoose.createConnection()>
  operational: <mongoose conn from mongoose.createConnection()>
  ... other connections
}

We use this connections map to find the right connection for each model - let’s call this conn and execute:
conn.model('Item', require('./items').default, 'items'),
with the correct Schema on that already active connection.

Now we have a “map” of active Models:

{
  Item: <mongoose model for Item>
  ... other models
}

Then, to use those connections and models with Forest middlewars:

requireAll({
  dirname: path.join(__dirname, 'routes'),
  recursive: true,
  resolve: (ModuleFactory) =>
    app.use('/forest', ModuleFactory.default(connectionsMap, modelsMap)),
});

requireAll({
  dirname: path.join(__dirname, 'middlewares'),
  recursive: true,
  resolve: (Module) => Module.default(app, connectionsMap),
});

This is probably where the normal Forest implementation is already familiar.
The routes and middlewares folders are the standard ones.
We do not use Forest’s “template” model generation - instead we create the models on our own with our own DB connections since we have a custom setup for those.

Does that help?

Perhaps I should also add that within the middlewares folder we have a forestadmin file that exports this:

export default async function (app, connections) {
  app.use(await Liana.init({
    configDir: path.join(__dirname, '../forest'),
    envSecret: process.env.FOREST_ENV_SECRET,
    authSecret: process.env.FOREST_AUTH_SECRET,
    connections,
    objectMapping: Mongoose,
  }));

  console.log(chalk.cyan('Your admin panel is available here: https://app.forestadmin.com/projects'));
};

That connections parameter is the same connection map I showed above we have created. And you can see how we use the configDir which contains the Liana configs for the collections on top of the models we created.

And on the routes folder we have route files that export something like this:
items.js

export default (connectionsMap, modelsMap) => {
  const router = express.Router();
  const permissionMiddlewareCreator = new PermissionMiddlewareCreator('Item');
  // Code for some routes overrides
}

And those two params are again the connections and models map which we need to implement custom routes.

As you can see, this all revolves around the fact that model name and collection name are not the same thing. And the model name is how we reference all our collections in the code we use to configure Liana. That line of code that tries to look for a collection name in the “fields” map is the odd thing for me.

This is code from the forest-express package - on the Liana.collection() function:

var collection = _.find(Schemas.schemas, {
    name: name
  });

  if (!collection) {
    collection = _.find(Schemas.schemas, {
      nameOld: name
    });

    if (collection) {
      name = collection.name;
      logger.warn("DEPRECATION WARNING: Collection names are now based on the models names. Please rename the collection \"".concat(collection.nameOld, "\" of your Forest customisation in \"").concat(collection.name, "\"."));
    }
  }

You can clearly see that there was a shift to use collection names which are based on the Model names. And this code has a fallback to the oldName just in case. In our schema file you can see for example:

{
    "name": "Item",
    "nameOld": "items",
...

So the mongoose “Model” is the name of the collection and the mongoose mongodb collection name is the “oldName”.
That code on the Query Builder should use model names to comply with this change - but it does not. It used to have a fallback (intentional or not) that made it still work and create joins but after introducing the flattener change this fallback is broken.

Lets hope this is enough to reproduce and provide a fix.

@vince @shohanr Guys - I want to make sure you saw all our input on this - we invest a lot to find the root cause here.
We are unable to complete the upgrade to Liana 8 and really stuck.
Thanks.

Hello @Yoad_Snapir,

Really sorry for the delay. I’ve pinged @vince and @shohanr so we can reply in a short delay. In the meanwhile, I will also take a third look at your issue.

Kind regards,
Morgan

1 Like

Hi @Yoad_Snapir,
we are looking into it.
We’ll let you know as soon as we found something

A fix will be released in next version available in a few seconds now