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

save command is still written to db, even though subsequent save fails #18

Closed
rushabhnagda11 opened this issue Aug 28, 2017 · 6 comments

Comments

@rushabhnagda11
Copy link

rushabhnagda11 commented Aug 28, 2017

Both are mongoose schemas.

task.save(Product,product.product) .save(ProductFinals,_.assign(product.productFinal, {product_id : { $ojFuture : "0._id"}}))

The data in the first command is fine.
Data for the second save command has a validation error (intentional).

 ` task.run({useMongoose : true})
  .then(() => {
    res.sendStatus(200)
  })
  .catch((err) => {
    console.log(JSON.stringify(err))
  })`

Expeceted behavior : Everything is rolled back
Observed Behavior : Goes to the error block of the run command, but the first save is committed to the db.

@rushabhnagda11 rushabhnagda11 changed the title Doing 2 saves, with 1 error, 1 write is still committed save command is still written to db, even though subsequent save fails Aug 28, 2017
@e-oj
Copy link
Owner

e-oj commented Aug 28, 2017

can you show all the relevant code or give more information? I haven't been able to reproduce this issue.

@e-oj
Copy link
Owner

e-oj commented Aug 28, 2017

It could also be helpful if you showed what the db looked like before and after the run function was called.

@rushabhnagda11
Copy link
Author

rushabhnagda11 commented Aug 30, 2017

Figured the issue. modelName is being used in rollBackTask instead of collectionName in roller. But mongoose pluralizes model names (i.e Product becomes products (note the lower case))

Also, I've not quite got the use of the iife in rollBackTask. Can't a let step = "" be used?

I can give a PR for this

@e-oj
Copy link
Owner

e-oj commented Aug 30, 2017

"let step" in place of iife will work but I don't want to mix different javascript versions. I might eventually refactor the whole library to use es6.

@e-oj
Copy link
Owner

e-oj commented Aug 30, 2017

Feel free to open a pr. They're always welcome 👍

@e-oj e-oj added the bug label Aug 30, 2017
e-oj pushed a commit that referenced this issue Sep 3, 2017
This was referenced Sep 3, 2017
Merged
@e-oj
Copy link
Owner

e-oj commented Sep 3, 2017

@rushabhnagda11 I've actually added this fix as part of the new release. Update your Fawn version and things should work fine. Feel free to reopen this issue if it persists. Thanks for your help 👍 👍

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

No branches or pull requests

2 participants