bookshelf-modelbase icon indicating copy to clipboard operation
bookshelf-modelbase copied to clipboard

Upsert not working when querying for multiple fields?

Open Saeris opened this issue 7 years ago • 5 comments

So I've been able to successfully use the upsert function to update data when querying for a single field, but when passing a has of multiple fields I get the following error:

Error: A model cannot be updated without a "where" clause or an idAttribute.

For example, this works:

return Models.Artist
      .upsert({ name: name }, { website: website })
      .then(model => model.toJSON())

Where Artist has 2 columns: name and website and I want to search existing artists by name, update their website field if the artist already exists, create if not found.

But this does not work:

return Models.Printing
        .upsert({ card: card, set: set, number: number }, { ...fields })
        .then(model =>  model.toJSON())

Where Printing has about a dozen fields (which I am using spread syntax here as the fields are irrelevant), and I want to search existing printings by card, set, and number, ie:

SELECT * FROM printing WHERE card = '3' AND set = '2' AND number = '5'

Of course this will work when a row does not already exist, because it will be created instead. The problem only arises when trying to update an existing row.

For reference, Printing does have an ID attribute, so if a selection is successfully made the result should have an ID attribute, so I don't know why that would cause the error.

I've tried re-writing upsert in different ways, such as:

upsert: function (selectData, updateData, options) {
  return this.findOne(selectData, extend(options, { require: false }))
    .bind(this)
    .then(function (model) {
      return model
        ? model.where(selectData).save(
          updateData,
          extend({ patch: true, method: 'update' }, options)
        )
        : this.create(
          extend(selectData, updateData),
          extend(options, { method: 'insert' })
        )
    });
}

To ensure that if there was not an ID attribute, the where condition was met as per the error message. But doing so results in a model being returned with only the selectData attributes.

I'm not sure what to do from here short of dropping down into Knex and trying to be more explicit in constructing my queries than the methods provided by Bookshelf. That seems unnecessarily complicated for what should be a simple problem.

Saeris avatar May 16 '17 16:05 Saeris

Yeah the way upsert is currently written doesn't allow for bulk upserts because it internally uses Model.findOne so you'll at most get one model back.

  1. First issue, the error: The error you're seeing sounds like it might be a bug. What is your id field? Is it auto-assigned or do you create ids on the fly? What options are you passing? Would be help if you could add a console.log(model) below line 159 in your-app/node-models/bookshelf-modelbase/lib/index.js

  2. Second issue, writing a method that works for bulk upserts (please excuse types)

bulkUpsert: function (selectData, updateData, options) {
  return this.findAll(selectData, extend(options, { require: false }))
  .bind(this)
  .then(function (collection) { 
    return collection.length
      ? collection.invokeThen('save', updateData, extend({ patch: true, method: 'update' }, options)
      : this.create(
        extend(selectData, updateData),
        extend(options, { method: 'insert' })
      )
  })
})

bsiddiqui avatar May 16 '17 22:05 bsiddiqui

It's not a bulk insert though. I want to insert one row, or update one if there is one row that matches 3 fields.

Just to clear up any misunderstanding, all I'm trying to do is create a where clause with 3 parameters.

As for IDs, each model just has a bigIncrements field with the name 'id', which I realize is probably not the default that Bookshelf is expecting, but I do have it set as the primary key. I'm not sure if thee is something else I need to set to specify that it's the ID attribute to bookshelf.

Saeris avatar May 16 '17 22:05 Saeris

@Saeris if you want to update if there's only 1 row, then you can change the fn I sent you to check if collection.length === 1

Bookshelf's fetch fn return the first match so that won't guarantee that there was only one match

bsiddiqui avatar May 16 '17 22:05 bsiddiqui

As I mentioned earlier, need some more info if you're still having an issue with ModelBase.upsert, which finds the first match and updates it or creates

bsiddiqui avatar May 16 '17 22:05 bsiddiqui

I can try to put together an example for you this evening, but for now you can just refer to my project to see what's going on:

https://github.com/Saeris/Scribe

I've had upsert work for me in one of two ways:

  1. For selectData, I pass a single field (ie: { name: "string" }), for which it returns the first result, which as you point out is what it should do.

  2. For selectData and updataData I pass in every field, which either matches the first result, but more often just creates an entirely new row.

According to your description of how findOne works, it should return to me the first match for the hash of fields I pass to it. Because I never want to have duplicate rows in this case, where what I have here is basically a composite key of card set, and number, that key should be unique in that table. I should never have to deal with collections because of that, and ideally I would like to avoid doing so.

Right now upsert works fine for most of my other tables, because most of them are uniquely named. I'm leaving out the context as that's irrelevant to the issue, but you can gather that from looking at the project I linked above.

Let me know if that helps your understanding, and I'll get back to you with more info.

Saeris avatar May 16 '17 23:05 Saeris