1

I'm learning node.js currently and this is my first ever project in it. It's (supposedly) a simple to-do list app where there are multiple lists I can load/edit/save/remove.

In the todo_list.ejs file I have a div where I list all the collection names:

    <div id="list_container" class="lists">
        <ul id="col_list" class="collection-list">
            <% lists.forEach(list => { %>
                <li class="collection-list-item">
                    <div class="list-name-container">                        
                        <a href="/<%=list.name %>" class="list-link">
                            <span class="list-name" name="list_name"><%=list.name %></span>
                        </a>
                    </div>
                </li>
            <% }) %>
        </ul>
    </div>

looks like this: enter image description here

When I click on the link of a list. I try to use the following code to load in a new list (which is a mongodb collection):

app.route("/:list_name").get((req, res) => {
MongoClient.connect(process.env.DB_CONNECT, (err, db) => {
    if(err) throw err;
    var database = db.db("myFirstDatabase");
    const cursor = database.collection(req.params.list_name).find({}); /* stuck here */
    database.listCollections().toArray((err, collections) => {
        if(err) throw err;
        db.close();
        collections.forEach(element => {
            if(element.name == req.params.list_name){
                current_list = element;
                current_list_name = element.name;
            }
        });
        task.find({}, (err, todo_tasks) => { /*currently using the model.find() method to list all the documents which always looks at the "tasks" collection*/
            res.render("todo_list.ejs", { tasks: todo_tasks, lists: collections, curr_list: current_list_name });
        });
    });
 });
 });

I commented where I'm stuck in the code above. I'm trying to get a mongodb collection by name, and then load all its contents onto a list after, but I don't know how to find a collection by name. Reading through the node.js documentation lead me to the cursor object, which has a ton of info and properties I have no clue what to do with...

Is there a simple way to find a collection by name and get a list of it's documents?

EDIT 1:

this is where i add tasks:

//ADD TASK TO LIST
app.post('/', async (req, res) => {
const tsk = new task({ /*the mongodb model for tasks*/
    content: req.body.content,
    deadline: req.body.deadline
});

try {
    await tsk.save();
    res.redirect("/");
} catch(e) {
    console.log(e);
    res.redirect("/");
}
});
Improve this question
4
  • There are a lot of things that are wrong in your code (not trying to roast or anything, I've been there myself, lost in the JS world). Leave me a comment here if you want a detailed answer (which I'll post later on), or if not, try any of the other answers below and see if they work for you. Commented Jan 4, 2022 at 16:41
  • @GaëtanBoyals by all means, if my code were good on the very first project i'd be surprised. i'm just trying to learn. Commented Jan 4, 2022 at 16:44
  • I'll take that as a yes then. I'll leave a detailed answer addressing what I see in your code excerpt. Commented Jan 4, 2022 at 16:50
  • thank you very much in advance, any and all info is helpful Commented Jan 4, 2022 at 17:04

2 Answers 2

Reset to default
2

I will not address the EJS part in this answer as I'm not qualified and the code you provided seems all good. However, I'll review the back-end part.

Also, since I don't know what kind of coding background you have (if any), this answer will contain a lot of explanation on perhaps simple concepts.

Summary

From your second code snippet, there are a couple things that are to be discussed:

  1. Asynchronous code
  2. The database connection and generalities
  3. Actual implementation
  4. Code conception
  5. [EDIT]: Save/Edit implementations

There is also a lot more to cover depending on the knowledge of OP, such as try/catch clauses, MongoDB models validation, the usage of express's Router and more but I will only edit my answer if needed.

Asynchronous code

For the rest of the answer, most of the code will be surrounded by async/await keywords. These are necessary for the code to work properly.

Basically, JS being a language that is made for the web, you sometimes need to wait for network or database requests to be done before you do any other action. That's where the callbacks, the promises or the async/await syntax (which is syntactic sugar for promises) come in handy.

Let's say you need, like your example, to retrieve a list of tasks:

app.route("/:list_name").get((req, res) => {
MongoClient.connect(process.env.DB_CONNECT, (err, db) => {
    if(err) throw err;
    var database = db.db("myFirstDatabase");
    const cursor = database.collection(req.params.list_name).find({}); /* stuck here */
    console.log(cursor);
    // ..........
  });
});

