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

#159987415 implement article read time #33

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

tomiadebanjo
Copy link
Contributor

@tomiadebanjo tomiadebanjo commented Sep 21, 2018

What does this PR do?

implement article read time

Description of Task to be completed?

  • Add wordsPerMinute column to the user model
  • Add number of words column to the article model
  • create a function for calculating read time
  • add beforeValidate hook to calculate no of words in the article model

How should this be manually tested?

  • Clone the repository.
  • Checkout the ft-article-read-time-159987415 branch.
  • Run npm install on your local machine.
  • Run the db migrations using npm run migrate.
  • Start the server using npm run start_dev.
  • Using postman or any similar. get a token upon registration and use it to test these routes:
    localhost:8000/api/v1/articles/:articleId

What are the relevant pivotal tracker stories?

#159987415

.set('authorization', hashedToken)
.send()
.end((err, res) => {
console.log(res.body);

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

.set('authorization', hashedToken)
.send()
.end((err, res) => {
console.log(res.body);

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

@tomiadebanjo tomiadebanjo force-pushed the ft-article-read-time-159987415 branch 2 times, most recently from 5b585bf to 4ad88e7 Compare September 21, 2018 09:53
@coveralls
Copy link

coveralls commented Sep 21, 2018

Pull Request Test Coverage Report for Build 489

  • 25 of 25 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 94.674%

Totals Coverage Status
Change from base Build 443: 0.1%
Covered Lines: 695
Relevant Lines: 736

💛 - Coveralls

- Add wordsPerMinute column to the user model
- Add number of words column to the article model
- create function for calculating read time
- add beforeValidate hook to calculate noOfWords in the article model

[Finishes #159987415]
@iAmao
Copy link
Contributor

iAmao commented Sep 21, 2018

Check your PR title

@tomiadebanjo tomiadebanjo changed the title feat(article): implement article read time #159987415 implement article read time Sep 22, 2018
* @returns {string} read time of an article
*/
const readTime = async (userId, article) => {
// Number of words in 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.

It is very good practice to comment your codes, nevertheless, you may also want to avoid redundant comment to keep best practices.
Moving forward you may want to check out this site https://code.tutsplus.com/tutorials/top-15-best-practices-for-writing-super-readable-code--net-8118 to learn more on best practices.
Aside from this, you did a very good job. Nice one 👍

@tomiadebanjo
Copy link
Contributor Author

tomiadebanjo commented Sep 23, 2018 via email

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

5 participants