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

#167484603 Fetch all available profiles #19

Merged
merged 10 commits into from
Aug 15, 2019

Conversation

malaba6
Copy link
Contributor

@malaba6 malaba6 commented Aug 14, 2019

What does this PR do?

  • Creates the endpoint to fetch all the user profiles
  • Tests the endpoints
  • Adds the user's endpoint in the API documentation
  • Fixes the user can successfully login test

Description of Task to be completed?

  • Create a method called fetchProfiles inside the class Profile in the profileController file.
  • Add the route in the routes folder to fetch profiles with the get method
  • In the userProfile.test file, add tests for this endpoint
  • In swagger.json file, add /api/v1/profiles in the swagger file's path to document this endpoint

How should this be manually tested?

In Postman or Swagger or any other tool of your choice visit this enpoint:
/api/v1/profiles to fetch a list of profiles

Any background context you want to provide:

To access this endpoint, User must be authenticated. And use the token generated for Loggin or Registration as Authorisation in the 'header'.

What are the relevant pivotal tracker stories?

List user functionality

Screenshots?

In Postman
Screen Shot 2019-08-14 at 04 22 29

In swagger
Screen Shot 2019-08-14 at 04 20 36

raymond42 and others added 3 commits August 13, 2019 05:25
src/middleware/editProfile.js Outdated Show resolved Hide resolved
src/tests/userprofile.test.js Outdated Show resolved Hide resolved
@raymond42 raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-19 August 14, 2019 09:49 Inactive
Copy link
Collaborator

@raymond42 raymond42 left a comment

Choose a reason for hiding this comment

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

Working good. But first, there are some conflicts needed to be resolved

@malaba6
Copy link
Contributor Author

malaba6 commented Aug 15, 2019

Working good. But first, there are some conflicts needed to be resolved

On it. This happened because of the merging. Let me rebase and get rid of the conflict

raymond42 and others added 3 commits August 15, 2019 18:08
This allows an authenticated user to fetch other users profiles
[Finishes #167484603]
This allows an authenticated user to fetch all available profiles
[Delivers #167484603]
@raymond42 raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-19 August 15, 2019 16:19 Inactive
@raymond42 raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-19 August 15, 2019 16:38 Inactive
@raymond42 raymond42 temporarily deployed to codinggeeks-ah-backnd-st-pr-19 August 15, 2019 17:07 Inactive
Copy link
Contributor

@mkiterian mkiterian left a comment

Choose a reason for hiding this comment

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

I also think the /api/profiles endpoint shouldn't require authentication. You can also remove the auth requirement on /api/profiles/:username

data: users
});
} catch (err) {
// console.log(err);
Copy link
Contributor

Choose a reason for hiding this comment

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

do not mute the error. I'd rather you throw the error, this is important for debugging purposes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay...fixed!

});

describe('GET /api/v1/profiles', () => {
it('Should A list of profiles if user is identified', (done) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

grammar: "Should return a list ..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright! done!

});
});

describe('GET /api/v1/profiles', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd also like to see an assertion about the number of profiles returned is what we expect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd also like to see an assertion about the number of profiles returned is what we expect

On it...

@malaba6
Copy link
Contributor Author

malaba6 commented Aug 15, 2019

I also think the /api/profiles endpoint shouldn't require authentication. You can also remove the auth requirement on /api/profiles/:username

I authenticated this endpoint because it is required according to the story description on the PT board here
Screen Shot 2019-08-15 at 19 57 11

for /api/profiles/:username, I removed the authentication before I pushed, so it does not require authentication

@malaba6 malaba6 added finished and removed Need Reviews Feedback to PR raised labels Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants