0

I've got this piece of code that I seem to be getting in a bit of a muddle with. What it does is create users. Now, if a user has a company, then that company should be created along with the user and linked accordingly. If the company already exists, then it shouldn't be created and it shouldn't be attributed to the user.

First the code looks for a company, if it can't find one then one is created. Life is good. But if I were to add an else to my "if (!company)" check i would be duplicating the majority of my create user code. I also believe I can't check the company and then run the user creation synchronously as I would usually do in a different language. Hence i'm getting a little stuck..

module.exports = {
  postUsers: (req, res) => {
    'use strict'
    Company.findOne({name: req.body.company}, (err, company) => {
      if (err) {
        Logger.error(err)
        return res.send(500, err)
      }
      if (!company) {
        // only attribute a company if one doesn't exist
        // don't want users to assign themselves to existing companies automatically
        // need approval in place from an existing company member
        let newCompanyToAdd = new Company({
          name: req.body.company
        })
        newCompanyToAdd.save(err => {
          if (err) {
            Logger.error(err)
            return res.send(500, err)
          }
          let user = new User({
            username: req.body.username,
            password: req.body.password,
            firstname: req.body.firstname,
            lastname: req.body.lastname,
            company: newCompanyToAdd.id
          })
          user.save(err => {
            if (err) {
              return res.send(500, err)
            }
            res.status(200).json({ message: 'New User Added' })
          })
        })
      }
    })
  }

EDIT#

  postUsers: (req, res) => {
    'use strict'
    let user = new User({
      username: req.body.username,
      password: req.body.password,
      firstname: req.body.firstname,
      lastname: req.body.lastname
    })
    Company.findOne({name: req.body.company}, (err, company) => {
      if (err) {
        Logger.error(err)
        return res.send(500, err)
      }
      if (!company && req.name.company !== undefined) {
        // only attribute a company if one doesn't exist
        // don't want users to assign themselves to existing companies automatically
        // need approval in place from an existing company member
        let newCompanyToAdd = new Company({
          name: req.body.company
        })
        newCompanyToAdd.save(err => {
          if (err) {
            Logger.error(err)
            return res.send(500, err)
          }
          user.company = newCompanyToAdd._id
        })
      }
    })
    user.save(err => {
      if (err) {
        return res.send(500, err)
      }
      res.status(200).json({ message: 'New User Added' })
    })
  }
NiceYellowEgg
  • 552
  • 1
  • 5
  • 13

1 Answers1

0

I'm not totally sure I understand the overall goal. But seems like you're worried about having the add user code be replicated because you need to add the user regardless of if the company already exists or not. Is there any reason you can't save the user first, and in the callback, conditionally create the company if necessary?

James Q Quick
  • 429
  • 3
  • 5
  • I could do, but I'd have to update the user with the companyId if needed which would be an extra dB call? – NiceYellowEgg May 17 '18 at 14:10
  • If you're not too worried about the timing of which occurs first (creating the company and creating the user), then after you find the existing company or not, you can set the company id on the user to the found id or null. Then, outside of that if loop just save the user. The one thing that changes here is that saving the company and user would both be asynchronous. other than, I would just break out the save user logic into it's own function and simply call it. It's very likely that you might need to use this function in another place anyway. – James Q Quick May 17 '18 at 14:16
  • i haven't abstracted out the save part yet, but is this what you're thinking? see edit – NiceYellowEgg May 17 '18 at 14:43