Undestroy method for paranoid models

[seth-admittedly]

Undestroy allows for restoring destroyed paranoid models. Mirrors the syntax of destroy, with both model (bulk) and instance options.

Implementing this without resorting to manual SQL required tweaking the query generator to allow for omitNull to be passed as an option with an individual query.

[janmeier]

I think we should fail early if deletedAt is not define (before hooks)

[janmeier]

On of the tests is failing under pg.

Other than that, just some small comments, please ask if you have problems with promise style in tests

[mickhansen]

Might restore be a better name for undestroy?

[seth-admittedly]

@janmeier Do those promise tests look right? Don't have much experience with them.

[seth-admittedly]

@mickhansen Agreed

[seth-admittedly]

@janmeier Is it better to return a Promise.reject or throw a error synchronously?

[cusspvz]

@seth-admittedly usually they prefer an error throw :)

+1, thanks for the PR!

[cusspvz]

Usually they prefer an error throw since you should always call you methods right.

An error should be triggered into the promise if isn't at your handling scope. :)

[janmeier]

@seth-admittedly I'd prefer promise tests that are not so nested. So rather this:

something.then(function () {
  return somethingelse()
}).then(function () {
  return anotherPromise();
})

Than

something.then(function () {
  return somethingelse().then(function () {
    return anotherPromise();
  })
})

But it's fine for now :)

[janmeier]

As @cusspvz we usually just throw the error synchronously, since the error is not because of a DB error or something other async thing, but because of a programming error from the user.

You can also change the assertion to

expect(self.User.restore({where : {username : "Peter"}})).to.throw(Error, 'Model is not paranoid')

To avoid the awkvard expect false to be true in the succes handler

[seth-admittedly]

The only way I could get the throw to test correct was with the kludgey looking try/catch. Otherwise the throw seemed to interfere with the promise chain being returned correctly. I couldn't find an example of a promisey throw test, suggestions on how to make it better are very welcome.

[janmeier]

Shouldn't this be restore?

[janmeier]

For some reason you have to wrap it in a function - still not pretty, but better

return this.User.create({username : "Peter", secretValue : "42"})
      .then(function(user){
          expect(function () {
            self.User.restore({where : {secretValue : "42"}})
          }).to.throw(Error, "Model is not paranoid");
      })

[seth-admittedly]

Thanks for the typo catch and the function wrapper suggestion, which makes sense once you see it.

[janmeier]

Good work as always @seth-admittedly !