Skip to content
This repository has been archived by the owner on May 9, 2021. It is now read-only.

#161291020 Implement time to read an article #54

Merged
merged 1 commit into from
Nov 15, 2018

Conversation

darthrighteous
Copy link
Collaborator

What does this PR do?

Add the time to read an article to the article table

Description of Task to be completed?

  • add a readTime column to the article table
  • calculate the time taken to read an article and add to the article table

How should this be manually tested?

  • clone the repo, checkout to this branch feature/161291020/calculate-time-to-read
  • set up your local db and .env variables
  • sign up a user using the sign up route
  • create an article using the create article route
  • see the calculated reading time in the response or query the db

Any background context you want to provide?

The readTime value is in microseconds. Transformations can be done on the front-end.

What are the relevant pivotal tracker stories?

161291020

Screenshots (if appropriate)

Articles table columns:
screen shot 2018-11-14 at 3 28 45 pm

Read time of a created article:
screen shot 2018-11-14 at 3 29 37 pm

Questions:

N/A

@coveralls
Copy link

coveralls commented Nov 14, 2018

Pull Request Test Coverage Report for Build 548

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.009%) to 93.068%

Totals Coverage Status
Change from base Build 542: 0.009%
Covered Lines: 578
Relevant Lines: 600

💛 - Coveralls

@darthrighteous darthrighteous temporarily deployed to valinor-ah-backend-stagi-pr-54 November 14, 2018 14:35 Inactive
@darthrighteous darthrighteous force-pushed the feature/161291020/calculate-time-to-read branch from 8182b7b to 0461f74 Compare November 14, 2018 15:41
@darthrighteous darthrighteous temporarily deployed to valinor-ah-backend-stagi-pr-54 November 14, 2018 15:44 Inactive
@darthrighteous darthrighteous temporarily deployed to valinor-ah-backend-staging November 14, 2018 15:45 Inactive
@darthrighteous darthrighteous force-pushed the feature/161291020/calculate-time-to-read branch from 0461f74 to f0ac7ea Compare November 14, 2018 15:56
@darthrighteous darthrighteous temporarily deployed to valinor-ah-backend-stagi-pr-54 November 14, 2018 15:57 Inactive
@darthrighteous darthrighteous temporarily deployed to valinor-ah-backend-staging November 14, 2018 16:00 Inactive
- add a readTime column to the article table
- calculate the time taken to read an article

[Finished #161291020]
@darthrighteous darthrighteous force-pushed the feature/161291020/calculate-time-to-read branch from f0ac7ea to fe3fa91 Compare November 14, 2018 16:04
@darthrighteous darthrighteous temporarily deployed to valinor-ah-backend-staging November 14, 2018 16:05 Inactive
@darthrighteous darthrighteous temporarily deployed to valinor-ah-backend-staging November 14, 2018 16:39 Inactive
@sulenchy sulenchy self-requested a review November 15, 2018 08:23
Copy link
Contributor

@sulenchy sulenchy left a comment

Choose a reason for hiding this comment

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

I can see the stat.time is the reading time in milliseconds from readingTime package. I think having it in minutes would be a good idea so that whoever wants to consume the API can do that without any further manipulation.

@darthrighteous
Copy link
Collaborator Author

@sulenchy Thanks for the feedback. I thought about this and I think it's better to have the time in milliseconds so we have options on the front end. For example, if we wanted to display the seconds as well, not just the minutes. Considering the calculation isn't complex(simple division), I think doing on the frontend won't be an issue. What do you think?

@mbilesanmi mbilesanmi merged commit fa23715 into develop Nov 15, 2018
@mbilesanmi mbilesanmi deleted the feature/161291020/calculate-time-to-read branch November 15, 2018 22:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants