Support updates that filter on multiple discriminator keys with `$in`

[thernstig]

Do you want to request a feature or report a bug? Bug

What is the current behavior? Using updateOne() on discriminated models properties does not work. Here is a repro script.

If the current behavior is a bug, please provide the steps to reproduce.

const testSchema = new mongoose.Schema(
  {
    title: { type: String, required: true },
    subtitle: String,
    kind: { type: String, required: true }
  },
  { timestamps: true, discriminatorKey: 'kind' }
);

const Test = mongoose.model('Test', testSchema);

const testSchemaChild = new mongoose.Schema({
  label: String
});

const TestChild = Test.discriminator('TestChild', testSchemaChild, 'testchild');

(async () => {
  await mongoose.dropDatabase();

  try {
    await Test.create({
      title: 'Title 1',
      subtitle: 'subtitle 1',
      label: 'label 1',
      kind: 'testchild'
    });
    await Test.create({
      title: 'Title 2',
      subtitle: 'subtitle 2',
      kind: 'testchild'
    });

    let test = await Test.find();
    console.log(JSON.stringify(test, null, 4));
    const doc = await Test.updateOne(
      { label: 'label 1' },
      { label: 'NEW LABEL' }
    );
    console.log(JSON.stringify(doc, null, 4));

    test = await Test.find();
    console.log(JSON.stringify(test, null, 4));
  } catch (error) {
    console.log(`${error}`);
  }
  console.log(`END`);
  process.exit(0);
})();

Output

[
    {
        "kind": "testchild",
        "_id": "5ceb9e2c5449f12f0707231f",
        "title": "Title 1",
        "subtitle": "subtitle 1",
        "label": "label 1",
        "createdAt": "2019-05-27T08:22:04.478Z",
        "updatedAt": "2019-05-27T08:22:04.478Z",
        "__v": 0
    },
    {
        "kind": "testchild",
        "_id": "5ceb9e2c5449f12f07072320",
        "title": "Title 2",
        "subtitle": "subtitle 2",
        "createdAt": "2019-05-27T08:22:04.498Z",
        "updatedAt": "2019-05-27T08:22:04.498Z",
        "__v": 0
    }
]
{
    "n": 1,
    "nModified": 1,
    "ok": 1
}
[
    {
        "kind": "testchild",
        "_id": "5ceb9e2c5449f12f0707231f",
        "title": "Title 1",
        "subtitle": "subtitle 1",
        "label": "label 1",
        "createdAt": "2019-05-27T08:22:04.478Z",
        "updatedAt": "2019-05-27T08:22:04.509Z",
        "__v": 0
    },
    {
        "kind": "testchild",
        "_id": "5ceb9e2c5449f12f07072320",
        "title": "Title 2",
        "subtitle": "subtitle 2",
        "createdAt": "2019-05-27T08:22:04.498Z",
        "updatedAt": "2019-05-27T08:22:04.498Z",
        "__v": 0
    }
]
END

What is the expected behavior? I expected it to update label to the new label. Note that if you change the update to e.g. this, it works:

    const doc = await Test.updateOne(
      { title: 'Title 1' },
      { title: 'NEW' }
    );

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version. Moongose 5.5.2 Mongodb 3.6.3 Node.js 10.13.0

[thernstig]

A question in regards to this. As can be seen, the the __v is not updated with an updateOne(), but mongoose is able to handle the timestamp. As it is able to handle the timestamp, would it also not be possible to update the version? Or is this another bug?

[vkarpov15]

@thernstig re: __v, that's intentional behavior right now. Versioning is meant to work with save(), if you use Model.updateOne() you're on your own for versioning.

And your original bug report is also expected behavior. When you do

await Test.updateOne(
      { label: 'label 1' },
      { label: 'NEW LABEL' }
    );

Mongoose has no way of knowing what type label is. It could hypothetically guess by looking at the Test model's discriminators, but what happens if multiple discriminators define different types for label? Instead, you should do:

const doc = await TestChild.updateOne(
      { label: 'label 1' },
      { label: 'NEW LABEL' }
    );

[thernstig]

Is this supposed to work then?

const doc = await Test.updateOne(
      { label: 'label 1', kind: 'testchild'},
      { label: 'NEW LABEL' }
    );

[vkarpov15]

@thernstig we'll add support for that, that doesn't work currently.

[nhitchins]

Another issue with the above which is directly related to #6087 is that you can not update the discriminatorKey value via the discriminator model when doing findByIdAndUpdate

This updates the discriminator key but not the label attribute

const doc = await Test.findByIdAnUpdate(
      '5dd4c54187e4851735ac00e6',
      { label: 'label 1', kind: 'testchild' }
    );

This only works when the existing doc has kind set to 'testchild'

const doc = await TestChild.findByIdAndUpdate(
      '5dd4c54187e4851735ac00e6',
      { label: 'NEW LABEL', kind: 'testchild' }
    );

This means that you can not change both the discriminatorKey value and set attributes based on the new discriminator model in a single update. Possibly a good reason against attaching discriminator keys to queries automatically?

[vkarpov15]

@nhitchins I'm not sure how your comments are related to this issue, can you please clarify?

[Marsup]

Hi,

Sorry for reviving an old issue, regarding thernstig's last comment, is there a way to do it for multiple types like so?

const doc = await Test.updateMany(
      { label: 'label 1', kind: { $in: ['testchild1', 'testchild2'] } },
      { label: 'NEW LABEL' }
    );

[vkarpov15]

@Marsup unfortunately not currently. You would need to do two separate updateMany() calls.

await Test.updateMany(
      { label: 'label 1', kind: 'testchild1' },
      { label: 'NEW LABEL' }
    );
await Test.updateMany(
      { label: 'label 1', kind: 'testchild2' },
      { label: 'NEW LABEL' }
    );

[piyushk96]

@vkarpov15 multiple calls would not be good if we have many values for kind. It will be great if $in is supported

[vkarpov15]

@piyushk96 the only workaround we have right now would be to use strict: false as follows

const doc = await Test.updateMany(
      { label: 'label 1', kind: { $in: ['testchild1', 'testchild2'] } },
      { label: 'NEW LABEL' },
      { strict: false }
    );

That skips type checking for the label in the update.

Another potential alternative would be to create a separate model with just the testchild1 and testchild2 discriminators. So a model that always has label.