JS being asynchronous by default, if you run this code, chances are high that cursor will be undefined. The reason for that is that the code doesn't wait for the database.collection(............. to finish in order to continue the execution. But with the help of the aforementioned callback/promises/async-await, our code can now wait until this instruction is done.

You can read on async/await here and here, and see here that MongoDB examples are using async/await as well, but you will see in the following sections more "practical" usages of it.

Keep in mind that what you are using (whether it is callbacks, promises or async/await syntax) is completely up to you and your preferences.

Database connection

As the code is currently written, everytime a user clicks on any item on your list, a connection to MongoDB will be established, and that connection doesn't belong to the route handler. Your back-end app should connect to the database once (at least for this case, it could prove useful to initiate multiple connections for some advanced cases), and close the connection when your back-end app stops (generally not the case with an API).

Atlas cloud databases for example, have a limit of 500 connections. Meaning that if, let's say, 501 users click on an item simultaneously on your front-end list, the best case scenario is someone doesn't get what he asked, but it could be worse.

For this matter, you have several options. One would be to go with a framework that helps you leverage some of the code and boilerplate, such as Mongoose or work with the native MongoDB driver which we will do, since you seem to already work with that and I strongly believe working with the lowest layer first will make you learn higher-level frameworks way faster.

Now, let's tackle the problem. We want to put the database connection somewhere else where it'll be called once. Again, there's several options you can go with, but I like to create a class for it, and exporting a new instance to do what I want anywhere in my code. Here is a (really) simple example of what my minimal go-to looks like:

mongo-client.js:

const { MongoClient } = require('mongodb');

class MongoCli {
  constructor() {
    let url = `mongodb://testuser:[email protected]:27017/?authSource=my_database_name`;
    this.client = new MongoClient(url, { useUnifiedTopology: true });
  }

  async init() {
    if (this.client) {
      await this.client.connect();
      this.db = this.client.db('test');
    } else
      console.warn("Client is not initialized properly");
  }
}

module.exports = new MongoCli();

Actual implementation

Of course, this code on his own won't work and we need to call and wait for it, before defining routes. So, right before app.route("/:list_name")............, call this: await MongoCli.init();.

Here is what my (again, really) simple server.js look like (I have separated the mongo-client code from the server):

const express = require('express');
const MongoCli = require('./mongo-cli.js');

const server = async () => {
  const app = express();

  await MongoCli.init();

  app.route("/:list_name").get(async (req, res) => {
    
  });
  return app;
};

module.exports = server;

Now, let's start implementing what you really want from the beginning, a.k.a once a user click on a topic of tasks, it will display all the tasks on the topic he clicked:

const express = require('express');
const MongoCli = require('./mongo-cli.js');

const server = async () => {
  const app = express();

  await MongoCli.init();

  app.route("/:list_name").get(async (req, res) => {
    // we will query the collection specified by req.params.list_name
    // then, .find({}) indicates we want all the results (empty filter)
    // finally, we call .toArray() to transform a Cursor to a human-readable array
    const tasks = await MongoCli.db.collection(req.params.list_name).find({}).toArray();
    // making sure we got what we needed, you can remove the line below
    console.log(tasks);
    // return a HTTP 200 status code, along with the results we just queried
    res.status(200).json(tasks);
  });
  return app;
};

module.exports = server;

Quite simple, right? Keep in mind my server.js might not look quite as yours since there are many ways to handle this and it is to the developer to find his own preferred method, but you get the idea.

Code conception

We got our GET route going, we get the results when we call the route, everything's great! ... not quite.

What happens now if we have, say, 1500 topics of tasks? Should we really create 1500 different collections, knowing that a task consist of a description, a status, a deadline, eventually a name? Sure, we can do it, but it doesn't mean we have to.

Instead, what about creating one and only collection tasks, and adding a key topic to it?

Considering the above sentences, here's what the route would now look like:

const express = require('express');
const MongoCli = require('./mongo-cli.js');

