Calculated fields and smart fields problems (regressions in 6.3.7 and 6.6.2?)

Hi Forest guys.

It looks like there’s a problem with calculated fields since you released v6.3.7 and we can see a second problem with smart fields since v6.6.2.

Expected behavior

Fields calculated “on the fly” (with a getter in the field) are expected to show in the summary view (even when a “related data” element is added to the view). And smart fields values are supposed to always show.

Actual behavior

There are two problems.

Problem with fields calculated with a getter (since v6.3.7)

In our sequelize model we have some fields whose value is calculated with a sequelize getter in the field itself. Here is the field definition inside our sequelize model:

providerCommissionsPercentage: {
  type: DataTypes.DECIMAL(5, 2),
  get() {
    // Workaround until sequelize issue #8019 is fixed see https://github.com/sequelize/sequelize/issues/8019#issuecomment-523265759
    const value = this.getDataValue('providerCommissionsPercentage')
    return value === null ? null : parseFloat(value)
  },
  field: 'provider_commissions_percentage',
  allowNull: false,
},

Let’s say we have a “main” model with a “hasMany” relationship with another “linked” model (which “belongsTo” th main model.

With version 6.3.7, in case of relationships, the instance passed to any smart field of the “main” model when it’s loaded by the elements shown of the “linked” model in the “related data” element in the summary view, contains only the id. So calling this.getDataValue('providerCommissionsPercentage') in the “main” model creates an unexpected behavior, because the dataValue of the field is not there any more (there’s only the id in the instance received!!). So, in the Forest front end the values are first shown (when the “main” model is loaded), then they disappear (maybe the values of the main model loaded by the “linked” model elements shown in the “related data” are written to the store of the single page application? They should not! I thin the store update is not prevented when the “main” model data is re-loaded due to the related collections… or something like this…).

The same happens in the v6.3.15, where you don’t re-load at all the smart fields of the “main” model when loading the “linked” elments in the “related data” view item. But I think, maybe, you still write the missing values in the store of the single page application (front end), so the values disappear immediately after loading.

See this video:

With version 6.6.2 the same still happens, but we also have another problem, related to the smart fields.

Problem with smart fields (since v6.6.2)

The smart fields show NaN instead of their actual value and the previously described problem is still there. In our case, the NaN value is due to the price widget The problem is that the underlying value is undefined. In the following picture, the red arrow points to the field that disappears if a related data collection is set in the summary view (see the above video). And the purple arrows point to the NaN smart fields of the collection.

This is the smart field definition (I evicted almost everything, to be sure to be free from other errors):

const { collection } = require('forest-express-sequelize')

collection('Booking', {
  fields: [
    {
      field: 'netProviderPayoutField',
      type: 'Number',
      get: () => {
        console.log('Here we are!')
        return 10 // booking.netProviderPayout
      },
    },
  ],
})

More over, I can’t find the Here we are! string any where in the logs. It looks like the smart field is not called at all.
And I can’t see the same behavior in all collections: in some other collection smart fields show properly. And in some other collections the value of the smart fields is not loaded. Can’t explain why this happens, but the problem came with v6.6.2.

I tried to reduce the interested model to its “minimum” by removing all the fields but the id, I removed all the relationships in the model and all the smart fields and actions associated to the collection, but I have the same result.

Downgrading to 6.6.1 solves this second problem.

Hello @Matteo,

Thank you for your feedback. The team just released version 6.6.3 for forest-express-sequelize. It will hopefully fix your issue.

Please let me know if this is the case.

Thank you @Guillaume_Deslandes
v6.6.3 solves the second problem described above.

But the behavior described in the first problem in the above paragraph "Problem with fields calculated with a getter (since v6.3.7)" is still there.

The video above still shows exactly what happens now, even with 6.6.3. As far as I can tell, it’s a regression introduced with v6.3.7.

Hi @Matteo,

I understand you turn a stringified decimal right back to decimal because of a bug (or a lack) in sequelize, using a sequelize getter.
The following is working on my side:

// Using:
const value = this.dataValues['providerCommissionsPercentage'];
// instead of:
const value = this.getDataValue('providerCommissionsPercentage')

Let say if it helps.
If not, please let me know if the field is working (as a string) without de getter.

Thank you @Sliman_Medini
Yes, accessing dataValues directly is a good workaround and now everything work fine. Thank you.

But let me better understand, please.
Are you going to fix this?
Or the getDataValue method is not recommended?
And why this method creates a problem (only in the context of a sub collection, since the release 6.3.7, where you stopped loading the second level related collections)?

Thank you in advance for your explanations.
Matteo

Hi Matteo,
Version 6.6.2 seems to have a regression for the get one record: the smart fields are not computed from the lumber app.
Downgrading to version 6.6.0 forest-express-sequelize seems to fix the problem.

We are investigating the problem to release a patch.
Let me know if this fixes your current issue.

Hi @SebastienP,
thank you for your reply. But it looks like your answer doesn’t consider the previous posts in this thread.

Just to recap:

So now I have no more problems to solve: no need to downgrade to 6.6.0. Well… nothing I have experienced yet :slightly_smiling_face:

I was just wondering which is the reason because we shouldn’t use the getDataValue function in the field getter. @Sliman_Medini, may you please elaborate?

Thank you in advance,
Matteo

1 Like

Hello @Matteo,

Thank you for your feedback :slight_smile:
I don’t really understand why it would not work with getDataValue function as it is defined as:

getDataValue(key) {
   return this.dataValues[key];
}

So it should be the exact same thing.
Which sequelize version do you use?

Furthermore, can you bring more data about your main model and your linked model?
A basic reproducible project would be great from a simple psql script.

If I understand correctly you declare the provider_commissions_percentage field and its getter in a main model in the models folder. But you also create a smart field for this main model in the forest folder? Is there also a getter pertained to this smart field?

I tried to reproduce your setup.
When displaying a record of the main model in the summary or detail view, its related data listing the linked model are correctly displayed.

What am I missing ?
Could you tell me more about the smart field you are talking about.

Have a great day :smiley:

Hi @Guillaume_Cisco
I apologize for the delayed reply, but we had Chrismas… So let’s start the new year with an in deep analysis of the problem.

I was wrong. The problem is still there and it’s not solved with your workaround. Changing getDataValue with dataValues has no effect at all. I’m sorry, but I, as I said, I was wrong.

So here is any information needed to see, understand and (hopefully) reproduce the problem.

SQL, models and environment background

Here are all the relevant information needed to reproduce the problem.

Versions

From package.json:

  • "sequelize": "^5.22.3",
  • "forest-express-sequelize": "6.6.3",

Postgres: 12

SQL Entities and relationships

CREATE TABLE public.main (
	id integer NULL,
	amount numeric(5,2) NULL,
	created_at timestamptz(0) NULL
	updated_at timestamptz(0) NULL
	CONSTRAINT newtable_pk PRIMARY KEY (id)
);

CREATE TABLE public.related (
	id integer NULL,
	id_main integer NULL,
	some_data varchar NULL,
	created_at timestamptz(0) NULL
	updated_at timestamptz(0) NULL
	CONSTRAINT related_pk PRIMARY KEY (id),
	CONSTRAINT related_fk FOREIGN KEY (id_main) REFERENCES public.main(id) ON DELETE CASCADE ON UPDATE CASCADE
);

Sequelize

MainItem fields definition:
        id: {
          allowNull: false,
          primaryKey: true,
          type: DataTypes.INTEGER,
        },
        amount: {
          // C
          type: DataTypes.DECIMAL(5, 2),
          get() {
            // Workaround until sequelize issue #8019 is fixed
            const value = this.dataValues.amount
            return value === null ? null : parseFloat(value)
          },
          field: 'amount',
          allowNull: false,
        },
        createdAt: {
          allowNull: true,
          type: DataTypes.DATE,
          field: 'created_at',
        },
        updatedAt: {
          allowNull: true,
          type: DataTypes.DATE,
          field: 'updated_at',
        },
RelatedItem fields definition
        id: {
          allowNull: false,
          primaryKey: true,
          type: DataTypes.INTEGER,
        },
        idMain: {
          allowNull: true,
          primaryKey: false,
          field: 'id_main',
          type: DataTypes.INTEGER,
        },
        someData: {
          allowNull: true,
          field: 'some_data',
          type: DataTypes.STRING,
        },
        createdAt: {
          allowNull: true,
          type: DataTypes.DATE,
          field: 'created_at',
        },
        updatedAt: {
          allowNull: true,
          type: DataTypes.DATE,
          field: 'updated_at',
        },
      },
