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.

Sign up for GitHub

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

Jump to bottom

#159206054 User can create articles #20

Merged
merged 1 commit into from
Aug 21, 2018
Merged

#159206054 User can create articles #20

merged 1 commit into from
Aug 21, 2018

Conversation

anneKay
Copy link
Contributor

@anneKay anneKay commented Aug 3, 2018 •
edited
Loading

What does this PR do?

  • Allow users to create articles
  • Allow users to get an article by slug
  • Allow users to get all articles
  • Allow users to update their article by slug
  • Allow users to delete their article by slug

Description of Task to be completed?

  • Create a route for authenticated user to create an article with slugs generated for uniqueness and
    should be timestamped
  • Create a route for authenticated or non-authenticated user to read/get an article
  • Create a route for authenticated or non-authenticated user to view/get all articles,
  • Create a route for authenticated user to update only an article(s) created by the user
  • Create a route for authenticated user to delete only an article(s) created by the user

How should this be manually tested?

In postman navigate to;
(POST) http://localhost:3000/api/articles - create article
(GET) http://localhost:3000/api/articles/:slug - get an article
(GET) http://localhost:3000/api/articles - get all articles
(PUT) http://localhost:3000/api/articles/:slug - update an article by slug
(DELETE) http://localhost:3000/api/articles/:slug - delete an article by slug

Any background context you want to provide?

  • Pull in changes and run npm install
  • Start server with npm run dev

What are the relevant pivotal tracker stories?

https://www.pivotaltracker.com/story/show/159206054

houndci-bot
houndci-bot reviewed Aug 3, 2018
View reviewed changes
seeders/20180731042502-demo-user.js Outdated
}], {}),

down: (queryInterface, Sequelize) => {
return queryInterface.bulkDelete('Users', null, {});
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 6 indent

seeders/20180731042502-demo-user.js Outdated
updatedAt: '2018-07-27 13:36:27.179+01',
}], {}),

down: (queryInterface, Sequelize) => {
Copy link

Choose a reason for hiding this comment

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

'Sequelize' is defined but never used no-unused-vars
Unexpected block statement surrounding arrow body; move the returned value immediately after the => arrow-body-style

seeders/20180731042502-demo-user.js Outdated
@@ -0,0 +1,13 @@
module.exports = {
up: (queryInterface, Sequelize) => queryInterface.bulkInsert('Users', [{
Copy link

Choose a reason for hiding this comment

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

'Sequelize' is defined but never used no-unused-vars

models/Article.js Outdated
{
foreignKey: 'userId',
onDelete: 'CASCADE',
});
Copy link

Choose a reason for hiding this comment

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

Expected a newline before ')' function-paren-newline

models/Article.js Outdated
}, {});
Article.associate = (models) => {
// associations can be defined here
Article.belongsTo(models.User,
Copy link

Choose a reason for hiding this comment

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

Expected a newline after '(' function-paren-newline

index.js Outdated
@@ -18,6 +18,13 @@ const debug = debugLog('index');

// Create global app object
const app = express();
console.log(config.cloudinary);
Copy link

Choose a reason for hiding this comment

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

Unexpected console statement no-console

@coveralls
Copy link

coveralls commented Aug 3, 2018 •
edited
Loading

Pull Request Test Coverage Report for Build 552

  • 96 of 102 (94.12%) changed or added relevant lines in 14 files are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.3%) to 87.625%
Changes Missing Coverage Covered Lines Changed/Added Lines %
helpers/sendmail.js 0 1 0.0%
middlewares/checkUser.js 14 16 87.5%
controllers/ArticleController.js 28 31 90.32%
Files with Coverage Reduction New Missed Lines %
controllers/UsersController.js 4 77.94%
Totals Coverage Status
Change from base Build 500: 0.3%
Covered Lines: 389
Relevant Lines: 431
💛 - Coveralls

@anneKay anneKay force-pushed the ft_159206054_user_create_articles branch from de5b2c9 to e2252fe Compare August 4, 2018 09:06
houndci-bot
houndci-bot reviewed Aug 4, 2018
View reviewed changes
routes/api/articleRoutes.js Outdated
router.get('/articles/:slug', ArticleControllers.getArticle);
router.get('/articles', ArticleControllers.listAllArticles);

export default router;
Copy link

Choose a reason for hiding this comment

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

Newline required at end of file but not found eol-last

models/Article.js Outdated
onDelete: 'CASCADE',
});

};
Copy link

Choose a reason for hiding this comment

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

Block must not be padded by blank lines padded-blocks

models/Article.js Outdated
{
foreignKey: 'userId',
onDelete: 'CASCADE',
});
Copy link

Choose a reason for hiding this comment

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

Expected a newline before ')' function-paren-newline

models/Article.js Outdated