const server = async () => {
  const app = express();

  await MongoCli.init();

  app.route("/:topic_wanted").get(async (req, res) => {
    // we now know the collection is named 'tasks'
    // then, .find({topic: req.params.topic_wanted}) indicates we want all the results where the key 'topic' corresponds to req.params.topic_wanted
    // finally, we call .toArray() to transform a Cursor to a human-readable array
    const tasks = await MongoCli.db.collection('tasks').find({topic: req.params.topic_wanted}).toArray();
    // making sure we got what we needed
    console.log(tasks);
    // return a HTTP 200 OK, along with the results we just queried
    res.status(200).json(tasks);
  });
  return app;
};

module.exports = server;

Last words

I hope I'm not too off-topic and my answer could help you. Also, I saw while writing the answer that you need to figure out how to post tasks now. Please let me know in the comments if you need further information/explanation or even help for posting tasks.

EDIT (added):

Save/Edit implementations

Seeing your implementation of creating a new task, I assume you already use mongoose. Unfortunately, when declaring a model in Mongoose, it will automatically search for (or create if it doesn't exist) the collection having the same name of your declared model, except in lowercase and pluralized (see here for more info). Meaning you can't declare a new task and assign it to a collection named "users" for example.

That's where the part 4 of this answer, "Code conception", comes into play. Otherwise, the code you edited-in has no "major" flaw.

Improve this answer
Sign up to request clarification or add additional context in comments.

9 Comments

If you can be bothered to explain how to save/edit an imported list i'd be greatful. at the moment, when i add a task it always adds to the(i guess default) collection called "tasks". if necessary, i'll add the part where i add tasks to a list.
Sure thing! Indeed, I'd like you to edit your post with the part where you add tasks so I can see what needs to be changed.
added. if more info is needed, let me know
Thanks! Edited my answer as well!
yes, this code uses the mongoose model. all operations work off of it (add/edit/remove task). i guess i'll work through and change it with the help of your answer.
|
1

Try this, this should work.

Changes that I made :-

  1. MongoDb connect callback function changed to async.
  2. Add toArray() function in the end of database.collection(req.params.list_name).find({});
  3. And made the above function to await.

You can choose .then or async/await, it is up to you!

app.route("/:list_name").get((req, res) => {
MongoClient.connect(process.env.DB_CONNECT,async (err, db) => {
    if(err) throw err;
    var database = db.db("myFirstDatabase");
    const todo_tasks = await database.collection(req.params.list_name).find({}).toArray(); /* add '.toArray()' */
    database.listCollections().toArray((err, collections) => {
        if(err) throw err;
        db.close();
        collections.forEach(element => {
            if(element.name == req.params.list_name){
                current_list = element;
                current_list_name = element.name;
            }
        });
        res.render("todo_list.ejs", { tasks: todo_tasks, lists: collections, curr_list: current_list_name });
      });
    });
 });

After some improvements :-

app.route("/:list_name").get((req, res) => {
  // Connecting to MongoDb database
  MongoClient.connect(process.env.DB_CONNECT, async (err, db) => {
    if (err) throw err;
    // Choosing 'myFirstDatabase' database
    const database = db.db("myFirstDatabase");

    let todo_tasks = [];
    let collections = [];
    let current_list_name = "";

    // Getting selected list items(todo tasks) to array
    try {
      todo_tasks = await database.collection(req.params.list_name).find({}).toArray(); // Change :- Add '.toArray()'
    } catch (err) {
      if (err) throw err;
    }

    // Getting collections names
    try {
      collections = await database.listCollections().toArray();
      db.close();
    } catch (err) {
      if (err) throw err;
    }

    // Getting selected list details
    collections.forEach(element => {
      if (element.name === req.params.list_name) {
        current_list = element; // I don't understand what this code here
        current_list_name = element.name;
      }
    });

    // Rendering front end
    res.render("todo_list.ejs", {
      tasks: todo_tasks,
      lists: collections,
      curr_list: current_list_name,
    });
  });
});
Improve this answer

7 Comments

It does work, thank you. Now i just have to figure out how to actually add tasks to the loaded list.
If this works, then mark this answer as accepted answer, see this :- stackoverflow.com/help/someone-answers
Also check the improved version of code that I added to the answer.
I'll wait first to see what the detailed answer above will be like, if it doesn't help more than your answer, i will accept. Also the current_list was one of the ways i tried to get a collection and its contents, i just forgot to remove it..
Then you can upvote the answer, I deserve that if the code works or helpful for you.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.