freeCodeCamp / pantry-for-good

An open source food bank logistics and inventory management tool
Other
395 stars 189 forks source link

fix issue in test "admins can set other users as admins" #367

Closed spotlesscoder closed 6 years ago

spotlesscoder commented 6 years ago

caused by mongodb exception on saving user (see https://github.com/chevex-archived/mongoose-auto-increment/issues/74)

spotlesscoder commented 6 years ago

@jspaine, @kenjiO: This solves the problem with the unit test but introduces another one in the test 'Returns 400 if contents are not set':

It seems to me that in the new mongoose version, the validation gives a different result for an empty array. In server/controllers/packing.js, line 51, the validation is not failing anymore. I'm not sure what the intended behaviour is at this point. Does a package need to have contents at this point or is it ok to create it without contents and set them later on?

// Validate the data for the new packages
    try {
      await Promise.all(
        //error is not thrown here anymore
        newPackages.map(customerPackage => customerPackage.validate())
      )
    } catch (err) {
      if (err instanceof mongoose.Error.ValidationError) {
        throw new BadRequestError(err.message)
      } else {
        throw err
      }
    }
jspaine commented 6 years ago

Looks good. I think the old behaviour is right, a package should have at least one item. Seems undefined is somehow being converted to an empty array which doesn't fail the required check... It's easy to make a custom validator for that field though.

Also got a message that {useMongoClient: true} is no longer needed in mongoose.connect in server/server.js

Thanks for looking into this.

spotlesscoder commented 6 years ago

Great, it works!

jspaine commented 6 years ago

Thanks!