Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not a true upsert? #10

Closed
bristles5 opened this issue Sep 11, 2019 · 6 comments
Closed

Not a true upsert? #10

bristles5 opened this issue Sep 11, 2019 · 6 comments

Comments

@bristles5
Copy link

bristles5 commented Sep 11, 2019

I have a model that has default values.

employeeId: {
        type: String,
        required: true
    },
    isInvited: {
        type: Boolean,
        default: false
    },
    isNominee: {
        type: Boolean,
        default: false
    },
    isAttendee: {
        type: Boolean,
        default: false
    }

Upon initial upsertMany it creates a document with

{
 employeeId: 1,
 isInvited: true
}

If i want to upsertMany again only make the isNominee true,

{
 employeeId: 1,
isNominee: true
}

The modelToObject makes an object like

{
 employeeId:1,
 isInvited: false,
 isNominee:  true,
 isAttendee: false
}

This is incorrect behavior because I want to maintain the isInvited being true from the initial create. Any idea how to resolve this issue? The same thing happens for extra properties that may exist on initial create but are omitted from the upsert. If the object passed in does not contain the property then the upsert removes that property when it creates the new model from the modelToObject.
ex...
Initial Object in DB:

{
 shape: 'square',
color: 'Yellow',
text: 'Here'
}

OBJ to be updated:

{
  shape: 'square',
 color: 'Blue'
}

When your function is called the replaced document did not keep the text property from the original document

@adamreisnz
Copy link
Owner

Hi there, could you provide a standalone Node script that reproduces this behaviour? This will make it easier for me to analyse the problem and implement a fix.

@z0marlin
Copy link

Any updates on this? Facing the same issue.

@adamreisnz
Copy link
Owner

As per the last comment. Could you provide a standalone Node script that reproduces this behaviour? This will make it easier for me to analyse the problem and implement a fix

@z0marlin
Copy link

z0marlin commented May 14, 2020

Sorry for the late reply. Here is a standalone script.

const mongoose = require('mongoose');
const mup = require('@meanie/mongoose-upsert-many');
const { Schema } = mongoose;

const demoSchema = new Schema({
  field1: String,
  field2: {
    default: 'hello',
    type: String
  },
  field3: Number
});

demoSchema.plugin(mup);

const demoModel = mongoose.model('Demo', demoSchema);

async function demonstrateIssue() {
  // Connection part skipped

  // Sample doc. Note that the value of field2 is explicitly provided.
  var newDoc = new demoModel({
    field1: "abc",
    field2: "xyz",
    field3: 11
  });
  await newDoc.save();

  // items to be updated.
  const items = [{
    field1: "abc",
    field3: 5
  }];
  const mf = ["field1"];

  // Expected behaviour - Update field1 and field3 of all
  // documents using field1 as the matching criteria.
  const res = await demoModel.upsertMany(items, mf);
}

@adamreisnz
Copy link
Owner

Ok I see what you mean now, I have asked on the progress of Automattic/mongoose#8271 which will resolve this issue once it lands.

@adamreisnz
Copy link
Owner

adamreisnz commented Jun 6, 2020

Fixed for now, allowing you to bypass the model conversion using the schema setting upsertEnsureModel: false. The default of this setting will remain true, because sometimes you want to have default values, like a timestamp.

I've also added a upsertNoDefaults config setting, which will only be available once Automattic/mongoose#8271 is implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants