forest-express-sequelize [-] Smart Field Search - Ease the search on smart fields and deprecate old ways to do it

Pull Request checklist:

[x] Write an explicit title for the Pull Request
[x] Write changes made in the CHANGELOG.md
[ ] Create automatic tests
[ ] Test manually the implemented changes
[ ] Review my own code (indentation, syntax, style, simplicity, readability)
[ ] Wonder if you can improve the existing code

Before

We had to do this kind of thing

collection('customers', {
  fields: [{
    field: 'fullname',
    type: 'String',
    get: (customer) => {
      return customer.firstname + ' ' + customer.lastname;
    },
    search: function (query, search) {
      let s = models.sequelize;
      let split = search.split(' ');

      var searchCondition = {
        [Op.and]: [
          { firstname: { [Op.like]: `%${split[0]}%` } },
          { lastname: { [Op.like]: `%${split[1]}%` } },
        ]
      };

      query.where[Op.and][0][Op.or].push(searchCondition);

      return query;
    }
  }]
});

After

collection('customers', {
  fields: [{
    field: 'fullname',
    type: 'String',
    get: (customer) => {
      return customer.firstname + ' ' + customer.lastname;
    },
    search: function (query, search) {
      let s = models.sequelize;
      let split = search.split(' ');

      return {
        [Op.and]: [
          { firstname: { [Op.like]: `%${split[0]}%` } },
          { lastname: { [Op.like]: `%${split[1]}%` } },
        ]
      };
    }
  }]
});

Feb 03 '20 16:02 VincentMolinie

Pull Request checklist:

[x] Write an explicit title for the Pull Request
[x] Write changes made in the CHANGELOG.md
[ ] Create automatic tests
[ ] Test manually the implemented changes
[ ] Review my own code (indentation, syntax, style, simplicity, readability)
[ ] Wonder if you can improve the existing code

Before

We had to do this kind of thing

collection('customers', {
  fields: [{
    field: 'fullname',
    type: 'String',
    get: (customer) => {
      return customer.firstname + ' ' + customer.lastname;
    },
    search: function (query, search) {
      let s = models.sequelize;
      let split = search.split(' ');

      var searchCondition = {
        [Op.and]: [
          { firstname: { [Op.like]: `%${split[0]}%` } },
          { lastname: { [Op.like]: `%${split[1]}%` } },
        ]
      };

      query.where[Op.and][0][Op.or].push(searchCondition);

      return query;
    }
  }]
});

After

collection('customers', {
  fields: [{
    field: 'fullname',
    type: 'String',
    get: (customer) => {
      return customer.firstname + ' ' + customer.lastname;
    },
    search: function (query, search) {
      let s = models.sequelize;
      let split = search.split(' ');

      return {
        [Op.and]: [
          { firstname: { [Op.like]: `%${split[0]}%` } },
          { lastname: { [Op.like]: `%${split[1]}%` } },
        ]
      };
    }
  }]
});

So you can't use include to join if necessary?

Feb 11 '20 16:02 arnaudbesnier

Pull Request checklist:

[x] Write an explicit title for the Pull Request
[x] Write changes made in the CHANGELOG.md
[ ] Create automatic tests
[ ] Test manually the implemented changes
[ ] Review my own code (indentation, syntax, style, simplicity, readability)
[ ] Wonder if you can improve the existing code

Before

We had to do this kind of thing

collection('customers', {
  fields: [{
    field: 'fullname',
    type: 'String',
    get: (customer) => {
      return customer.firstname + ' ' + customer.lastname;
    },
    search: function (query, search) {
      let s = models.sequelize;
      let split = search.split(' ');

      var searchCondition = {
        [Op.and]: [
          { firstname: { [Op.like]: `%${split[0]}%` } },
          { lastname: { [Op.like]: `%${split[1]}%` } },
        ]
      };

      query.where[Op.and][0][Op.or].push(searchCondition);

      return query;
    }
  }]
});

After

collection('customers', {
  fields: [{
    field: 'fullname',
    type: 'String',
    get: (customer) => {
      return customer.firstname + ' ' + customer.lastname;
    },
    search: function (query, search) {
      let s = models.sequelize;
      let split = search.split(' ');

      return {
        [Op.and]: [
          { firstname: { [Op.like]: `%${split[0]}%` } },
          { lastname: { [Op.like]: `%${split[1]}%` } },
        ]
      };
    }
  }]
});

So you can't use include to join if necessary?

You are absolutely right. I thought about it, but most of the time includes does not work as expected (For example try with a hasMany...).

I think we need to rework a little bit the smart field to add a requirement property for example That field could look like requirement: ['users.*', 'companies.{id,name}']...

Tell me what you think about it @arnaudbesnier

Feb 12 '20 08:02 VincentMolinie

Pull Request checklist:

[x] Write an explicit title for the Pull Request

[x] Write changes made in the CHANGELOG.md

[ ] Create automatic tests

[ ] Test manually the implemented changes

[ ] Review my own code (indentation, syntax, style, simplicity, readability)

[ ] Wonder if you can improve the existing code

Before

We had to do this kind of thing
collection('customers', {
  fields: [{
    field: 'fullname',
    type: 'String',
    get: (customer) => {
      return customer.firstname + ' ' + customer.lastname;
    },
    search: function (query, search) {
      let s = models.sequelize;
      let split = search.split(' ');

      var searchCondition = {
        [Op.and]: [
          { firstname: { [Op.like]: `%${split[0]}%` } },
          { lastname: { [Op.like]: `%${split[1]}%` } },
        ]
      };

      query.where[Op.and][0][Op.or].push(searchCondition);

      return query;
    }
  }]
});
After
collection('customers', {
  fields: [{
    field: 'fullname',
    type: 'String',
    get: (customer) => {
      return customer.firstname + ' ' + customer.lastname;
    },
    search: function (query, search) {
      let s = models.sequelize;
      let split = search.split(' ');

      return {
        [Op.and]: [
          { firstname: { [Op.like]: `%${split[0]}%` } },
          { lastname: { [Op.like]: `%${split[1]}%` } },
        ]
      };
    }
  }]
});
So you can't use include to join if necessary?
You are absolutely right. I thought about it, but most of the time includes does not work as expected (For example try with a hasMany...).

I think we need to rework a little bit the smart field to add a requirement property for example That field could look like requirement: ['users.*', 'companies.{id,name}']...

Tell me what you think about it @arnaudbesnier

I am not ok to remove flexibility here.

The day developers will update to the new (major?) version containing this code, they will potentially lose their Smart Field search feature (if they used include). There is no alternatives with the current code.

I am not fan of the requirement attribute idea as presented, because:

it is defined in the field but only impacts the search behaviour.
from my point of view, all this search configuration/implementation should be located in the search function.

Feb 19 '20 09:02 arnaudbesnier

Actually after thinking about it the include is useless as of right now the call is async so you can do your search on the other model if necessary and then do a id in [myListOfId] for example @arnaudbesnier

Feb 28 '20 13:02 VincentMolinie

Ok, I think this something could also have with the current implementation.

What is important is what is removed. With the new implementation, you won't have the ability to do it in a single SQL query anymore.

Mar 02 '20 07:03 arnaudbesnier

Ok, I think this something could also have with the current implementation.

What is important is what is removed. With the new implementation, you won't have the ability to do it in a single SQL query anymore.

yeah but like I said it won't work most of the time, because if you try to do it on many relationships it won't work. And belongsTo might work but not like you might expect...

Mar 17 '20 10:03 VincentMolinie