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

#161291016 Enable pagination support #41

Merged
merged 2 commits into from
Nov 13, 2018

Conversation

KvNGCzA
Copy link
Contributor

@KvNGCzA KvNGCzA commented Nov 11, 2018

What does this PR do?

have get api/v1/articles?page=number&limit=number route working with pagination support.

Description of Task to be completed?

  • add pagination support for fetch articles route
  • add validator for pageNumber
  • add middleware to fetch all articles route
  • add tests for fetch articles route

How should this be manually tested?

  • npm start to start your server
  • npm run db-init to pouplate your database
  • use your browser or postman to hit the localhost:{PORT}/api/v1/articles route

Any background context you want to provide?

N/A

What are the relevant pivotal tracker stories?

#161291016

Screenshots (if appropriate)

Fetching all articles
screen shot 2018-11-12 at 12 51 07 pm

Error if page query and limit query are not integers
screen shot 2018-11-12 at 12 56 18 pm

Error if page query or limit query are less than 1
screen shot 2018-11-12 at 12 57 32 pm

Error if available page number is exceeded
screen shot 2018-11-11 at 10 23 01 am

Error if queries are not provided
screen shot 2018-11-13 at 11 52 40 am

Questions:

N/A

@darthrighteous darthrighteous temporarily deployed to valinor-ah-backend-stagi-pr-41 November 11, 2018 08:17 Inactive
@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from 20b8ff2 to 3f4dfd7 Compare November 11, 2018 08:38
@KvNGCzA KvNGCzA temporarily deployed to valinor-ah-backend-stagi-pr-41 November 11, 2018 08:38 Inactive
@KvNGCzA KvNGCzA changed the title feature(ArticleController): enable pagination support #161291016 Enable pagination support Nov 11, 2018
@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from 3f4dfd7 to 8d21655 Compare November 11, 2018 08:57
@KvNGCzA KvNGCzA temporarily deployed to valinor-ah-backend-stagi-pr-41 November 11, 2018 08:57 Inactive
@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from 8d21655 to db2d0b6 Compare November 11, 2018 09:22
@KvNGCzA KvNGCzA temporarily deployed to valinor-ah-backend-stagi-pr-41 November 11, 2018 09:22 Inactive
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 observe you are using req.params. I would have preferred req.query because pagination is more like filtration. I think a URL such as /article?page=2 looks much better than /article/page/2. Can you weigh the balance?

server/controllers/ArticleController.js Outdated Show resolved Hide resolved
server/controllers/ArticleController.js Outdated Show resolved Hide resolved
server/controllers/ArticleController.js Outdated Show resolved Hide resolved
@KvNGCzA KvNGCzA temporarily deployed to valinor-ah-backend-stagi-pr-41 November 12, 2018 09:14 Inactive
@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from a76be92 to 2bfa76b Compare November 12, 2018 09:27
@KvNGCzA KvNGCzA temporarily deployed to valinor-ah-backend-stagi-pr-41 November 12, 2018 09:27 Inactive
@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from 2bfa76b to 30ddcd2 Compare November 12, 2018 11:44
@KvNGCzA KvNGCzA temporarily deployed to valinor-ah-backend-stagi-pr-41 November 12, 2018 11:44 Inactive
@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from 30ddcd2 to b817789 Compare November 12, 2018 11:45
@KvNGCzA KvNGCzA temporarily deployed to valinor-ah-backend-stagi-pr-41 November 12, 2018 11:45 Inactive
@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from b817789 to bb85ad1 Compare November 12, 2018 11:50
@KvNGCzA KvNGCzA temporarily deployed to valinor-ah-backend-stagi-pr-41 November 12, 2018 11:50 Inactive
@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from bb85ad1 to b4da9ad Compare November 12, 2018 13:15
Copy link
Contributor

@augustineezinwa augustineezinwa left a comment

Choose a reason for hiding this comment

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

@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from b4da9ad to 152891d Compare November 12, 2018 13:43
@KvNGCzA KvNGCzA temporarily deployed to valinor-ah-backend-stagi-pr-41 November 12, 2018 13:43 Inactive
@coveralls
Copy link

coveralls commented Nov 12, 2018

Pull Request Test Coverage Report for Build 478

  • 29 of 29 (100.0%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.5%) to 92.799%

Totals Coverage Status
Change from base Build 471: 0.5%
Covered Lines: 459
Relevant Lines: 478

💛 - Coveralls

@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from 152891d to 629a9a7 Compare November 12, 2018 14:38
@KvNGCzA KvNGCzA temporarily deployed to valinor-ah-backend-stagi-pr-41 November 12, 2018 14:38 Inactive
@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from 629a9a7 to b8a7cd3 Compare November 12, 2018 15:18
@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from b8a7cd3 to f30df11 Compare November 12, 2018 16:11
@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from f30df11 to 5cc041a Compare November 13, 2018 10:51
- add pagination support for fetch articles route
- add validator for pageNumber
- add middleware to fetch all articles route
- add tests for fetch articles route

[Finishes #161291016]
@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from 5cc041a to fbe802b Compare November 13, 2018 12:38
- ensure page query is used

[Finishes #61291016]
@KvNGCzA KvNGCzA force-pushed the feature/161291016/enable-pagination branch from fbe802b to 3d8c084 Compare November 13, 2018 12:44
@mbilesanmi mbilesanmi merged commit 377b869 into develop Nov 13, 2018
@mbilesanmi mbilesanmi deleted the feature/161291016/enable-pagination branch November 13, 2018 13:14
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.

7 participants