Article.associate = (models) => {
// associations can be defined here
Article.belongsTo(models.User,
Copy link

Choose a reason for hiding this comment

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

Expected a newline after '(' function-paren-newline

@anneKay anneKay force-pushed the ft_159206054_user_create_articles branch from e2252fe to 431acbb Compare August 4, 2018 09:15
houndci-bot
houndci-bot reviewed Aug 4, 2018
View reviewed changes
routes/api/index.js Outdated
@@ -1,6 +1,7 @@
import { Router } from 'express';

import users from './users';
import articlesRoute from './articleRoutes';
Copy link

Choose a reason for hiding this comment

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

'articlesRoute' is defined but never used no-unused-vars

houndci-bot
houndci-bot reviewed Aug 4, 2018
View reviewed changes
routes/api/index.js Outdated
@@ -1,6 +1,7 @@
import { Router } from 'express';

import users from './users';
import articlesRoute from './articleRoutes';
Copy link

Choose a reason for hiding this comment

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

'articlesRoute' is defined but never used no-unused-vars

@anneKay anneKay force-pushed the ft_159206054_user_create_articles branch from 431acbb to 99568ec Compare August 4, 2018 09:29
klevamane
klevamane reviewed Aug 5, 2018
View reviewed changes
controllers/ArticleController.js Outdated
const articleObject = {
title, description, body, tagList, imageUrl, userId
};
/** check if image was provided in the request
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment should be inside the border
/**
*comment to be made
*/

klevamane
klevamane reviewed Aug 5, 2018
View reviewed changes
controllers/ArticleController.js Outdated
.catch(() => createArticleHelper(res, articleObject));
}

/** if there no image was provided go ahead to create the article */
Copy link
Contributor

Choose a reason for hiding this comment

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

use // for single line comment

klevamane
klevamane reviewed Aug 5, 2018
View reviewed changes
controllers/ArticleController.js Outdated
attributes: { exclude: ['id', 'userId'] }
})
.then((article) => {
/** if the article does not exist */
Copy link
Contributor

Choose a reason for hiding this comment

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

use // for single line comment

Copy link

Choose a reason for hiding this comment

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

Why do you think so? is there any impact it will have?

klevamane
klevamane reviewed Aug 5, 2018
View reviewed changes
controllers/ArticleController.js Outdated

return res.status(200).json({ article });
})
.catch(() => res.status(501).send('oops seems there is an error find the article'));
Copy link
Contributor

Choose a reason for hiding this comment

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

"seems there is an error finding the article"
if changes are made here I suppose it should also be effected on the test.

Copy link

Choose a reason for hiding this comment

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

I don't understand, can you please colour?

