Keystone-classic: Errors returned by schema.pre('remove') middleware don't display in the Admin UI

Created on 12 Aug 2014  路  13Comments  路  Source: keystonejs/keystone-classic

As reported by David Vermeir in the Google Group:

When you try to delete an item but throw an error in a 'remove' hook it doesn't show up in the flashMessages in the detail view, or in the alert box in the list view.

Test case:

/*
* Stop deletion of challenge if already active
* */
Challenge.schema.pre('remove', function(next) {
    if(this.status === 1) { return next(new Error("You can't remove a challenge once it's been activated!")); }
    keystone.list('UserChallenge').model.findOne({challenge: this._id}).exec(function(err, userChallenge) {
        if(userChallenge) { return next(new Error("You can't remove a challenge that's in use by a player!")); }
        next();
    });
});

Return an error in the next() callback usually works for 'save' hooks but not for 'remove'.

In the detail view it shows the following error in the flash messages section:

There was an error deleting Challenge (logged to console)

In the list view it shows the following error in the alert popup:

There was an error deleting the challenge. ( error: database error)

So it's successfully stopping the 'remove' but not showing the errors I throw.

bug

Most helpful comment

Any news, guys?

All 13 comments

@morenoh149 @JedWatson I'm still having issues with this, I have a pre-save hook checking for duplicate e-mail adresses. If hit on a duplicate adress i call next with an error object.

I'm getting error info in console, but no flash:

Error saving changes to User 56cc44d1ba1638443b250fdf: [Error: E-mail already exists.]

The error message don't show up in the admin ui when creating a new user. Am I approaching this the wrong way?

@larsha That should work. I do it all the time in my projects. Could you post a bare-bones example that reproduces this behaviour?

Hi @creynders, here's an example of the code:

User.schema.pre('save', function(next) {
  User.model.findOne({ 'email': this.email }).exec()
    .then((user) => {
      if (user) {
        return next(new Error('E-mail already exists on another user.'));
      }

      return next();
    })
    .catch((err) => {
      next(new Error(err));
    });
});

Here's a screenshot of the admin ui part when no flash error shows after hitting create with an e-mail adress already registered.
skarmavbild 2016-02-24 kl 09 48 15

Although the error shows up in the console as mentioned: Error saving changes to User 56cd6e4a71770fe8624a3d38: [Error: E-mail already exists on another user.]

Is this 0.3? or 0.4? Come to think of it, I don't think I've got any custom validation on initial fields, maybe the bug's only with them. I'll try to verify it asap.

It's 0.3, that would be helpful, thanks @creynders !

I have got the same error, with keystone 0.3.16. The error shows in the console, but the flash message not show.

I can confirm the bug only happens when creating items, not when editing. Don't know whether it applies to 0.4 too. BTW @larsha this would always fail when trying to save an already existing user, since the query will return the user you're editing.

Thanks @creynders, some more logic around that one I guess, maybe exclude that user by it's id should solve that problem?

_Edit:_
Solved that issue like this:
User.model.findOne({ 'email': this.email, '_id': { '$ne': this._id } }).exec()

fixed in #2566 for v0.4.

Not working in 4.0.0-beta.5

Any update on this @creynders - Not working in 4.0.0-beta.5

Any news, guys?

Thanks @creynders, some more logic around that one I guess, maybe exclude that user by it's id should solve that problem?

_Edit:_
Solved that issue like this:
User.model.findOne({ 'email': this.email, '_id': { '$ne': this._id } }).exec()

thanks man for the 'id' part saved life :)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

josephg picture josephg  路  4Comments

celiao picture celiao  路  4Comments

calebmcelroy picture calebmcelroy  路  3Comments

molomby picture molomby  路  5Comments

sorryididntmeantto picture sorryididntmeantto  路  3Comments