-
Notifications
You must be signed in to change notification settings - Fork 49
Project 3 - Backend (Ian Lau, Qian Ling, Charles Lee) #53
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
base: main
Are you sure you want to change the base?
Changes from all commits
6a2f39d
2526d6a
2e7846b
3dceb4d
d68bd64
d768bb6
7f36e14
b011099
e5ff760
6ff8a1a
8832eb6
5215737
c83785e
1680ba8
0a17b40
3963dec
8e5e3fa
c24f654
e9a7f7a
6367fb8
ca9fda7
aa343f9
6721634
1e70e44
7d962b4
46d48ea
992de9a
6684fdc
284ae75
4fb98d0
ec244e5
ca995aa
994a71c
eaf3955
769ef63
9112d5a
8dfdde4
7f79540
41db171
36b83ef
78430c7
b61432c
f9a42ca
aafdfc9
84ba10a
806ed05
4a99b67
6ff88a3
58e8563
7d088c2
acf7aba
3ebbcd4
9fc14b0
a09bb80
5566195
33afa03
566075a
82be93f
87871b5
42844e4
fcf0207
3076ed2
2fcf918
2cdefb9
ce66e1c
7cabd9b
3664fea
9dba3cc
c6e0332
45c33fa
07a448a
bc296db
4c92c08
4d1881f
95d31b2
959715d
bb3f632
ec3630f
36b34ba
f56070f
cea9d1f
8005b6f
51f2d59
16d7e8a
4dad95a
ded4be6
d5d537a
1f98d80
ca17378
79c7dc3
be69160
c833030
f68117f
5a37054
2037450
f155d04
536b18a
bb2b044
122e32e
0fb0306
15974c3
56d6a0f
28fe699
9554612
e1b29c4
91e0a58
1a1ef78
8474802
12f5801
1ca7bcc
7fe4318
fe8cc40
6257014
5f85887
ddc850f
b8736da
93c4fe2
159bedb
70c2ff8
9e47f1a
458d45f
b3e582e
4b46533
6b1502e
fbb1f60
93ce61c
ef077ad
aa709a4
7772cd1
c4f9cfc
a522b0e
95e002f
fb5ea7b
546a4a2
c76d041
59f18d4
bf87b15
3f47332
88e7f4b
bf7d946
13db1c1
8c3ac93
810f2bd
784689e
e8c1b68
78d19d4
83e254b
cf15e33
c2d063e
8369a1c
6e87071
eefd08e
333ebb5
eb98a35
ad81b18
da96add
933c8a3
9a81b8c
60aadf6
bc0e3e2
25f93ab
276fef3
3dc9233
53e4139
2592ce6
3d5ca9f
cc8b566
80fb22a
b714682
b3407bf
b39ea53
03a878e
291b01e
5035794
b18ba78
2558406
9273dc9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| node_modules/ | ||
| node_modules/ | ||
| .env | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| const path = require("path"); | ||
|
|
||
| module.exports = { | ||
| config: path.resolve("config", "database.js"), | ||
| "models-path": path.resolve("db", "models"), | ||
| "seeders-path": path.resolve("db", "seeders"), | ||
| "migrations-path": path.resolve("db", "migrations"), | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| require("dotenv").config(); | ||
|
|
||
| module.exports = { | ||
| development: { | ||
| username: process.env.DB_USERNAME, | ||
| password: process.env.DB_PASSWORD, | ||
| database: process.env.DB_NAME, | ||
| host: process.env.DB_HOST, | ||
| dialect: process.env.DB_DIALECT, | ||
| }, | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| const ValidationChecker = require("./ValidationChecker"); | ||
|
|
||
| class CommentsController extends ValidationChecker { | ||
| constructor(db) { | ||
| super(); | ||
| this.commentModel = db.comment; | ||
| this.postModel = db.post; | ||
| this.userModel = db.user; | ||
| } | ||
|
|
||
| async getComments(req, res) { | ||
| const { postId } = req.params; | ||
| try { | ||
| this.checkNumber(postId, "postId"); | ||
| const comments = await this.commentModel.findAll({ | ||
| where: { commentedPostId: postId }, | ||
| include: "commenter", | ||
| order: [["createdAt", "ASC"]], | ||
| }); | ||
| return res.json(comments); | ||
| } catch (error) { | ||
| return res.status(400).json({ error: true, msg: error }); | ||
| } | ||
| } | ||
|
|
||
| async createComment(req, res) { | ||
| const { postId } = req.params; | ||
| try { | ||
| this.checkNumber(postId, "postId"); | ||
| const { userEmail, content } = req.body; | ||
| this.checkStringFromBody(userEmail, "userEmail"); | ||
| this.checkStringFromBody(content, "content"); | ||
|
Comment on lines
+29
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you could check express-validator for something similar |
||
| const postData = await this.postModel.findByPk(postId); | ||
| if (!postData) { | ||
| throw new Error("No Such Post Found."); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you do this, you basically respond with 400 (bad request). Whereas here you have a scenario that there was no post found. So this kinda returns the wrong status code. We should handle this in a different manner, with the correct status code applied |
||
| } | ||
| const user = await this.userModel.findOne({ | ||
| where: { email: userEmail }, | ||
| }); | ||
| const comment = await postData.createComment({ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if no user was found, how can you created a comment? This could potentially create corrupted data There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, since the user email comes from the req.body, I could probably create a comment for other users |
||
| commenterId: user.id, | ||
| content, | ||
| }); | ||
| return res.json({ user, comment }); | ||
| } catch (error) { | ||
| return res.status(400).json({ error: true, msg: error }); | ||
| } | ||
| } | ||
| } | ||
| module.exports = CommentsController; | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,198 @@ | ||||||
| const ValidationChecker = require("./ValidationChecker"); | ||||||
| class PostsController extends ValidationChecker { | ||||||
| constructor(db) { | ||||||
| super(); | ||||||
| this.postModel = db.post; | ||||||
| this.userModel = db.user; | ||||||
| this.categoryModel = db.category; | ||||||
| this.commentModel = db.comment; | ||||||
| this.likeModel = db.like; | ||||||
| this.sequelize = db.sequelize; | ||||||
| } | ||||||
|
|
||||||
| async getOne(req, res) { | ||||||
| const { postId } = req.params; | ||||||
| try { | ||||||
| this.checkNumber(postId, "postId"); | ||||||
| const data = await this.postModel.findByPk(postId, { | ||||||
| include: [ | ||||||
| "author", | ||||||
| "likes", | ||||||
| "categories", | ||||||
| { | ||||||
| model: this.commentModel, | ||||||
| include: { model: this.userModel, as: "commenter" }, | ||||||
| }, | ||||||
| ], | ||||||
| }); | ||||||
| return res.json(data); | ||||||
| } catch (error) { | ||||||
| return res.status(400).json({ error: true, msg: error }); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| async getPostsFromCategory(req, res) { | ||||||
| const { category } = req.params; | ||||||
| let { sortBy, limit, page } = req.query; | ||||||
| const sortByList = ["newestPost", "newestComment", "popular"]; | ||||||
| try { | ||||||
| this.checkStringFromParams(category, "category"); | ||||||
| if (limit) { | ||||||
| this.checkNumber(limit, "limit"); | ||||||
| } | ||||||
| if (page) { | ||||||
| this.checkNumber(page, "page"); | ||||||
| } | ||||||
| if (!!page && !limit) { | ||||||
| throw new Error("Must have limit for page"); | ||||||
| } | ||||||
| if (!!sortBy && !sortByList.includes(sortBy)) { | ||||||
| throw new Error( | ||||||
| `ValueError: Wrong value of sortBy ("newestPost"/"newestComment"/"popular")` | ||||||
| ); | ||||||
| } | ||||||
|
Comment on lines
+38
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel we should handle this with some kind of middleware and abstract it away. |
||||||
| const postsOption = this.getPostsOption(sortBy, limit, page); | ||||||
| const categoryInstance = await this.categoryModel.findOne({ | ||||||
| where: { name: category }, | ||||||
| }); | ||||||
| if (!categoryInstance) { | ||||||
| throw new Error(`Category: ${category} cannot be found `); | ||||||
| } | ||||||
| const posts = await categoryInstance.getPosts({ | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just eagerLoad the posts of the category? That way you don't need to query the DB twice |
||||||
| joinTableAttributes: [], | ||||||
| ...postsOption, | ||||||
| }); | ||||||
| if (!!limit) { | ||||||
| const offset = page ? (page - 1) * limit : 0; | ||||||
| return res.json(posts.slice(offset, limit)); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should use offset and limit on the DB query itself, not as a slice operation :) The DB will offset and limit for you already |
||||||
| } | ||||||
| return res.json(posts); | ||||||
| } catch (error) { | ||||||
| return res.status(400).json({ error: true, msg: error }); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| getPostsOption(sortBy) { | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would pluralize this, as you get multiple options returned |
||||||
| const postsOption = { | ||||||
| include: ["author", { model: this.likeModel, attributes: [] }], | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are the attributes empty? Do we need those? |
||||||
| group: ["post.id", "author.id"], | ||||||
| attributes: { | ||||||
| include: [ | ||||||
| [ | ||||||
| this.sequelize.fn("COUNT", this.sequelize.col("likes.id")), | ||||||
| "likeCount", | ||||||
| ], | ||||||
| ], | ||||||
| }, | ||||||
| }; | ||||||
| switch (sortBy) { | ||||||
| case "newestPost": | ||||||
| postsOption.order = [["createdAt", "DESC"]]; | ||||||
| break; | ||||||
| case "newestComment": | ||||||
| postsOption.include.push({ | ||||||
| model: this.commentModel, | ||||||
| attributes: [], | ||||||
| }); | ||||||
| postsOption.group.push("comments.created_at"); | ||||||
| postsOption.order = [["comments", "createdAt", "DESC"]]; | ||||||
| break; | ||||||
| case "popular": | ||||||
| postsOption.order = [["likeCount", "DESC"]]; | ||||||
| break; | ||||||
| } | ||||||
| return postsOption; | ||||||
| } | ||||||
|
|
||||||
| async createPost(req, res) { | ||||||
| const { categories, authorEmail, ...data } = req.body; | ||||||
| const t = await this.sequelize.transaction(); | ||||||
| try { | ||||||
| this.checkStringFromBody(authorEmail, "authorEmail"); | ||||||
| this.checkStringFromBody(data.title, "title"); | ||||||
| this.checkStringFromBody(data.content, "content"); | ||||||
| this.checkArray(categories, "categories"); | ||||||
| const author = await this.userModel.findOne({ | ||||||
| where: { email: authorEmail }, | ||||||
| }); | ||||||
| data.authorId = author.id; | ||||||
| const newPost = await this.postModel.create(data, { transaction: t }); | ||||||
| await this.addingCategoriesToPost(categories, newPost, t); | ||||||
| await t.commit(); | ||||||
| return res.send(newPost); | ||||||
| } catch (error) { | ||||||
| await t.rollback(); | ||||||
| return res.status(400).json({ error: true, msg: error }); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| async addingCategoriesToPost(categories, post, t) { | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| const categoryInstances = []; | ||||||
| for (const category of categories) { | ||||||
| const categoryInstance = await this.categoryModel.findOne({ | ||||||
| where: { name: category }, | ||||||
| }); | ||||||
| if (!categoryInstance) { | ||||||
| throw new Error(`Category name "${category}" cannot be found.`); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As stated above, we shouldn't always throw errors and return 400 across the board. By right the catch block should be used to return 500 for UNEXPECTED server errors. All other errors, that we know of and could be in control of, should be handled with an appropriate status code and error message - that would be best practice |
||||||
| } | ||||||
| categoryInstances.push(categoryInstance); | ||||||
| } | ||||||
| await post.setCategories(categoryInstances, { transaction: t }); | ||||||
| } | ||||||
|
|
||||||
| async checkIsUserLikedPost(postId, userEmail) { | ||||||
| const post = await this.postModel.findByPk(postId); | ||||||
| if (!post) { | ||||||
| throw new Error("No Such Post Found"); | ||||||
| } | ||||||
| const user = await this.userModel.findOne({ where: { email: userEmail } }); | ||||||
| if (!post) { | ||||||
| throw new Error("No Such User Found"); | ||||||
| } | ||||||
| const isUserLikedPost = await post.hasLiker(user); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isUserLikedPost and hasLiker, both names are bit confusing to me. This doesn't read nicely, and should be renamed ideally.
|
||||||
| return [isUserLikedPost, post, user]; | ||||||
| } | ||||||
|
|
||||||
| async getIsUserLikedPost(req, res) { | ||||||
| const { postId, userEmail } = req.params; | ||||||
| try { | ||||||
| this.checkNumber(postId, "postId"); | ||||||
| this.checkStringFromParams(userEmail, "userEmail"); | ||||||
| const [isUserLikedPost] = await this.checkIsUserLikedPost( | ||||||
| postId, | ||||||
| userEmail | ||||||
| ); | ||||||
| return res.json(isUserLikedPost); | ||||||
| } catch (error) { | ||||||
| return res.status(400).json({ error: true, msg: error }); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| async toggleLike(req, res) { | ||||||
| const { postId, userEmail, like } = req.body; | ||||||
| try { | ||||||
| this.checkNumber(postId, "postId"); | ||||||
| this.checkStringFromBody(userEmail, "userEmail"); | ||||||
| const [isUserLikedPost, post, user] = await this.checkIsUserLikedPost( | ||||||
| postId, | ||||||
| userEmail | ||||||
| ); | ||||||
| if (like !== isUserLikedPost) { | ||||||
| throw new Error( | ||||||
| `User already ${ | ||||||
| isUserLikedPost ? "liked" : "unliked" | ||||||
| } this post before` | ||||||
| ); | ||||||
| } | ||||||
| if (isUserLikedPost) { | ||||||
| await post.removeLiker(user); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
| } else { | ||||||
| await post.addLiker(user); | ||||||
| } | ||||||
| return res.json(!isUserLikedPost); | ||||||
| } catch (error) { | ||||||
| return res.status(400).json({ error: true, msg: error }); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| module.exports = PostsController; | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| class ValidationChecker { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not a controller, so should be place in a different folder :) |
||
| checkNumber(number, idName) { | ||
| if (isNaN(Number(number))) { | ||
| throw new Error(`Wrong Type of ${idName}`); | ||
| } | ||
| } | ||
| checkStringFromParams(string, strName) { | ||
| if (!string || !string.length) { | ||
| throw new Error(`${strName} required`); | ||
| } | ||
| } | ||
| checkStringFromBody(string, strName) { | ||
| if (typeof string !== "string") { | ||
| throw new Error(`${strName} must be string`); | ||
| } | ||
| if (!string.length) { | ||
| throw new Error(`${strName} required`); | ||
| } | ||
| } | ||
| checkArray(array, arrName) { | ||
| if (!array.length) { | ||
| throw new Error(`${arrName} require at least one data.`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| module.exports = ValidationChecker; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no README