Associations (accordingly to SQL relationships):
    MainItem.hasMany(models.RelatedItem, {
      foreignKey: 'idMain',
      sourceKey: 'id',
    })

and

    RelatedItem.belongsTo(MainItem, {
      foreignKey: 'id_main',
      target: 'id',
    })

How to spot the problem

See the video in the previous post (yeah, it’s related to different collections, but… you get the very same behaviour).
As soon as the summary view (with a “related items” table) is loaded, the amount field disappears with a “flicker”.
When you remove the “related items” table from the summary view the problem is not there any more.

My hypothesis about the problem

  • The “related items” table loads all the related records
  • each related record re-loads the main record through the “belongs to” relationship
  • but the instance of the main record (in this case) only contains the id, then the value of this.dataValues.amount is undefined.
  • but the workaround suggested in the sequelize issue #8019 returns NaN when the value is undefined. Because parseInt(undefined) === NaN.
  • I think that in the Forest front end, when a new value in the main model is re-loaded by the records in the “related items” table, the value of the main model is replaced if the value is not undefined. And somehow NaN is mapped to null, so a dash is shown (with a flicker).

test to see if my hypotesis is right

Change the getter of the amount field as follows:

          get() {
            console.log('amount', this.dataValues.amount ? this.dataValues.amount : this.dataValues)
            // Workaround until sequelize issue #8019 is fixed
            const value = this.dataValues.amount
            return value === undefined || value === null ? value : parseFloat(value)
          },