klevamane
klevamane reviewed Aug 5, 2018
View reviewed changes
controllers/ArticleController.js Outdated
attributes: { exclude: ['id', 'userId'] }
})
.then((articles) => {
/** check if there was no article created */
Copy link
Contributor

Choose a reason for hiding this comment

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

use // for single line comment

Copy link

Choose a reason for hiding this comment

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

The response above as regards the comment format applies here

klevamane
klevamane reviewed Aug 5, 2018
View reviewed changes
controllers/ArticleController.js Outdated
if (!article) {
return res.status(404).json({
errors: {
body: [
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be put on a single line? If so I suggest it should.

Copy link

Choose a reason for hiding this comment

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

The reason the code was broken down that way is for readability.

klevamane
klevamane reviewed Aug 5, 2018
View reviewed changes
controllers/ArticleController.js Outdated
/** check if there was no article created */
if (articles.length === 0) {
return res.status(200).json({
message: 'Your request was successful but no articles created',
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest you just say "No articles found", as this is just a direct information to the user trying to retrieve articles

Copy link

Choose a reason for hiding this comment

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

Okay, yes but aren't we saying the same thing? To colour it, a user wants to get all the articles, the request was successful but there are no articles in the database yet.

klevamane
klevamane reviewed Aug 5, 2018
View reviewed changes
controllers/ArticleController.js Outdated
res.status(500).json({
errors: {
body: [
'sorry there was an error updating this request',
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be "sorry there was an error deleting the Article"

Copy link
Contributor Author

@anneKay anneKay Aug 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

thank you for spotting that out

klevamane
klevamane suggested changes Aug 5, 2018
View reviewed changes
Copy link
Contributor

@klevamane klevamane left a comment

Choose a reason for hiding this comment

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

Review the suggestions here and make changes. Let me know if u have any information regarding this. cheers

@anneKay anneKay force-pushed the ft_159206054_user_create_articles branch from 99568ec to 9f88bf1 Compare August 6, 2018 01:12
houndci-bot
houndci-bot reviewed Aug 6, 2018
View reviewed changes
models/Article.js Outdated
Article.associate = (models) => {
// associations can be defined here

Article.belongsTo(models.User,
Copy link

Choose a reason for hiding this comment

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

Expected a newline after '(' function-paren-newline

houndci-bot
houndci-bot reviewed Aug 6, 2018
View reviewed changes
models/Article.js Outdated
Article.associate = (models) => {
// associations can be defined here

Article.belongsTo(models.User,
Copy link

Choose a reason for hiding this comment

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

Expected a newline after '(' function-paren-newline

@anneKay anneKay force-pushed the ft_159206054_user_create_articles branch 5 times, most recently from 7dbbcfd to e875c01 Compare August 7, 2018 14:33
@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from e875c01 to 21afeb2 Compare August 8, 2018 11:32
klevamane
klevamane approved these changes Aug 8, 2018
View reviewed changes
Copy link
Contributor

@klevamane klevamane left a comment

Choose a reason for hiding this comment

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

I think the slug title needs be changed from extensive hash generated value.

@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from 21afeb2 to 46b9abc Compare August 8, 2018 13:31
@klevamane klevamane temporarily deployed to fargo-ah-staging-pr-20 August 14, 2018 12:40 Inactive
@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from 867632a to ec1ea07 Compare August 14, 2018 15:09
@klevamane klevamane temporarily deployed to fargo-ah-staging-pr-20 August 14, 2018 15:09 Inactive
@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from ec1ea07 to d978a68 Compare August 15, 2018 10:58
@klevamane klevamane temporarily deployed to fargo-ah-staging-pr-20 August 15, 2018 10:58 Inactive
@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from d978a68 to 8a9a3b2 Compare August 15, 2018 17:34
@klevamane klevamane temporarily deployed to fargo-ah-staging-pr-20 August 15, 2018 17:34 Inactive
@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from 8a9a3b2 to 8da0286 Compare August 16, 2018 09:57
@klevamane klevamane temporarily deployed to fargo-ah-staging-pr-20 August 16, 2018 09:57 Inactive
@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from 8da0286 to 7d703cf Compare August 16, 2018 10:07
@klevamane klevamane temporarily deployed to fargo-ah-staging-pr-20 August 16, 2018 10:07 Inactive
@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from 7d703cf to 73a0ae1 Compare August 17, 2018 13:39
@klevamane klevamane temporarily deployed to fargo-ah-staging-pr-20 August 17, 2018 13:39 Inactive
@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from 73a0ae1 to e10c9d8 Compare August 19, 2018 13:25
@klevamane klevamane temporarily deployed to fargo-ah-staging-pr-20 August 19, 2018 13:26 Inactive
@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from e10c9d8 to 790509b Compare August 20, 2018 08:01
@klevamane klevamane temporarily deployed to fargo-ah-staging-pr-20 August 20, 2018 08:01 Inactive
@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from 790509b to 17ca01a Compare August 20, 2018 08:48
@klevamane klevamane temporarily deployed to fargo-ah-staging-pr-20 August 20, 2018 08:48 Inactive
@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from 17ca01a to 5b23b81 Compare August 20, 2018 14:50
@klevamane klevamane temporarily deployed to fargo-ah-staging-pr-20 August 20, 2018 14:50 Inactive
@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from 5b23b81 to 9765bb8 Compare August 20, 2018 16:00
@klevamane klevamane temporarily deployed to fargo-ah-staging-pr-20 August 20, 2018 16:00 Inactive
ft(create-article): create user article
2ed2ba5 
- Create model for article

ft(article): create article
	- create helper function to generate slug from title

feat(articles): update and delete artices

- modify article model
- add method to edit article
- add method to delete article
- restrict update to just (3) times

[Delivers #159206054]

feat(validate-input): Validate user input

- validate user input
- create user controller class
- check if email is unique
- check if username is unique
- fix Hound CI file extension error
- test for signup route

[Delivers #159206048]

chore(coverage): Increase coverage by 75%

- add tests for user profile modifications
- modify a utility function
- modify validate.js

[Delivers #159403248]

ft(create-article): create user article
- Write unit tests
- Create model for article and association with user model
- Create middleware to validate data to create article
- Create helper function to create article
- Create helper function to generate unique slugs
- Add image upload feature using cloudinary
- Create route and controller to create article
- Create route and controller get an article using slug parameter
- Create route and controller to get all articles

[Delivers #159206054 #159206060]
@Lumexralph Lumexralph force-pushed the ft_159206054_user_create_articles branch from 9765bb8 to 2ed2ba5 Compare August 21, 2018 18:41
anuonifade
anuonifade approved these changes Aug 21, 2018
View reviewed changes
Copy link
Contributor

@anuonifade anuonifade left a comment

Choose a reason for hiding this comment

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

LGTM

@anuonifade anuonifade merged commit f80bcb2 into develop Aug 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@Lumexralph Lumexralph Lumexralph left review comments

@anuonifade anuonifade anuonifade approved these changes

@klevamane klevamane klevamane approved these changes

@OKiMaureen OKiMaureen OKiMaureen approved these changes

@Oloyedesinmiloluwa Oloyedesinmiloluwa Oloyedesinmiloluwa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@anneKay @coveralls @houndci-bot @Lumexralph @anuonifade @klevamane @OKiMaureen @Oloyedesinmiloluwa