Conversation
jalada
left a comment
jalada
left a comment
There was a problem hiding this comment.
This is looking good! A few bits and pieces here, see what you think.
| 'use strict'; | ||
|
|
||
| module.exports = { | ||
| up: (queryInterface, Sequelize) => { |
There was a problem hiding this comment.
Oh hello 4 space tabs!
We don't have a documented convention, but the rest of the code is 2 spaces for tabs.
| Text: { | ||
| defaultValue: false, | ||
| allowNull: false, | ||
| type: Sequelize.STRING |
There was a problem hiding this comment.
What does this translate down to in PostgreSQL? Is it a VARCHAR? Because if so, this will have a 255 character limit.
| const Review = sequelize.define('Review', { | ||
| BookId: DataTypes.INTEGER, | ||
| UserId: DataTypes.INTEGER, | ||
| Rating: DataTypes.INTEGER, |
There was a problem hiding this comment.
We should validate that this is between the 2 numbers we accept. (1 to 5?)
|
|
||
| const router = Router(); | ||
|
|
||
| const Op = Sequelize.Op; |
There was a problem hiding this comment.
You could also change the import line above to something like import { Op }, Sequelize from 'sequelize;`. I think...
| book = await Book.findByPk(req.params.id); | ||
| reviews = await Review.findAll({where: {BookId: {[Op.eq]: book.id}}}); | ||
| if(reviews.length > 0) { | ||
| rating = reviews.reduce((a, review) => a + review.Rating,0) / reviews.length; |
There was a problem hiding this comment.
This code feels like something best placed in the Book or Review model instead, what do you think?
| if(reviews.length > 0) { | ||
| rating = reviews.reduce((a, review) => a + review.Rating,0) / reviews.length; | ||
| } | ||
| // res.json(book); |
| @@ -58,13 +57,21 @@ router.post('/', catchAsync(async (req, res) => { | |||
| // Get a single book by ID. | |||
| // TODO: Should render HTML, not return JSON. | |||
There was a problem hiding this comment.
This TODO can be removed; it's rendering HTML now!
| const review = await req.user.createReview({BookId, Text, Rating}); | ||
|
|
||
|
|
||
| // const loan = await req.user.createLoan({BookId, dueDate}); |
3 participants
No description provided.