Educado-App / educado-backend

The repository for the educado content delivery services
GNU General Public License v3.0
2 stars 4 forks source link

Mongo URI Deprecation Warning #190

Open Laskiri opened 4 days ago

Laskiri commented 4 days ago

Issue Description

Briefly describe the issue you're facing.: When starting the server, a deprecation warning for the Mongo URL shows

Steps to Reproduce

  1. npm start on dev
  2. Observe the warning

Desired Behavior

No warnings when starting the server

Proposed Solution

  1. Update mongoose to the latest version with npm install mongoose@latest
  2. Remove "useNewUrlParser: true, useUnifiedTopology: true, useFindAndModify: false, useCreateIndex: true," options passed to connectToDB; these are no longer necessary in newer versions of mongoose
  3. Update all instances of creating objectIDs across the backend server to utilize new keyword eg: "const creator = await ContentCreatorModel.findOne({ baseUser: mongoose.Types.ObjectId(req.params.id) }); " --> "const creator = await ContentCreatorModel.findOne({ baseUser: new mongoose.Types.ObjectId(req.params.id) }); "
  4. Resolve any other unforeseen issues that might occur with adopting a newer version of mongoose.

Additional Information

Warning in terminal:

image

Other startup warnings was fixed in: https://github.com/Educado-App/educado-backend/commit/ea83949c568ec67f384d1926b76ff389cf41e225

madsWesth commented 2 days ago

Looks like it's an issue when using a version 20 of node with an old mongodb driver: https://jira.mongodb.org/browse/NODE-5802 and mongoose uses the mongodb driver under the hood.

The fix here is to just update mongoose to the latest version. We also have the mongodb driver installed without ever using it, as far as i can tell. Might as well remove that package while we are at it.

Maybe we should also bump the project to a newer version of node? 🤔 The backend is using version 12 while the frontend and mobile doesnt have a version specified. They should probably all use the same version for consistency if possible.

Laskiri commented 1 day ago

Looks like it's an issue when using a version 20 of node with an old mongodb driver: https://jira.mongodb.org/browse/NODE-5802 and mongoose uses the mongodb driver under the hood.

The fix here is to just update mongoose to the latest version. We also have the mongodb driver installed without ever using it, as far as i can tell. Might as well remove that package while we are at it.

Maybe we should also bump the project to a newer version of node? 🤔 The backend is using version 12 while the frontend and mobile doesnt have a version specified. They should probably all use the same version for consistency if possible.

Yeah updating mongose does resolve the warning, but also requires us to make a lot of small changes like using new keyforword for making ObjectID's :)

madsWesth commented 1 day ago

Find and replace all? 🤔 maybe we can get away with making a middleware that passes the objectID forward through the req object. I have already kinda done this https://github.com/Educado-App/educado-backend/blob/main/middlewares/validateId.js

Laskiri commented 1 day ago

Find and replace all? 🤔 maybe we can get away with making a middleware that passes the objectID forward through the req object. I have already kinda done this https://github.com/Educado-App/educado-backend/blob/main/middlewares/validateId.js

The middleware looks like a great addition if we can seamlessly integrate it with all the relevant routes, if I understand it correctly then we can afterwards simply pass the req.id straight to the findOne methods etc?

Laskiri commented 1 day ago

straight

Ot req.ObjectId with the current naming