dmfay / massive-js

A data mapper for Node.js and PostgreSQL.
2.49k stars 158 forks source link

Further detail on Massive 5.0.0 breaking changes #605

Closed bmoquist closed 6 years ago

bmoquist commented 6 years ago

Hello - I started upgrading to 5.0.0 and found the some of the notes in the breaking changes difficult to follow. Specifically, I encountered the following error:

Update requires a hash of fields=>values to update to
       at Writable.update (.../node_modules/massive/lib/writable.js:102:52)

when calling both the following functions:

  req.app.get(constants.db).users.findOne({user_id: userId})
  req.app.get(constants.db).users.update({
      user_id: req.params.userId,
      last_login: currentTimestamp
  })

I added {} as the options object, and this appeared to resolve the issue:

  req.app.get(constants.db).users.findOne({user_id: userId}, {})
  req.app.get(constants.db).users.update({
      user_id: req.params.userId,
      last_login: currentTimestamp
    }, {})

Is this how I should be handling the change assuming that I want the record with all the fields returned after execution? Also, is there a particular reason why the old syntax was removed as it seems cleaner?

In the breaking changes section of the Change Log, could you provide further explanation or a brief example of the difference for the following bullets:

-- empty options.fields is now recognized as an error instead of falling back to '*'
      - is this the breaking change that is impacting me above since I didn't include any options at all?
-- update() now requires separate criteria and changes, use save() to update record objects
    - what does 'separate criteria and changes' mean? 
    - sometimes, I use update instead of save to update record objects because I prefer an error to be thrown if the record object does not already exist where save would perform an insertion.  
-- field should now be specified with options.body
    - I'm not sure what this means or the context

Thanks very much for your help, Bryant

dmfay commented 6 years ago

update takes a minimum of two arguments now: criteria to find rows you want to change, and the column:value changes to make. The single-object version with a primary key and fields to update duplicated the functionality of save and made options object usage ambiguous (is this attempting to modify a column named single or is it an options object with a field single? that kind of thing). Your new update call is not modifying any fields at all since the changes object is empty.

I'm not following your findOne case; that error message is specific to the update method.

The rest:

  1. If your options object specifies fields: [...], you have to actually pass fields; if the array is empty, it's taken to be an error. If you don't specify fields in your options object or don't add options at all, no harm no foul.
  2. You can use the binary update for that, just pass a primary key or a criteria object like {id: 1234} as the first argument. It should emit (almost) exactly the same SQL and there's no chance of the ambiguity I mentioned above. The record will be returned in an array unless you pass options.single.
  3. This only matters if you're using updateDoc with a JSON field that isn't named body. You used to have to specify it as an additional argument, but that's been moved into the options object. The documentation reflects this but it came out a bit awkwardly in the changelog.

In general I'd recommend giving the docs a once-over if something throws you -- there's quite a bit that's different, and the changelog is necessarily a summary rather than a full accounting.

bmoquist commented 6 years ago

Thanks so much for the clear explanation. The inclusion of findOne was a mistake on my part.

For the benefit of others, here's an example change made to my original function. As pointed out, the data fields being changed need to move to the second argument. Adding the single option will return the first matching record instead of an array.

const criteria = {
   user_id: userId
}

const changes = {
   last_login: currentTimestamp
}

const options = {
   single: true
} 

req.app.get(constants.db).users.update(criteria, changes, options)

Thanks again!