Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reviews_BE #66

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Reviews_BE #66

wants to merge 3 commits into from

Conversation

g4rry420
Copy link
Collaborator

Signed-off-by: gurkiran_singh gurkiransinghk@gmail.com

What this PR does (required):

  • Added Changes to Reviews controller and model
  • Added router for reviews

Screenshots / Videos (required):

N/A

Any information needed to test this feature (required):

  • models => Review.js
  • controller => review.js
  • routes => review.js
  • Test them using the Postman

Any issues with the current functionality (optional):

No

Signed-off-by: gurkiran_singh <gurkiransinghk@gmail.com>
@g4rry420 g4rry420 linked an issue Jun 22, 2021 that may be closed by this pull request
Copy link
Contributor

@dproc96 dproc96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good start but I think there are some issues with the approach. It would help to think through when and how a feature like average review would be used and simplify the manner of fetching it accordingly

Comment on lines +16 to +20
if (userId === req.body.userId) {
return res.status(401).json({
error: "User is not authorized for reviewing their own profile"
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

reviewerUserId: {
type: ObjectId,
ref: "user",
unique: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means a user can only write one review ever, if you're trying to prevent having multiple reviews of and by the same user you could either do that in the controller or have a separate unique value that combines the two ids

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it.

const { Schema } = mongoose;
const { ObjectId } = Schema;

const reviewSchema = new mongoose.Schema({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a field for a written review and not require it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I do that

Comment on lines 49 to 52
Review.aggregate(
[
{ $group: { _id: "$userId", avgRating: { $avg: "$rating" } } }
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is getting the average rating of all users, this is not the best approach. I don't think having a specific route for this is ideal, would be better to have a way to populate this value in the profile when fetching the profile

g4rry420 added 2 commits June 22, 2021 12:18
Signed-off-by: gurkiran_singh <gurkiransinghk@gmail.com>
Signed-off-by: gurkiran_singh <gurkiransinghk@gmail.com>
Copy link
Collaborator

@joelmackenz joelmackenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BE: Reviews
3 participants