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

#168108145 Get All Genres #52

Merged
merged 1 commit into from
Aug 26, 2019
Merged

Conversation

allebd
Copy link
Contributor

@allebd allebd commented Aug 26, 2019

Pivotal tracker story

#168108145

What does this PR do?

Users should be able to view all genres

Summary of Task

  • Create get genres controller
  • Add test
  • Add swagger documentation

How can this be tested?

  • Use git clone to clone this repository to local branch
  • Use npm install to download all necessary packages
  • Ensure your database is created or you can use sequelize db:create if database does not exist
  • Use npm run test to test
  • Go your POSTMAN and type http://localhost:${PORT}/api/v1/genres using a GET request to view all genres

Screenshots (if appropriate)

Blank Diagramooo

Flow Chart to Get all genres

Screenshot 2019-08-26 at 3 43 48 PM

Screenshot of a successful request to get all genres

@allebd allebd requested a review from Nerocodes August 26, 2019 17:59
@Nerocodes Nerocodes temporarily deployed to ah-dahlia-staging-pr-52 August 26, 2019 17:59 Inactive
@Nkemjiks
Copy link
Contributor

Should i user be authenticated for them to get all genres @allebd ?

@allebd
Copy link
Contributor Author

allebd commented Aug 26, 2019

Should i user be authenticated for them to get all genres @allebd ?

Yes, you would have to be logged in to get all genres, according to the PO

@Nkemjiks
Copy link
Contributor

Nkemjiks commented Aug 26, 2019

I think that should be included in the flow chart. Also fix the conflicts

Create get genres controller
Add test
Add swagger documentation

[Finishes #168108145]
@allebd
Copy link
Contributor Author

allebd commented Aug 26, 2019

I think that should be included in the flow chart

Thank you for the observation, would work on it

@allebd
Copy link
Contributor Author

allebd commented Aug 26, 2019

I think that should be included in the flow chart

Thank you for the observation, would work on it

Treated accordingly

@Nkemjiks Nkemjiks merged commit b7650bf into staging Aug 26, 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