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

#167164986 Users should be able to create novels #25

Merged
merged 2 commits into from
Aug 5, 2019

Conversation

Nerocodes
Copy link
Contributor

@Nerocodes Nerocodes commented Jul 31, 2019

Pivotal tracker story

#167164986

What does this PR do?

  • Adds create novel endpoint
  • Adds verifyToken middleware

Summary of Task

  • Users should be able to create novels

How can this be tested?

Screenshots (if appropriate)

image

@Nerocodes Nerocodes temporarily deployed to ah-dahlia-staging-pr-25 July 31, 2019 11:50 Inactive
@Nerocodes Nerocodes force-pushed the feature/167164986/create-novels-endpoint branch from 85da93d to 5c3fbdc Compare July 31, 2019 19:02
@Nerocodes Nerocodes temporarily deployed to ah-dahlia-staging-pr-25 July 31, 2019 19:02 Inactive
@Nerocodes Nerocodes force-pushed the feature/167164986/create-novels-endpoint branch from 5c3fbdc to b8cc1ad Compare July 31, 2019 19:09
@Nerocodes Nerocodes temporarily deployed to ah-dahlia-staging-pr-25 July 31, 2019 19:09 Inactive
src/routes/index.js Outdated Show resolved Hide resolved
src/middlewares/verifyToken.js Outdated Show resolved Hide resolved
@Nerocodes Nerocodes force-pushed the feature/167164986/create-novels-endpoint branch from b8cc1ad to 5bdeec6 Compare August 1, 2019 10:24
@Nerocodes Nerocodes temporarily deployed to ah-dahlia-staging-pr-25 August 1, 2019 10:24 Inactive
@Nerocodes Nerocodes requested review from Ukhu and allebd August 1, 2019 10:27
@Nerocodes Nerocodes force-pushed the feature/167164986/create-novels-endpoint branch from 5bdeec6 to 5caef49 Compare August 1, 2019 10:49
@Nerocodes Nerocodes temporarily deployed to ah-dahlia-staging-pr-25 August 1, 2019 10:49 Inactive
@Nerocodes Nerocodes force-pushed the feature/167164986/create-novels-endpoint branch from 5caef49 to 37e8071 Compare August 1, 2019 18:46
@Nerocodes Nerocodes temporarily deployed to ah-dahlia-staging-pr-25 August 1, 2019 18:46 Inactive
@Nerocodes Nerocodes force-pushed the feature/167164986/create-novels-endpoint branch from 37e8071 to f2267c7 Compare August 2, 2019 14:07
@Nerocodes Nerocodes temporarily deployed to ah-dahlia-staging-pr-25 August 2, 2019 14:07 Inactive
@Nerocodes Nerocodes requested a review from OvieMudi August 2, 2019 14:29
Copy link
Contributor

@OvieMudi OvieMudi left a comment

Choose a reason for hiding this comment

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

LGTM

src/services/index.js Outdated Show resolved Hide resolved
@Nerocodes Nerocodes force-pushed the feature/167164986/create-novels-endpoint branch from f2267c7 to 0ab8965 Compare August 2, 2019 15:22
@Nerocodes Nerocodes temporarily deployed to ah-dahlia-staging-pr-25 August 2, 2019 15:22 Inactive
@Nerocodes Nerocodes force-pushed the feature/167164986/create-novels-endpoint branch from 0ab8965 to e6f08ac Compare August 2, 2019 15:31
@Nerocodes Nerocodes temporarily deployed to ah-dahlia-staging-pr-25 August 2, 2019 15:32 Inactive
@Nerocodes Nerocodes requested a review from allebd August 2, 2019 16:34
added verifyToken middleware
[Starts #167164986]
Copy link
Contributor

@allebd allebd left a comment

Choose a reason for hiding this comment

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

LGTM

* @returns {object} json
*/
const createNovel = async (req, res) => {
const newNovel = req.body;
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if a column that's not in the database is sent in the request body?

Copy link
Contributor Author

@Nerocodes Nerocodes Aug 5, 2019

Choose a reason for hiding this comment

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

It's handled in the createNovel service. That's where I destructure the fields from the body

body
} = novel;
const getGenre = await Genre.findOne({ where: { name: genre } });
const slug = `${title.toLowerCase().split(' ').join('-')}-${author.id}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we appending the author id to the slug? the slug should be unique

Copy link
Contributor Author

@Nerocodes Nerocodes Aug 5, 2019

Choose a reason for hiding this comment

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

It is unique, the reason for the author id is to allow for different users to create novels with the same name but to limit the user from uploading a novel twice or with the same name. I used medium article links as a reference https://medium.com/@nerocodes/a-new-tool-i-found-5c634618c2bf

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

.isEmpty()
.exists()
.withMessage(`${field} is a required field`)
.isIn(['action', 'thriller', 'romance', 'fiction', 'motivational'])
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think hardcoding this here is the best way to go considering you only covered a few genres

@joel-ace joel-ace merged commit b6e29ed into staging Aug 5, 2019
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.

6 participants