Now, actually we have no more flickers on the amount field.

And this is the relevant log (with comments):

OPTIONS /forest/MainItem/1 200 1.396 ms - 0
amount 12.32
amount 12.32
amount 12.32
amount 12.32    <--- why is this logged 4 times???? But it is not relevant to this problem
GET /forest/MainItem/1 304 25.435 ms - -
OPTIONS /forest/MainItem/1/relationships/RelatedItems?fields%5BRelatedItem%5D=MainItem%2CcreatedAt%2Cid%2CsomeData%2CupdatedAt&fields%5BMainItem%5D=id&page%5Bnumber%5D=1&page%5Bsize%5D=15&timezone=Europe%2FRome&sort=-id 200 3.683 ms - 0
amount { id: 1 } <--- Here it is! No amount field. I think this is being loaded by the _"related items"_ table record
amount { id: 1 } <--- Why twice? because in my test I have two entries in the _"related items"_ table :-)
GET /forest/MainItem/1/relationships/RelatedItems?fields%5BRelatedItem%5D=MainItem%2CcreatedAt%2Cid%2CsomeData%2CupdatedAt&fields%5BMainItem%5D=id&page%5Bnumber%5D=1&page%5Bsize%5D=15&timezone=Europe%2FRome&sort=-id 304 39.521 ms - -
OPTIONS /forest/MainItem/1/relationships/RelatedItems/count?fields%5BRelatedItem%5D=MainItem%2CcreatedAt%2Cid%2CsomeData%2CupdatedAt&fields%5BMainItem%5D=id&timezone=Europe%2FRome 200 2.620 ms - 0
GET /forest/MainItem/1/relationships/RelatedItems/count?fields%5BRelatedItem%5D=MainItem%2CcreatedAt%2Cid%2CsomeData%2CupdatedAt&fields%5BMainItem%5D=id&timezone=Europe%2FRome 304 29.998 ms - -

