-
Notifications
You must be signed in to change notification settings - Fork 0
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
#160609520 create and retrieve comments #26
Conversation
a37e78c
to
b03886a
Compare
Pull Request Test Coverage Report for Build 416
💛 - Coveralls |
@@ -11,13 +11,15 @@ | |||
"enzyme-adapter-react-16": "^1.6.0", | |||
"history": "^4.7.2", | |||
"jwt-decode": "^2.2.0", | |||
"moment": "^2.22.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MuhanguziDavid Do you need this when you have react-moment
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelbwambale , I installed react-moment
that package was probably installed as a dependency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool
.then((response) => { | ||
dispatch(getCommentsSuccess(response.data)); | ||
}) | ||
.catch(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @MuhanguziDavid.
This section might need more work though.
Why are you logging out the user?
Might it not be necessary to display that reason to that user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @RonKbS , I am logging out the user because the only way in which an error can occur is if the user's authentication token has expired.
I shall add a toast to display the reason for being logged out to the user.
b03886a
to
ce0b06d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback @RonKbS . I am not able to display the article being commented on because the functionality to display a single article has not yet been merged into develop. |
}); | ||
}); | ||
|
||
it('should it should add a comment payload when GET_COMMENTS_SUCCESS is true', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change the wording here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wording has been changed
- create and retrieve comments [Delivers #160609520]
ce0b06d
to
e8baaf3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What does this PR do?
It enables users to create and retrieve comments to articles
Description of tasks to be completed?
Currently, a user cannot create or view comments to articles.
This PR adds functionality to allow users to create comments to articles and view the comments to an article.
How should this be manually tested?
npm install
to install all dependenciesnpm start
Please add a comment
. Click theSave
button.The created comment should be saved and displayed among the list of comments.
What are the relevant pivotal tracker stories?
#160609520
Any background context you want to add?
N/A
Checklist: I completed these to help reviewers
My IDE is configured to follow the ESlint Conventions
I have added tests to cover my changes.
All new and existing tests passed.
My pull request is based on the latest changes of the develop branch.