feathers-sequelize icon indicating copy to clipboard operation
feathers-sequelize copied to clipboard

Use proper $and in patch and remove methods

Open DaddyWarbucks opened this issue 3 years ago • 2 comments

@fratzinger Made an excellent PR recently that ensures the user's $and query is not overwritten. See https://github.com/feathersjs-ecosystem/feathers-sequelize/commit/8de01449511b1bc1a301d077b72e4af67b0f95d1

We should similarly update the _patch and _remove methods to do so. Note that this issue should probably be accomplished first: https://github.com/feathersjs-ecosystem/feathers-sequelize/issues/382. Right now the _patch does not use and $and query, but if we update it to work more similarly to _remove then it will use an $and query I believe.

DaddyWarbucks avatar Jan 09 '22 18:01 DaddyWarbucks

Also, we likely need to do some more type checking on the $and. Note in this line that we assume the $and is an array. But the code also potentially returns an object instead of an array. https://github.com/feathersjs-ecosystem/feathers-sequelize/blob/db4b5ff9b89df9b163f9cd5d8c32de8c915ff71c/lib/index.js#L169 So if $and works with an object and not just an array, when we need to check that. I think it is most common that its an array so we should also return an array.

Something like

applyAndQuery(where, id) {
  const { and } = this.Op;
  const IdQuery = { [this.id]: id };
  const newWhere = Object.assign({}, where);

  if (!newWhere[and]) {
    newWhere[and] = [IdQuery]
    return newWhere;
  }

  if (Array.isArray(newWhere[and])) {
    newWhere[and] = [...newWhere[and], idQuery];
    return newWhere;
  }

  newWhere[and] = [newWhere[and], idQuery];
  return newWhere;
}

DaddyWarbucks avatar Jan 09 '22 19:01 DaddyWarbucks

Good catch! We should add a test for that. Maybe even on @feathersjs/adapter-tests and we could add $and to the official supported operators list at https://docs.feathersjs.com/api/databases/querying.html.

Op.and and Op.or can be used as an object, indeed (see https://sequelize.org/v6/manual/model-querying-basics.html#examples-with--code-op-and--code--and--code-op-or--code-). But I believe at the root level they're always meant to be arrays. It makes sense if they're nested within a column query like:

where: {
  rank: {
    [Op.and]: {
      [Op.lt]: 1000,
      [Op.eq]: null
    }
  },
}

fratzinger avatar Jan 09 '22 19:01 fratzinger

resolved by #429

fratzinger avatar May 12 '24 16:05 fratzinger