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.

Sign up for GitHub

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

Jump to bottom

#167899197 Users Should Be Able To Read Articles #56

Merged
merged 1 commit into from
Sep 12, 2019
Merged

#167899197 Users Should Be Able To Read Articles #56

merged 1 commit into from
Sep 12, 2019

Conversation

JuwonAbiola
Copy link
Contributor

What does this PR do?

  • Enables a user to view an article

Description of Task proposed in this pull request?

  • Create viewArticle method
  • Update Swagger Docs
  • Write test to test viewArticle method

How should this be manually tested (Quality Assurance)?

  • Explain how to manually test this feature. For example if changes were made in
    the UI or in the API, explain where and if any specific access is needed.

What are the relevant pivotal tracker stories?

Any background context you want to add (Operations Impact)?

  • The endpoint for view article is not protected
  • Run npm run db:rollback the article seeder file was updated..

What I have learned working on this feature:

  • I learned how to stub 500 error

Screenshots:

N/A

@JuwonAbiola JuwonAbiola added pr:review This is PR is currently under review by team pr:wip PR is still a work in progress type:feature A new feature and removed pr:review This is PR is currently under review by team labels Aug 21, 2019
@JuwonAbiola JuwonAbiola force-pushed the ft-Users-should-be-able-to-read-articles-167899197 branch 2 times, most recently from 38b5589 to 7f54d1b Compare August 21, 2019 14:41
@andela andela deleted a comment from hound bot Aug 21, 2019
@JuwonAbiola JuwonAbiola added pr:review This is PR is currently under review by team and removed pr:wip PR is still a work in progress labels Aug 21, 2019
chialuka
chialuka reviewed Aug 22, 2019
View reviewed changes
server/controllers/Articles.js Show resolved Hide resolved
test/articles/viewArticle.test.js Outdated Show resolved Hide resolved
test/articles/viewArticle.test.js Outdated Show resolved Hide resolved
@JuwonAbiola JuwonAbiola added pr:wip PR is still a work in progress and removed pr:review This is PR is currently under review by team labels Aug 22, 2019
@JuwonAbiola JuwonAbiola force-pushed the ft-Users-should-be-able-to-read-articles-167899197 branch from 7f54d1b to d2d450c Compare August 22, 2019 14:50
@JuwonAbiola JuwonAbiola force-pushed the ft-Users-should-be-able-to-read-articles-167899197 branch from d2d450c to 82de0d1 Compare August 22, 2019 15:55
@JuwonAbiola JuwonAbiola added pr:review This is PR is currently under review by team and removed pr:wip PR is still a work in progress labels Aug 22, 2019
@JuwonAbiola JuwonAbiola force-pushed the ft-Users-should-be-able-to-read-articles-167899197 branch from 82de0d1 to 6ea8bbf Compare August 23, 2019 08:32
@chialuka chialuka temporarily deployed to authors-haven-developmen-pr-56 August 23, 2019 08:32 Inactive
dinobi
dinobi previously approved these changes Aug 23, 2019
View reviewed changes
@JuwonAbiola JuwonAbiola force-pushed the ft-Users-should-be-able-to-read-articles-167899197 branch from 6ea8bbf to 73e2f8e Compare August 23, 2019 15:28
@JuwonAbiola JuwonAbiola temporarily deployed to authors-haven-developmen-pr-56 August 23, 2019 15:28 Inactive
@JuwonAbiola JuwonAbiola force-pushed the ft-Users-should-be-able-to-read-articles-167899197 branch from 73e2f8e to 267db47 Compare August 24, 2019 14:22
@JuwonAbiola JuwonAbiola force-pushed the ft-Users-should-be-able-to-read-articles-167899197 branch from 267db47 to 99572b2 Compare September 10, 2019 16:22
hound[bot]
hound bot reviewed Sep 10, 2019
View reviewed changes
test/models/article.test.js Show resolved Hide resolved
test/models/article.test.js Show resolved Hide resolved
test/models/article.test.js Show resolved Hide resolved
test/models/article.test.js Show resolved Hide resolved
test/helpers/articleResponse.test.js Show resolved Hide resolved
server/helpers/articleResponse.js Show resolved Hide resolved
server/helpers/articleResponse.js Show resolved Hide resolved
server/database/seeds/20190731140735-articles-seeds.js Show resolved Hide resolved
server/database/seeds/20190731140735-articles-seeds.js Show resolved Hide resolved
server/controllers/Articles.js Show resolved Hide resolved
@JuwonAbiola JuwonAbiola force-pushed the ft-Users-should-be-able-to-read-articles-167899197 branch from 99572b2 to 3c43504 Compare September 10, 2019 19:31
@JuwonAbiola JuwonAbiola force-pushed the ft-Users-should-be-able-to-read-articles-167899197 branch 3 times, most recently from 2cd6a07 to 44757f6 Compare September 11, 2019 16:44
hound[bot]
hound bot reviewed Sep 11, 2019
View reviewed changes
server/controllers/Articles.js Outdated Show resolved Hide resolved
server/controllers/Articles.js Show resolved Hide resolved
server/controllers/Articles.js Show resolved Hide resolved
server/controllers/Articles.js Show resolved Hide resolved
@JuwonAbiola JuwonAbiola force-pushed the ft-Users-should-be-able-to-read-articles-167899197 branch 2 times, most recently from eb88f68 to 34d3aca Compare September 11, 2019 17:37
@ayodejiAA
Copy link
Contributor

@jhayx, the return does not contain 'following'

@JuwonAbiola
feature[167899197]Users should be able to read articles
cfc2254 
- create controller method to view article
- write test to test view article controller method
[Delivers #167899197]
@JuwonAbiola JuwonAbiola force-pushed the ft-Users-should-be-able-to-read-articles-167899197 branch from 34d3aca to cfc2254 Compare September 12, 2019 10:15
hound[bot]
hound bot reviewed Sep 12, 2019
View reviewed changes
server/controllers/Auth.js
.redirect(`${process.env.CLIENT_URL}?token=${user.token}&email=${email}`);
.redirect(
`${process.env.CLIENT_URL}?token=${user.token}&username=${email}`
);
Copy link

Choose a reason for hiding this comment

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

Unexpected newline before ')' function-paren-newline

server/controllers/Auth.js
@@ -29,7 +29,9 @@ class Auth {
delete user.password;
response
.status(301)
.redirect(`${process.env.CLIENT_URL}?token=${user.token}&email=${email}`);
.redirect(
Copy link

Choose a reason for hiding this comment

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

Unexpected newline after '(' function-paren-newline

@dinobi dinobi merged commit 4833517 into develop Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hound hound[bot] hound left review comments

@chialuka chialuka chialuka left review comments

@dinobi dinobi dinobi approved these changes

Assignees
No one assigned
Labels
pr:review This is PR is currently under review by team type:feature A new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@JuwonAbiola @ayodejiAA @dinobi @chialuka