0

I am new to mongodb. I have a users db and collections dB in mongodb so I am trying to return all the collections for a particular user. in my collections schema, I have linked the user by using Types.ObjectId as shown below.

const CollectionsSchema = new Schema({
  user: {
    type: Schema.Types.ObjectId,
    ref: "users"
  },
  title: {
    type: String,
    trim: true
  },
  overview: { type: String },
  year: { type: String },
  poster: { type: String },
  rating: { type: Number },
  movieId: { type: Number },
  date: { type: Date, default: Date.now }
});

const Collections = mongoose.model("collections", CollectionsSchema);

module.exports = Collections;

Now I am creating a protected route that returns collections for a particular user as shown below

router.get("/movies/:user", requireAuth, (req, res) => {
  User.findOne({ id: req.user.id }).then(user => {
    Collections.find({ user: req.params.user })
      .then(collections => {
        if (collections.user.toString() !== req.user.id) {
          return res.status(401).json({ notauthorized: "User not authorized" });
        }

        res.json(collections);
      })

      .catch(err => res.status(400).json("Error: " + err));
  });
});

I get "Error: TypeError: Cannot read property 'toString' of undefined" because I have more than one collection, but when I use the findOne() as shown below 

router.get("/movies/:user", requireAuth, (req, res) => {
  // const userId = req.user._id;
  User.findOne({ id: req.user.id }).then(user => {
    Collections.findOne({ user: req.params.user })
      .then(collections => {
        if (collections.user.toString() !== req.user.id) {
          return res.status(401).json({ notauthorized: "User not authorized" });
        }

        res.json(collections);
      })

      .catch(err => res.status(400).json("Error: " + err));
  });
});

I successfully get one item from my collection.

Please, how can I use find(), loop through the collection and return all the items in a collection for a particular logged in user?

Thanking you all in anticipation of your help and time taken to look into this.

Blockquote

Blockquote

Improve this question

2 Answers 2

Reset to default
1

your problem is the line:

  if (collections.user.toString() !== req.user.id)

because collections is an array of objects and hasn't property 'user'

so change it to 

router.get("/movies/:user", requireAuth, (req, res) => {
    User.findOne({ id: req.user.id }).then(user => {
        Collections.find({ user: req.params.user })
        .then(collections => {
            if (collections.findindex((obj)=>{ return obj.user.toString() !== req.user.id }) >-1) {
        return res.status(401).json({ notauthorized: "User not authorized" });
        }
    res.json(collections);
    })
   .catch(err => res.status(400).json("Error: " + err));
  });
  });

*** notice that ! actually this fix your issue but the if statement for checking user.id here is unnecessary because you filtered collections by user.id so this is semantic error

best approach is using another authorization checking method or this way

router.get("/movies/:user", requireAuth, (req, res) => {
    User.findOne({ id: req.user.id }).then(user => {
        Collections.find({ user: req.params.user })
        .then(collections => {
            if (collections.length < 1) {
        return res.status(401).json({ notauthorized: "User not authorized" });
        }
    res.json(collections);
    })
   .catch(err => res.status(400).json("Error: " + err));
  });
  });
Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Thanks Mohammad for your time. The first approach worked when with validations I changed "findindex" to findIndex() but for the second approach, I got the desired result however, the validations were gone. A user that wasn't logged in could still access its collection. So I went with the first approach. Thanks!
0

Understaning the problem

The error is because find() retrives an array and findOne() retrives an object.

look at:

Collections.find({ ... }, arg1 => { ... 
Collections.findOne({ ... }, arg2 => { ...

In the example above arg1 is an array, the property user doesn't exists in an array. Therefore arg1.user should return undefined. Now it's clear why the error:

"Error: TypeError: Cannot read property 'toString' of undefined"

if arg1.user is undefined you cannot call arg1.user.toString().

But in example above arg2 is an movie object that has a user property and the statement arg2.user.toString() is perfecly fine.

Problem understood, now lets fix it.

Fixing without change too much:

router.get("/movies/:user", requireAuth, (req, res) => {
  User.findOne({ id: req.user.id }).then(user => {
    Collections.find({ user: req.params.user })
      .then(collections => {
        const data = collections.filter(c => c.user.toString() === req.user.id);

        res.json(data);
      })

      .catch(err => res.status(400).json("Error: " + err));
  });
});

A better approach MAY be (assuming you trying to send all movies that match the user):

router.get("/movies/:user", requireAuth, (req, res) => {
  if (req.params.user !== req.user.id) {
    return res.status(401).json({ notauthorized: "User not authorized" });
  }

  Collections
    .find({ user: req.params.user })
    .then(collections => {
      res.json(collections);
    })
    .catch(err => res.status(400).json("Error: " + err));
});

Or even better: (assuming you need to send all movies that the logged user can see):

router.get("/movies/logged-user", requireAuth, (req, res) => {
  Collections
    .find({ user: req.user.id })
    .then(collections => {
      res.json(collections);
    })
    .catch(err => res.status(400).json("Error: " + err));
});

In the snippet above /movies/:user was changed to /movies/logged-user

When a : is used on express url, express will make a router param. Check Express API Reference for more information.

This: /movies/:user
Matches: /movies/abc, /movies/123, /movies/abc123

basicaly express will match anything after /movies/ and put the value on req.params.user.

But if you want to return the movies that a logged user can see, maybe the url should be:

/movies/logged-user ONLY matches this exact url. There is not route params here, logged-user is a static text and is part of url.

It could be argued that logged-user is not necessary. Maybe the correct url should be:

/movies

The difference between /movies/logged-user and /movies is purely semantical. The first one tells explicit to the person that will consume the api url that the movies that will be listed are those who logged user can see. The second one tells that will be listed movies, but not specify anything else, maybe misleading an idea that will be listed all movies.

Improve this answer

2 Comments

Thanks! I found it easier to understand the first approach.
Hi Jonny, could you pls explain more hoe the last approach works? I just copied your code and pasted in mine and it also worked. what does "logged-user" stand for? is it built-into mongodb? will my endpoint remain "/movies/logged-user" or I need to replace "logged-user" with a param? Thanks in advance!

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.