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

Promise hell #33

Open
raq929 opened this issue Dec 12, 2016 · 3 comments
Open

Promise hell #33

raq929 opened this issue Dec 12, 2016 · 3 comments

Comments

@raq929
Copy link
Contributor

raq929 commented Dec 12, 2016

Remove thens within thens in example controller. Best practice for promises is not nesting thens. The return value from the previous then will be passed to the following one if information needs to be passed on.

@raq929
Copy link
Contributor Author

raq929 commented Dec 12, 2016

For example, update can be:

const update = (req, res, next) => {
  let search = { _id: req.params.id, _owner: req.currentUser._id };
  Example.findOne(search)
    .then(example => {
      if (!example) {
        return next();
      }

      delete req.body._owner;  // disallow owner reassignment.
      return example.update(req.body.example) 
    })
    .then(() => res.sendStatus(200));
    .catch(err => next(err));
};

@gaand
Copy link

gaand commented Dec 13, 2016

I believe this refactoring introduces a bug as it may call next and res.sendStatus(200).

@raq929
Copy link
Contributor Author

raq929 commented Dec 13, 2016

Agreed. I'd like to get our of the promise nesting if possible, but this isn't the right way to do it. =/

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

No branches or pull requests

2 participants