-
Notifications
You must be signed in to change notification settings - Fork 11
Review books #25
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: master
Are you sure you want to change the base?
Review books #25
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| 'use strict'; | ||
|
|
||
| module.exports = { | ||
| up: (queryInterface, Sequelize) => { | ||
| return queryInterface.createTable('Reviews', { | ||
| id: { | ||
| allowNull: false, | ||
| autoIncrement: true, | ||
| primaryKey: true, | ||
| type: Sequelize.INTEGER | ||
| }, | ||
| BookId: { | ||
| allowNull: false, | ||
| type: Sequelize.INTEGER, | ||
| references: { | ||
| model: 'Books', | ||
| key: 'id' | ||
| } | ||
| }, | ||
| UserId: { | ||
| allowNull: false, | ||
| type: Sequelize.INTEGER, | ||
| references: { | ||
| model: 'Users', | ||
| key: 'id' | ||
| } | ||
| }, | ||
| Rating: { | ||
| allowNull: false, | ||
| type: Sequelize.INTEGER | ||
| }, | ||
| Text: { | ||
| defaultValue: false, | ||
| allowNull: false, | ||
| type: Sequelize.STRING | ||
|
Contributor
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 does this translate down to in PostgreSQL? Is it a VARCHAR? Because if so, this will have a 255 character limit. |
||
| }, | ||
| createdAt: { | ||
| allowNull: false, | ||
| type: Sequelize.DATE | ||
| }, | ||
| updatedAt: { | ||
| allowNull: false, | ||
| type: Sequelize.DATE | ||
| } | ||
| }); | ||
| }, | ||
| down: (queryInterface, Sequelize) => { | ||
| (queryInterface, Sequelize) => { | ||
| return queryInterface.dropTable('Reviews'); | ||
| } | ||
| } | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| 'use strict'; | ||
| module.exports = (sequelize, DataTypes) => { | ||
| const Review = sequelize.define('Review', { | ||
| BookId: DataTypes.INTEGER, | ||
| UserId: DataTypes.INTEGER, | ||
| Rating: DataTypes.INTEGER, | ||
|
Contributor
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. We should validate that this is between the 2 numbers we accept. (1 to 5?) |
||
| Text: DataTypes.STRING | ||
| }, {}); | ||
| Review.associate = function({Book, User}) { | ||
| Review.belongsTo(Book); | ||
| Review.belongsTo(User); | ||
|
|
||
| }; | ||
|
|
||
|
|
||
| return Review; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,11 @@ | ||
| import { Router } from 'express'; | ||
| import { Book } from '../models'; | ||
| import { Book, Review } from '../models'; | ||
| import catchAsync from '../lib/catchAsync'; | ||
| import { pick } from 'lodash'; | ||
| import Sequelize from 'sequelize'; | ||
|
|
||
| const router = Router(); | ||
|
|
||
| const Op = Sequelize.Op; | ||
|
Contributor
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 also change the import line above to something like |
||
| // Fields people are permitted to modify using the API. | ||
| const permittedParams = ['title', 'author']; | ||
|
|
||
|
|
@@ -14,7 +14,6 @@ router.get('/', catchAsync(async (req, res) => { | |
| let books; | ||
| if (req.query.q) { | ||
| try { | ||
| const Op = Sequelize.Op; | ||
| books = | ||
| await Book.findAll({ where: | ||
| { | ||
|
|
@@ -58,13 +57,21 @@ router.post('/', catchAsync(async (req, res) => { | |
| // Get a single book by ID. | ||
| // TODO: Should render HTML, not return JSON. | ||
|
Contributor
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 TODO can be removed; it's rendering HTML now! |
||
| router.get('/:id', catchAsync(async (req, res) => { | ||
| let book; | ||
| let reviews; | ||
| let rating = 0; | ||
| try { | ||
| const book = await Book.findByPk(req.params.id); | ||
| res.json(book); | ||
| 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; | ||
|
Contributor
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 code feels like something best placed in the |
||
| } | ||
| // res.json(book); | ||
|
Contributor
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. It's OK to delete this :) |
||
| } catch(e) { | ||
| console.warn(e); | ||
| res.status(400).send(e.toString()); | ||
| } | ||
| res.render('books/show', {book: book, reviews: reviews, rating: rating}); | ||
| })); | ||
|
|
||
| // Update a book by ID. Supply fields in the same way as creating a book. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import { Router } from 'express'; | ||
| import { ensureLoggedIn } from 'connect-ensure-login'; | ||
|
|
||
| import { Book, Review} from '../models'; | ||
| import catchAsync from '../lib/catchAsync'; | ||
|
|
||
| const router = Router(); | ||
| const env = process.env.NODE_ENV || 'development'; | ||
|
|
||
| router.post('/', | ||
| ensureLoggedIn(), | ||
| catchAsync(async (req, res) => { | ||
| const {BookId, Text, Rating} = req.body; | ||
|
|
||
| const review = await req.user.createReview({BookId, Text, Rating}); | ||
|
|
||
|
|
||
| // const loan = await req.user.createLoan({BookId, dueDate}); | ||
|
Contributor
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's this? |
||
|
|
||
| res.redirect('/books/'+BookId); | ||
| }) | ||
| ); | ||
| export default router; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| <p><%- book.title %></p> | ||
| <p><%- book.author %></p> | ||
| <p>average rating: <%- rating %></p> | ||
|
|
||
| <% reviews.forEach((review) => { %> | ||
| <p>Stars: <%- review.Rating %> <%- review.Text %></p> | ||
| <hr/> | ||
| <% }) %> | ||
| <hr/> | ||
| <form action="/review" method="post"> | ||
| <input type="hidden" name="BookId" value="<%- book.id %>"> | ||
| <p>Rate this book</p> | ||
| <label for="Rating">Stars</label> | ||
| <select name="Rating"> | ||
| <option value="1">1</option> | ||
| <option value="2">2</option> | ||
| <option value="3">3</option> | ||
| </select> | ||
| <br/> | ||
| <label for="Text">My opinion about this book</label> | ||
| <br/> | ||
| <textarea name="Text" rows="8" cols="80"></textarea> | ||
| <br/> | ||
| <button type="submit">Submit my review</button> | ||
| </form> |
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.
Oh hello 4 space tabs!
We don't have a documented convention, but the rest of the code is 2 spaces for tabs.