So this is a good workaround. But I think that in your front end you should not load the main record for each record in the “related items” table.
Btw, I’ll leave my getter manage the undefined value too, because it can be useful whenever a limited instance “scope” (I mean a sequelize “scope”) is used.

my question

May you please acknowledge my test and my explanations?
Don’t you think that the main record should not be re-loaded with the related data table?

Thank you i advance.

Hello @Matteo,

Thank you for your detailed message :slight_smile:
I tried to reproduce your issue, but had no success.

We will do a step by step processus for understanding what is going on here :slight_smile:
First of all, I can see in your associations sourceKey and target defined. Did you add it yourself?
It looks like you do not need them by default.

Here is what I used for trying to reproduce your issue:

Creating the psql definition.

create table if not exists number_test
(
	id serial not null constraint number_test_pk primary key,
	provider_commissions_percentage numeric(5,2) not null
);

create table if not exists related_to
(
	id serial not null constraint related_to_pk primary key,
	name varchar not null,
	number_id integer constraint related_to_number_test_id_fk references number_test
);

Here you can see your “main” table is the “number_test” table including a numeric type field.
Then the related table “related_to” pointing to “number_test” thanks to a foreign key on “number_id” field.

Here are the generated models from lumber (I added the psql workaround):

const NumberTest = sequelize.define('numberTest', {
    providerCommissionsPercentage: {
      type: DataTypes.DECIMAL(5, 2),
      get() {
        // Workaround until sequelize issue #8019 is fixed see https://github.com/sequelize/sequelize/issues/8019#issuecomment-523265759
        const value = this.getDataValue('providerCommissionsPercentage')
        return value === null ? null : parseFloat(value);
      },
      field: 'provider_commissions_percentage',
      allowNull: false,
    },
  }, {
    tableName: 'number_test',
    underscored: true,
    timestamps: false,
    schema: process.env.DATABASE_SCHEMA,
  });

and

const RelatedTo = sequelize.define('relatedTo', {
    name: {
      type: DataTypes.STRING,
      allowNull: false,
    },
  }, {
    tableName: 'related_to',
    timestamps: false,
    schema: process.env.DATABASE_SCHEMA,
  });

Here are the associations:

NumberTest.associate = (models) => {
    NumberTest.hasMany(models.relatedTo, {
      foreignKey: {
        name: 'numberIdKey',
        field: 'number_id',
      },
      as: 'numberRelatedTos',
    });
  };

RelatedTo.associate = (models) => {
    RelatedTo.belongsTo(models.numberTest, {
      foreignKey: {
        name: 'numberIdKey',
        field: 'number_id',
      },
      as: 'number',
    });
  };

As you can see, neither sourceKey nor target are needed in the associations declaration.

Back on the UI, I add in the summary view of the “number_test” collection a new section with field “provider_commissions_percentage” and another section with related_data to “numberRelatedTos”.
I do not experience any flicker and data is displayed correctly.

What am I missing?

Quick question to be sure: Is your foreign key pointing to a primary key and not on another integer field?

Thank you @Guillaume_Cisco
I understand this is some kind of unusual problem, and I appreciate very much your effort to help me out. But I’m afraid I can’t tell what you are missing. I can only say that if you can’t reproduce then the setup you did is different from mine :man_shrugging:

Now I understood it can be helpful if I provide the most precise setup steps based upon lumber cli.
So I’ll try to do my best to give you any fine grained instruction to let you reproduce the issue with lumber cli. Follow me, let’s go! :+1:

Database

Create a Postgres database (i used testsq001 as the database name). Then run the following two scripts in your database sql console:

Tables
CREATE TABLE public.main (
  id integer NULL,
  amount numeric(5,2) NULL,
  CONSTRAINT newtable_pk PRIMARY KEY (id)
);

