Skip to content
This repository has been archived by the owner on Jan 13, 2021. It is now read-only.

Feature/stefan/favorites #49

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

stefanT9
Copy link
Contributor

@stefanT9 stefanT9 commented May 4, 2020

Added routes to get shares and to add/remove courses from favorites

Copy link
Member

@procateodor procateodor left a comment

Choose a reason for hiding this comment

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

Resolve conflicts.

@stefanT9
Copy link
Contributor Author

stefanT9 commented May 4, 2020

@stefanT9
Copy link
Contributor Author

stefanT9 commented May 4, 2020

User favorite courses


exports.getFavorites = async (req, res) => {
try {
const share = await req.db.Share.findById(req.param.ShareId)
Copy link
Member

@procateodor procateodor May 5, 2020

Choose a reason for hiding this comment

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

nu o sa mearga asta. Trebuie ObjectId(param.ShareId)
also use shareId and findOne({_id: ..}) instead.

favorites: user.favoriteCourses
})
} else {
return res.status(HttpStatus.Unathorized).json({
Copy link
Member

Choose a reason for hiding this comment

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

e cu litere mari UNATHORIZED

try {
let newFavoriteCourses = []

if (req.user) {
Copy link
Member

Choose a reason for hiding this comment

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

no need for this if. The requireAuth middleware do that.

Comment on lines 36 to 46
for (const course in req.body.coursesToAdd) {
newFavoriteCourses.push(course)
}
for (const course in req.user.favoriteCourses) {
newFavoriteCourses.push(course)
}
newFavoriteCourses = [new Set(newFavoriteCourses)]

for (const course in req.body.coursesToRemove) {
newFavoriteCourses.filter((item, index) => item !== course)
}
Copy link
Member

Choose a reason for hiding this comment

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

not work. you should first get the user: User.findOne({_id: ObjectId(req.user[idClaim])})
after add the new courses user.favoriteCourses = new Set(...user.favoriteCourses, ...body.coursesToAdd)
after filter them user.favoriteCourses.filter(course => coursesToRemove.includes(course))


await req.db.User.updateOne(
{ _id: ObjectId(req.user[idClaim]) },
{ password: newFavoriteCourses }
Copy link
Member

Choose a reason for hiding this comment

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

password...?

try {
let newRecivers = []

if (req.user) {
Copy link
Member

Choose a reason for hiding this comment

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

same, no need for this if

Comment on lines 76 to 86
for (const reciverId in req.body.reciverToAdd) {
newRecivers.push(reciverId)
}
for (const reciverId in req.share.recivers) {
newRecivers.push(reciverId)
}
newRecivers = [new Set(newRecivers)]

for (const reciverId in req.body.reciverToRemove) {
newRecivers.filter((item, index) => item !== reciverId)
}
Copy link
Member

Choose a reason for hiding this comment

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

do same as below.

for (const reciverId in req.body.reciverToRemove) {
newRecivers.filter((item, index) => item !== reciverId)
}
const share = req.db.Share.find({ ownerId: req.user.id })
Copy link
Member

Choose a reason for hiding this comment

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

this should be done first and edit the share item

}
const share = req.db.Share.find({ ownerId: req.user.id })
await req.db.Share.updateOne(
{ _id: share._id },
Copy link
Member

Choose a reason for hiding this comment

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

ObjectId

models/share.js Outdated

const shareSchema = new Schema(
{
owner: {
Copy link
Member

@procateodor procateodor May 5, 2020

Choose a reason for hiding this comment

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

You used ownerId in controller

routes/index.js Outdated
@@ -15,7 +16,7 @@ router.get('/', (req, res) => {
})

router.use('/auth', auth)

router.use('/share', share)
Copy link
Member

Choose a reason for hiding this comment

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

move after requireAuth

Copy link
Member

@procateodor procateodor left a comment

Choose a reason for hiding this comment

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

Resolve comments.

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

Successfully merging this pull request may close these issues.

2 participants