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

166816218 get all users #32

Merged

Conversation

fantastic-genius
Copy link
Contributor

@fantastic-genius fantastic-genius commented Jul 13, 2019

Description

An authenticated user should be able to get all users

Type of change

Please select the relevant option

  • Bug fix (non-breaking changes which fixes an issue)
  • New feature (non-breaking changes which adds functionality)
  • Breaking change (fix or feature that could cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Chore

How Has This Been Tested?

please describe the test that you ran to verify your changes

  • End to end
  • Integration

Checklist:

  • My code follows the style guidelines of this project
  • I have linted my code prior to submission
  • I have made corresponding changes to the documentation
  • My changes generates no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Pivotal Tracker

#166816218


describe('Get all Users', () => {
before(async () => {
await createTestFakeUsers();
Copy link
Contributor

Choose a reason for hiding this comment

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

Good job on using faker to generate data in your database to allow you test with. I think it would be good to clear the User table in your before block before running your test, since your test depends on the number of users it meets in the User table ---- line 391. This will avoid errors incase the User table had values before your test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no need for that because the user whose token is been used to access the endpoint is created at the top level before hook declared first within the test file. Token can't be gotten from the fake users created.

@fantastic-genius fantastic-genius force-pushed the feature/166816218-list-users-and-get-user-profile branch from 49ada01 to b740bad Compare July 16, 2019 09:50
routes/v1/users.js Outdated Show resolved Hide resolved
routes/v1/users.js Outdated Show resolved Hide resolved
@fantastic-genius fantastic-genius force-pushed the feature/166816218-list-users-and-get-user-profile branch 2 times, most recently from 240bbf7 to b9d1b19 Compare July 16, 2019 12:19
- Implement get all users feature
- Add test for feature
- Document feature

[Delivers #166816218]
@fantastic-genius fantastic-genius force-pushed the feature/166816218-list-users-and-get-user-profile branch from b9d1b19 to d0cde89 Compare July 16, 2019 14:25
@andela-johia andela-johia merged commit 3957506 into staging Jul 17, 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.

None yet

4 participants