CREATE TABLE public.related (
  id integer NULL,
  id_main integer NULL,
  some_data varchar NULL,
  CONSTRAINT related_pk PRIMARY KEY (id),
  CONSTRAINT related_fk FOREIGN KEY (id_main) REFERENCES public.main(id) ON DELETE CASCADE ON UPDATE CASCADE
);
Data
INSERT INTO public.main VALUES (1, 1.5);
INSERT INTO public.related VALUES (1, 1, 'one');
INSERT INTO public.related VALUES (2, 1, 'two');

Forest

I created a new forest project (in my setup I named it test-sq-001) and I followed the directions from the forest project setup page:

Now we have the project created from the lumber cli. In the project we have two model files: models/main.js and models/related.js.

Edit the file models/main.js (final resulting file below):

  • change the data type in the amount field (lumber cli maps sql numeric(5,2) to DOUBLE, but we need to use the DECIMAL(5,2)sequelize datatype) and
  • insert the (previously described) getter (with a useful console.log in it)

Here is the resulting models/main.js file:

// This model was generated by Lumber. However, you remain in control of your models.
// Learn how here: https://docs.forestadmin.com/documentation/v/v6/reference-guide/models/enrich-your-models
module.exports = (sequelize, DataTypes) => {
  const { Sequelize } = sequelize;
  // This section contains the fields of your model, mapped to your table's columns.
  // Learn more here: https://docs.forestadmin.com/documentation/v/v6/reference-guide/models/enrich-your-models#declaring-a-new-field-in-a-model
  const Main = sequelize.define('main', {
    amount: {
      // =============================================================
      // I commented out the following line
      // type: DataTypes.DOUBLE,
      //
      // and I added the following lines:
      type: DataTypes.DECIMAL(5,2),
      get() {
        console.log('amount', this.dataValues.amount ? this.dataValues.amount : this.dataValues)
        // Workaround until sequelize issue #8019 is fixed
        const value = this.dataValues.amount
        return value === null ? value : parseFloat(value)
      },
      // =============================================================
    },
  }, {
    tableName: 'main',
    timestamps: false,
    schema: process.env.DATABASE_SCHEMA,
  });

  // This section contains the relationships for this model. See: https://docs.forestadmin.com/documentation/v/v6/reference-guide/relationships#adding-relationships.
  Main.associate = (models) => {
    Main.hasMany(models.related, {
      foreignKey: {
        name: 'idMainKey',
        field: 'id_main',
      },
      as: 'idMainRelateds',
    });
  };

  return Main;
};

Nothing else was changed in the lumber generated project.

Let’s run npm run and you should be able to see the problem!!!

Evidence

This is a video showing what you are supposed to see:

And this is the (relevant) log:

amount 1.50
amount 1.50
amount 1.50
amount 1.50
GET /forest/main/1 304 - - 5.744 ms
amount { id: 1 }
amount { id: 1 }
amount { id: 1 }
amount { id: 1 }
GET /forest/main/1/relationships/idMainRelateds?fields%5Brelated%5D=id%2CidMain%2CsomeData&fields%5BidMain%5D=id&page%5Bnumber%5D=1&page%5Bsize%5D=15&timezone=Europe%2FRome&sort=-id 304 - - 9.386 ms
GET /forest/main/1/relationships/idMainRelateds/count?fields%5Brelated%5D=id%2CidMain%2CsomeData&fields%5BidMain%5D=id&timezone=Europe%2FRome 304 - - 6.219 ms

The console.log was hit 8 times while loading the summary page for the main record:

  • in the first 4 times (GET /forest/main/1) the this.dataValues.amount was correctly retrieved (1.5)
  • in the second 4 times (due to thee call GET /forest/main/1/relationships/idMainRelateds?.....) the this.dataValues.amount was undefined and the dataValues object was shown (no amount field there).

(why four + four times??? :thinking: can’t tell. But maybe you could explain?)

Possible workaround (and evidence of problem understanding)

Now, if I use the workaround described here below, everything works fine.
This is the workaround getter:

      get() {
        console.log('amount', this.dataValues.amount ? this.dataValues.amount : this.dataValues)
        // Workaround until sequelize issue #8019 is fixed
        const value = this.dataValues.amount
        return value === undefined || value === null ? value : parseFloat(value)
      },

