Save documents

[yamila-fraiman]

Is it possible to support saving existing documents. For example

db.User.findById('5997a409df4cc84e8ca3a8de').exec((err, user) => {
    lodash.assign(user, req.body);
    let transaction = Fawn.Task();
    transaction.save(user);
    transaction.update(....)
    transaction.run({useMongoose: true})
});

Actually if I try this code, I receive duplicated id because creates a new collection with same id

[e-oj]

The operation you described is an update so you should be using the update function

db.User.findById('5997a409df4cc84e8ca3a8de').exec((err, user) => {
    lodash.assign(user, req.body);
    let transaction = Fawn.Task();
    
    // update existing user
    transaction.update({_id: '5997a409df4cc84e8ca3a8de'}, user);
    transaction.update(....)
    transaction.run({useMongoose: true})
});

[yamila-fraiman]

Thank you for your answer. I know that it's an update but updates don't call pre save and if I use pre update I don't have access to the object so I'd like this feature

[e-oj]

I see your point. I'll tag this as an enhancement for the next release. Thanks for the suggestion.

[yamila-fraiman]

I think theres is a simple solution:

In goose.js inside function performSave, where you set doc._id, replace line by:

  if (doc._id) {
    doc.isNew = false;
  } else {
    doc._id = utils.generateId();
  }

It won't allow creating new documents by calling task.save(new Collection(data)). In order to solve that, maybe you can save a var isNew in the step which value is extracted from the collection parameter (step.options.isNew = collection.isNew;).

@e-oj do you think you can implement this feature?

[e-oj]

That's actually the plan but I'm implementing it as an extension of the update function to avoid confusion on how to rollback a save

[e-oj]

You'll be able to do it this way

db.User.findById('5997a409df4cc84e8ca3a8de').exec((err, user) => {
    let _user = lodash.assign({}, user, req.body);
    let transaction = Fawn.Task();
    
    // update existing user
    transaction.update(user, _user);
        .options({replace: true})
        .update(....)
        .run({useMongoose: true})
});

Will this work for you? @yamifr07

[yamila-fraiman]

No, because I use pre save. And I think it's a good idea to support saving existing documents. such as when I call collection.save in mongoose. It's not working with fawn because you serialize the collection and then create it without asking if is a new collection. So I think you need to save 'isNew' value before serializing and then set it to the Collection when restoring

[e-oj]

I know what you mean. this will be the underlying implementation for {replace: true}

if(step.options.replace){
  var doc = new Collection(data);
  doc.isNew = false;
  resultPromise = doc.save();
}
else{
  resultPromise = Collection.update(condition, data, step.options).exec();
}

return resultPromise.then(function (result) {
  results.push(result);

  return updateState(task, step.index, DONE, results);
});

[e-oj]

Is that different from what you want? @yamifr07

[yamila-fraiman]

mmm I think it's not a good idea to mix methods. I think that task.update should call update and task.save should call save. Otherwise users can have bugs without knowing.

[e-oj]

I don't think it's mixing methods because it's still an update; it's just an update implemented with a save function. There should be no confusion too, the replace option will be well documented. Also, It's for rollback convenience. Saves are rolled back by deleting; in this case though, what you're actually doing is an update. You don't want the whole updated document deleted during a rollback.

[yamila-fraiman]

Yes but update call update hooks and save call save hooks. And mongoose allows save and update to update elements. Why you don't want to use same methods as mongoose?

[e-oj]

I get your point. For convenience, and to reduce the learning curve, the functions in Fawn were modelled similarly to mongoose. However, they are two libraries with different goals. If a desired feature in fawn can be implemented in a way that deviates from the way mongoose is designed but makes development and usage of the library more intuitive, I have to go with that. As I said previously, saves are rolled back by deleting; in this case though, what you're actually doing is an update. You don't want the whole updated document deleted during a rollback. Check out the rollback code for saves

function rollbackSave(db, save, task) {
  var collection = db.collection(save.name);
  var _id = save.dataStore[0]._id;

  return collection.deleteOne({_id: _id}).then(function(){
    return updateState(task, save.index, ROLLED);
  });
}

and for updates

function rollbackRemoveOrUpdate(db, step, task) {
  var collection = db.collection(step.name);
  var chain = Promise.resolve();

  step.dataStore.forEach(function(data) {
    chain = chain.then(function() {
      var condition = {_id: data._id};

      return collection.findOne(condition).then(function (doc) {
        if (doc && step.type === UPDATE) {
          return collection.updateOne(condition, data);
        }
        else if (!doc && step.type === REMOVE) {
          return collection.insertOne(data);
        }

        return Promise.resolve();
      });
    });
  });

  return chain.then(function(){
    return updateState(task, step.index, ROLLED);
  });
}

Using {replace: true} (or a more descriptive option name) will require no changes to the code for rolling back both saves and updates. It also produces a more intuitive api for those kinds of updates IMHO.

[e-oj]

Also, Modifying the save function will require options because this

if (doc._id) {
  doc.isNew = false;
} else {
  doc._id = utils.generateId();
}

is sketchy for new documents with the _id field already set. So instead of adding options to save and rewriting that rollback function, I can extend the update options and not touch the rollback functions for the same effect