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

#159987407 User can post comments #12

Merged
merged 1 commit into from
Sep 14, 2018
Merged

Conversation

jamesenejo
Copy link
Contributor

@jamesenejo jamesenejo commented Sep 7, 2018

What does this PR do?

  • Adds a commenting feature to the application
  • Stops sending descriptive errors to users when logging fails.

Description of Task to be completed?

  • Have the commenting endpoint working
    /api/v1/articles/:articleId

How should this be manually tested?

  • After cloning the repo, cd into it and run 'npm start'
  • Using postman, you can test the endpoint

What are the relevant pivotal tracker stories?

#159987407

password: 'pasS1234',
})
.end((err, res) => {
token = res.body.data.token;

Choose a reason for hiding this comment

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

Use object destructuring prefer-destructuring

@@ -0,0 +1,88 @@
import chai from 'chai';
import chaiHttp from 'chai-http';
import app from '../server/app';

Choose a reason for hiding this comment

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

Missing file extension for "../server/app" import/extensions

@@ -0,0 +1,88 @@
import chai from 'chai';
import chaiHttp from 'chai-http';

Choose a reason for hiding this comment

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

Missing file extension for "chai-http" import/extensions

@@ -0,0 +1,88 @@
import chai from 'chai';

Choose a reason for hiding this comment

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

Missing file extension for "chai" import/extensions

@@ -0,0 +1,14 @@
import jwt from 'jsonwebtoken';
import dotenv from 'dotenv';
import Cryptr from 'cryptr';

Choose a reason for hiding this comment

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

Missing file extension for "cryptr" import/extensions

@@ -0,0 +1,13 @@
import express from 'express';

Choose a reason for hiding this comment

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

Missing file extension for "express" import/extensions

@@ -1,3 +1,5 @@
import bcrypt from 'bcrypt';

Choose a reason for hiding this comment

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

Missing file extension for "bcrypt" import/extensions

@@ -0,0 +1,42 @@
import jwt from 'jsonwebtoken';
import { config } from 'dotenv';
import Cryptr from 'cryptr';

Choose a reason for hiding this comment

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

Missing file extension for "cryptr" import/extensions

@@ -0,0 +1,42 @@
import jwt from 'jsonwebtoken';
import { config } from 'dotenv';

Choose a reason for hiding this comment

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

Missing file extension for "dotenv" import/extensions

@@ -0,0 +1,42 @@
import jwt from 'jsonwebtoken';

Choose a reason for hiding this comment

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

Missing file extension for "jsonwebtoken" import/extensions

@coveralls
Copy link

coveralls commented Sep 7, 2018

Pull Request Test Coverage Report for Build 368

  • 29 of 30 (96.67%) changed or added relevant lines in 4 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.4%) to 92.612%

Changes Missing Coverage Covered Lines Changed/Added Lines %
server/controllers/commentController.js 10 11 90.91%
Files with Coverage Reduction New Missed Lines %
server/helpers/ratingHelpers.js 2 84.62%
Totals Coverage Status
Change from base Build 366: -0.4%
Covered Lines: 445
Relevant Lines: 483

💛 - Coveralls

Copy link
Contributor

@iAmao iAmao left a comment

Choose a reason for hiding this comment

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

Nice, but git workflow could have been better. Try and level up in this area

.then(comment => Users.findById(userId, {
attributes: ['id', 'username', 'firstname', 'lastname', 'createdAt', 'updatedAt']
})
.then(user => res.status(201).jsend.success({ user, comment })));
Copy link
Contributor

Choose a reason for hiding this comment

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

No catch block. We always need to handle cases like this. Just in case

@jojitoon jojitoon temporarily deployed to metis-ah-staging-pr-12 September 7, 2018 19:18 Inactive
@@ -1,4 +1,11 @@
import helper from '../helpers/helpers';
import helpers from '../helpers/helpers';

Choose a reason for hiding this comment

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

Missing file extension for "../helpers/helpers" import/extensions

@@ -0,0 +1,22 @@
import helpers from '../helpers/helpers';

Choose a reason for hiding this comment

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

Missing file extension for "../helpers/helpers" import/extensions

@@ -0,0 +1,31 @@
import validator from 'validator';
import models from '../models/index';

Choose a reason for hiding this comment

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

Missing file extension for "../models/index" import/extensions

@@ -0,0 +1,31 @@
import validator from 'validator';

Choose a reason for hiding this comment

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

Missing file extension for "validator" import/extensions

server/app.js Outdated
@@ -5,6 +5,7 @@ import swaggerUi from 'swagger-ui-express';
import jsend from 'jsend';
import swaggerDocument from '../swagger.json';
import userRoutes from './routes/userRoutes';
import articlesRoutes from './routes/articlesRoutes';

Choose a reason for hiding this comment

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

Missing file extension for "./routes/articlesRoutes" import/extensions

@jojitoon jojitoon temporarily deployed to metis-ah-staging-pr-12 September 7, 2018 19:35 Inactive
@jojitoon jojitoon temporarily deployed to metis-ah-staging-pr-12 September 11, 2018 11:03 Inactive
password: 'pasS1234',
})
.end((err, res) => {
token = res.body.data.token;

Choose a reason for hiding this comment

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

Use object destructuring prefer-destructuring

password: 'pasS1234',
})
.end((err, res) => {
token = res.body.data.token;

Choose a reason for hiding this comment

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

Use object destructuring prefer-destructuring

password: 'pasS1234',
})
.end((err, res) => {
token = res.body.data.token;

Choose a reason for hiding this comment

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

Use object destructuring prefer-destructuring

- add article routes
- add seed to initialise a user, category and article
- create controller to handle comment posts
- create middleware validate user comments
- update documentation to capture new route
- write tests to test endpoint functionality
- refactor usersValidations to confirm with ES6 syntax and not to send descriptive errors when logging in

[Delivers ##159987407]

feat(comment): user can comment

- add article routes
- add seed to initialise a user, category and article
- create controller to handle comment posts
- create middleware validate user comments
- update documentation to capture new route
- write tests to test endpoint functionality

[Delivers ##159987407]

feat(comment): user can comment

- add article routes
- add seed to initialise a user, category and article
- create controller to handle comment posts
- create middleware validate user comments
- update documentation to capture new route
- write tests to test endpoint functionality
- refactor usersValidations to confirm with ES6 syntax and not to send descriptive errors when logging in

[Delivers ##159987407]

update tests
password: 'Password'
})
.end((err, res) => {
token = res.body.data.token;

Choose a reason for hiding this comment

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

Use object destructuring prefer-destructuring

@iAmao iAmao merged commit c7b9dfc into develop Sep 14, 2018
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.

None yet

6 participants