With this workaround we let pass the undefined value as is (please, se my previous post for a better explanation).

Exclude this is due to the datatype

I eventually tried also restoring the DOUBLE sequelize datatype in the lumber generated model. This is the default “translation” by lumber cli.

I did this test to collect evidence that this problem is not directly due to the adoption of the DECIMAL sequelize datatype.
And nothing changes: we have this problem with the DOUBLE datatype too.

Conclusions and questions

Did you reproduce? :pray:

Questions:

  • May you please acknowledge my test and my explanations?
  • May you please better explain why this happens?
  • Don’t you think that the main record should not be re-loaded with the related data table?
  • Why the getter is called multiple (4) times?

Versions doublecheck

This is the package.json created by lumber cli:

{
  "name": "test-sq-001",
  "version": "0.0.1",
  "private": true,
  "scripts": {
    "start": "node ./server.js"
  },
  "dependencies": {
    "body-parser": "1.19.0",
    "chalk": "~1.1.3",
    "cookie-parser": "1.4.4",
    "cors": "2.8.5",
    "debug": "~4.0.1",
    "dotenv": "~6.1.0",
    "express": "~4.17.1",
    "express-jwt": "5.3.1",
    "forest-express-sequelize": "^6.0.0",
    "morgan": "1.9.1",
    "require-all": "^3.0.0",
    "sequelize": "~5.15.1",
    "pg": "~8.2.2"
  }
}

More versions on my side:

$ node -v && npm -v
v14.9.0
6.14.8

Thank you in advance.

Hello @Matteo :wave:

After a very deep investigation, I’ve finally been able to reproduce your exact issue.
Indeed the sequelize get override is a condition for making it happens.
The datatype has nothing to do with it, I can confirm it.

What is going on here is that the section with the amount data is loaded correctly with a request from the frontend. Then a request is made for displaying the related data, BUT the related data only ask for the id of the referenced object. The SQL request won’t ask for more data than it needs. And unfortunately, after getting the result, the main object is updated with a NaN value on its amount field.
Sending a NaN value is the cause of the issue.
Replacing the code:

return value === null ? value : parseFloat(value)

with

return typeof value !== 'undefined' ? parseFloat(value) : undefined;

won’t trigger an update of the value of “amount” and then no flickering will appear.
It will work correctly. :tada:

Thank you for finding this one. It is a tough situation.

The reason why I could not reproduce in the first place is because I set the reference field of the “main” collection to “amount” and not to the default “id”.

If you do this, the related data request will call also for the “amount” data, and you will see no flickering. This is a workaround, although not ideal.

The best is not to update the value of a field if it is undefined in its context.
As we could see here, the value can be set to NaN and trigger an update.

I hope this explanation satisfies you.
Let me know if you want more details :slight_smile:

1 Like

Thank you @Guillaume_Cisco.
I’m pleased to understand that what we’re doing is correct. Because in this kind of circumstances I’m always concerned about the risk that something is out of my control. And, seen that we’re using Forest into our project (I mean that our project is not “lumber generated”), we always want to be sure that there’s not something wrong on our side, preventing Forest to work as expected.

So thank you for your explanations.

As a side note, considering that the getters are something quite usual in large projects, I’d suggest to mention this in the docs. The point is that since forest-expres-sequelize v6.3.7 the developers must “respect” (and let pass) the possible undefined value in their getters. Before v6.3.7 all the record was loaded there was not any “flicker” problem with the getters.

So the final solution for the getters is something like:

return typeof value === 'undefined' ? value : doWhateverYouWantWith(value)

Thank you.

But, if you don’t mind, I’d also like to understand why not avoiding at all this additional request. I mean, the main dataset have already been requested, then the related data was requested too… why do we need additional (multiple) requests to get again the main data set while loading any related record?

I’d also like to explain why the getter is called 4 + 4 times (see my logs above).

Thank you for any additional explanation and for your support!
Matteo