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

#165273535 : Add comment on article #28

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

kalsmic
Copy link
Contributor

@kalsmic kalsmic commented Jun 14, 2019

What does this PR do?

  • add a comment on an article

Description of Task to be completed?

  • add comment reducer and actions tests and implementation

How should this be manually tested?

  • pending

Any background context you want to provide?

  • None at the moment

What are the relevant pivotal tracker stories?

Screenshots (if appropriate)

  • None

@@ -0,0 +1,17 @@
// comment constants
import { ADD_COMMENT } from 'store/actions/commentTypes';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,33 @@
import commentReducer from 'store/reducers/commentReducer';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,2 @@
// eslint-disable-next-line import/prefer-default-export
export const ADD_COMMENT = 'ADD_COMMENT';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,41 @@
// Import axios
import axios from 'axios';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,44 @@
// helper functions
import { ADD_COMMENT } from 'store/actions/commentTypes';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from e9ffe89 to 534ad55 Compare June 14, 2019 12:50
});

it('should handle add comment errors', () => {
expect(

Choose a reason for hiding this comment

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

'expect' is not defined no-undef

);
});
it('should handle Add new comment', () => {
expect(

Choose a reason for hiding this comment

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

'expect' is not defined no-undef


describe('loginReducer state', () => {
it('should return the initial state', () => {
expect(commentReducer(undefined, {})).toEqual(

Choose a reason for hiding this comment

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

'expect' is not defined no-undef


return store.dispatch(createComment('article_slug', 'bdy'))
.then(() => {
expect(store.getActions()).toEqual(expectedActions);

Choose a reason for hiding this comment

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

'expect' is not defined no-undef

};
}

handleCommentSubmit = (e) => {

Choose a reason for hiding this comment

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

Parsing error: Unexpected token =

beforeEach(() => {
store = mockStore(initialState);
moxios.install();
wrapper = shallow(<CommentForm store={store} {...props} />);

Choose a reason for hiding this comment

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

Parsing error: Unexpected token <

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch 2 times, most recently from 8eb944d to 950414d Compare June 15, 2019 05:21
);
});
it('should handle loading status for comment', () => {
expect(

Choose a reason for hiding this comment

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

'expect' is not defined no-undef

};
/**
* This function deletes a comment for a specific article
* @param {string} article_slug

Choose a reason for hiding this comment

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

Expected JSDoc for 'articleSlug' but found 'article_slug' valid-jsdoc


/**
* This function gets a list of comments for a specific article
* @param {string} article_slug

Choose a reason for hiding this comment

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

Expected JSDoc for 'articleSlug' but found 'article_slug' valid-jsdoc

} = this.props;

const commentList = comments.reverse().map(comment => (
<CommentBox

Choose a reason for hiding this comment

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

Parsing error: Unexpected token <

const DeleteComment = ({
id, author, articleSlug, deleteComment
}) => (
<Fragment>

Choose a reason for hiding this comment

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

Parsing error: Unexpected token <

};
}

handleCommentSubmit = (e) => {

Choose a reason for hiding this comment

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

Parsing error: Unexpected token =

id, author: { username, }, created_at, body, articleSlug, deleteComment
}
) => (
<div className="comment">

Choose a reason for hiding this comment

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

Parsing error: Unexpected token <

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from 950414d to 90c52fb Compare June 15, 2019 05:41
import CommentBox from 'components/Comment/CommentBox';

export class Comment extends Component {
static propTypes = {

Choose a reason for hiding this comment

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

Parsing error: Unexpected token =



beforeEach(() => {
wrapper = shallow(<CommentForm {...props} />);

Choose a reason for hiding this comment

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

Parsing error: Unexpected token <

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch 2 times, most recently from 6181b0a to 0cab835 Compare June 15, 2019 05:47
import CommentBox from 'components/Comment/CommentBox';

export class Comment extends Component {
static propTypes = {

Choose a reason for hiding this comment

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

Parsing error: Unexpected token =

const DeleteComment = ({
id, author, articleSlug, deleteComment
}) => (
<Fragment>

Choose a reason for hiding this comment

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

Parsing error: Unexpected token <



beforeEach(() => {
wrapper = shallow(<CommentForm {...props} />);

Choose a reason for hiding this comment

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

Parsing error: Unexpected token <

};
}

handleCommentSubmit = (e) => {

Choose a reason for hiding this comment

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

Parsing error: Unexpected token =

id, author: { username, }, created_at, body, articleSlug, deleteComment
}
) => (
<div className="comment">

Choose a reason for hiding this comment

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

Parsing error: Unexpected token <

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from 0cab835 to cc454c0 Compare June 15, 2019 06:31
const author = { author: { username: 'arthur' } };

it('should render without crushing', () => {
wrapper = shallow(<CommentBox

Choose a reason for hiding this comment

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

Parsing error: Unexpected token <

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from cc454c0 to 45195b7 Compare June 15, 2019 13:05
});
});
it('should handle delete a comment', () => {
expect(

Choose a reason for hiding this comment

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

'expect' is not defined no-undef

});

it('should handle errors on Adding new comment', () => {
expect(

Choose a reason for hiding this comment

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

'expect' is not defined no-undef


return store.dispatch(removeComment('article_slug', 1))
.then(() => {
expect(store.getActions()).toEqual(expectedActions);

Choose a reason for hiding this comment

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

'expect' is not defined no-undef


return store.dispatch(removeComment('article_slug', 1))
.then(() => {
expect(store.getActions()).toEqual(expectedActions);

Choose a reason for hiding this comment

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

'expect' is not defined no-undef


return store.dispatch(readComments('article_slug', 1))
.then(() => {
expect(store.getActions()).toEqual(expectedActions);

Choose a reason for hiding this comment

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

'expect' is not defined no-undef


return store.dispatch(readComments('article_slug', 1))
.then(() => {
expect(store.getActions()).toEqual(expectedActions);

Choose a reason for hiding this comment

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

'expect' is not defined no-undef


return store.dispatch(createComment('article_slug'))
.then(() => {
expect(store.getActions()).toEqual(expectedActions);

Choose a reason for hiding this comment

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

'expect' is not defined no-undef

deleteComment: deleteFn,
authenticatedUsername: 'arthur'
};
const wrapper = mount(<DeleteComment {...props} />);

Choose a reason for hiding this comment

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

Parsing error: Unexpected token <

};

it('should render without crushing', () => {
wrapper = shallow(<Comment

Choose a reason for hiding this comment

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

Parsing error: Unexpected token <

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from 45195b7 to 08c9a8b Compare June 16, 2019 09:04
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from 08c9a8b to 38a924c Compare June 16, 2019 15:14
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from 38a924c to 825ca99 Compare June 16, 2019 17:28
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from 825ca99 to 46f83bb Compare June 16, 2019 19:31
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

deleteComment: deleteFn,
authenticatedUsername: 'arthur'
};
let wrapper = mount(<DeleteComment {...props} />);

Choose a reason for hiding this comment

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

Parsing error: Unexpected token <

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from 46f83bb to d39e052 Compare June 19, 2019 04:29
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from d39e052 to 92856af Compare June 19, 2019 04:32
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link
Contributor

@JamesMudidi JamesMudidi left a comment

Choose a reason for hiding this comment

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

@kalsmic, Nice work with the implementation. However, as I continue to review please work on the documentation since it was the first thing that caught my eye. refer to this documentation. you can reach out to @llwasampijja as well for more insights.


class App extends Component {
render() {
return (
<Router>
<Switch>
<Route exact path="/" render={props => <LandingPage {...props} />} />
<Route path="/comments" render={props => <Comment {...props} articleSlug="javascript-code" />} />
Copy link
Contributor

@nadralia nadralia Jun 20, 2019

Choose a reason for hiding this comment

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

Nice Work, @kalsmic , I see you are hard coded the slug, you could pass it from the URL and pick it like this: <Route path="/comments/:slug" render={props => <Comment {...props} />} /> then from your component you can then pick from
const { match } = this.props; if (match) { const { params: { slug } } = match; }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @nadralia, this is going to be removed, it is just for temporarily viewing the implementation but the Comment component is going to be attached to an article where it will receive the article slug.

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from 92856af to 73613b2 Compare June 20, 2019 07:32
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

}
];

export default commentTestData;

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,5 @@
export const ADD_COMMENT_SUCCESS = 'ADD_COMMENT_SUCCESS';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,99 @@
import React, { Component } from 'react';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,50 @@
import React from 'react';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,32 @@
// react
import React, { Fragment } from 'react';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,103 @@
import React from 'react';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,107 @@
import React, { Component } from 'react';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,25 @@
import React from 'react';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,50 @@
// react
import React from 'react';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,54 @@
import React from 'react';

Choose a reason for hiding this comment

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

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

Copy link
Contributor

@llwasampijja llwasampijja left a comment

Choose a reason for hiding this comment

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

Hi @kalsmic , awesome job you have done. I would suggest you avoid using variable names which may conflict with the language keywords. An example is id, which has its own use in JSX. Also, it is not descriptive as it leaves me wondering whether you referring to the comment id, article id, or user id!
check this in CommentBox.js

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from 73613b2 to a42af05 Compare June 20, 2019 13:19
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from a42af05 to 0fc6a4a Compare June 20, 2019 13:29
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

@kalsmic
Copy link
Contributor Author

kalsmic commented Jun 20, 2019

Hi @kalsmic , awesome job you have done. I would suggest you avoid using variable names which may conflict with the language keywords. An example is id, which has its own use in JSX. Also, it is not descriptive as it leaves me wondering whether you referring to the comment id, article id, or user id!
check this in CommentBox.js

Thanks, @llwasampijja, I have worked on this feedback

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from 0fc6a4a to a752d87 Compare June 20, 2019 14:20
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from a752d87 to 641409b Compare June 20, 2019 14:39
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

 - add comment reducer and actions tests and implementation
 - add comment form

 [starts #165273535]
@kalsmic kalsmic force-pushed the feat-article-comment-165273535 branch from 641409b to 12e69d7 Compare June 21, 2019 12:24
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

The configured stylelint version is not supported.
The configured stylelint version is not supported. 
See a list of supported versions on our docs page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants