From d237be78b643c33d271bf4805692051f4be53168 Mon Sep 17 00:00:00 2001 From: Kshitij Sobti Date: Tue, 21 Sep 2021 10:04:49 +0530 Subject: [PATCH] feat: add support for loading endorsed and un-endorsed comments separately for question type posts --- src/data/constants.js | 26 +- src/data/hooks.js | 30 ++ src/discussions/comments/CommentsView.jsx | 152 ++++++--- .../comments/CommentsView.test.jsx | 293 +++++++++--------- src/discussions/comments/comment/Comment.jsx | 63 +++- src/discussions/comments/comment/proptypes.js | 3 + .../data/__factories__/comments.factory.js | 14 +- src/discussions/comments/data/api.js | 5 + src/discussions/comments/data/redux.test.js | 91 +++++- src/discussions/comments/data/selectors.js | 12 +- src/discussions/comments/data/slices.js | 35 ++- src/discussions/comments/data/thunks.js | 8 +- src/discussions/comments/messages.js | 32 +- src/discussions/common/ActionsDropdown.jsx | 11 +- .../discussions-home/DiscussionsHome.jsx | 63 ++-- .../data/__factories__/threads.factory.js | 2 +- .../posts/post-actions-bar/PostActionsBar.jsx | 2 +- .../posts/post-actions-bar/messages.js | 2 +- .../posts/post-editor/PostEditor.jsx | 8 +- src/discussions/posts/post/Post.jsx | 28 +- src/discussions/posts/post/PostHeader.jsx | 75 ++--- src/discussions/posts/post/messages.js | 5 + 22 files changed, 636 insertions(+), 324 deletions(-) create mode 100644 src/data/hooks.js diff --git a/src/data/constants.js b/src/data/constants.js index b87bf999..b5ebb26e 100644 --- a/src/data/constants.js +++ b/src/data/constants.js @@ -12,6 +12,28 @@ export const ThreadType = { DISCUSSION: 'discussion', }; +/** + * Enum to map between endorsement status and friendly name. + * @readonly + * @enum + */ +export const EndorsementStatus = { + ENDORSED: 'endorsed', + UNENDORSED: 'unendorsed', + DISCUSSION: 'discussion', +}; + +/** + * Maps endorsement status and the corresponding API parameter. + * @readonly + * @enum + */ +export const EndorsementValue = { + [EndorsementStatus.ENDORSED]: true, + [EndorsementStatus.UNENDORSED]: false, + [EndorsementStatus.DISCUSSION]: null, +}; + /** * Edit actions for posts and comments * @readonly @@ -121,7 +143,7 @@ const BASE_PATH = '/discussions/:courseId'; export const Routes = { DISCUSSIONS: { - PATH: `${BASE_PATH}?`, + PATH: BASE_PATH, }, POSTS: { PATH: `${BASE_PATH}/topics/:topicId`, @@ -162,7 +184,7 @@ export const Routes = { }, }; -export const ALL_ROUTES = [] +export const ALL_ROUTES = [Routes.DISCUSSIONS.PATH] .concat(Routes.COMMENTS.PATH) .concat(Routes.TOPICS.PATH) .concat([Routes.POSTS.ALL_POSTS, Routes.POSTS.MY_POSTS]); diff --git a/src/data/hooks.js b/src/data/hooks.js new file mode 100644 index 00000000..bc5fce84 --- /dev/null +++ b/src/data/hooks.js @@ -0,0 +1,30 @@ +/* eslint-disable import/prefer-default-export */ +import { useState } from 'react'; + +import { useDispatch } from 'react-redux'; + +/** + * A hook that creates an enhanced version of dispatch that can track the loading state. + * + * This hook will return a boolean that tracks the current loading state, and a function + * that can be used an an alternative to dispatch for dispatching thunks. If dispatch + * is called with a thunk it's loading state will be reflected in the boolean. + * + * If you need to track multiple requests, or multiple types of requests, use multiple + * instances of this hook. e.g. one for loading and one for saving. + * + * @return {(boolean|(function(*=): Promise)|*)[]} + */ +export function useDispatchWithState() { + const dispatch = useDispatch(); + const [isDispatching, setDispatching] = useState(false); + const dispatchWithState = async (thunk) => { + setDispatching(true); + await dispatch(thunk); + setDispatching(false); + }; + return [ + isDispatching, + dispatchWithState, + ]; +} diff --git a/src/discussions/comments/CommentsView.jsx b/src/discussions/comments/CommentsView.jsx index b03a54dd..73d0c04f 100644 --- a/src/discussions/comments/CommentsView.jsx +++ b/src/discussions/comments/CommentsView.jsx @@ -1,4 +1,5 @@ import React, { useEffect } from 'react'; +import PropTypes from 'prop-types'; import { useDispatch, useSelector } from 'react-redux'; import { useParams } from 'react-router'; @@ -7,32 +8,22 @@ import { ensureConfig, getConfig } from '@edx/frontend-platform'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { Button, Spinner } from '@edx/paragon'; +import { EndorsementStatus, ThreadType } from '../../data/constants'; +import { useDispatchWithState } from '../../data/hooks'; import { Post } from '../posts'; import { selectThread } from '../posts/data/selectors'; import { markThreadAsRead } from '../posts/data/thunks'; -import { - selectThreadComments, - selectThreadCurrentPage, - selectThreadHasMorePages, -} from './data/selectors'; +import { selectThreadComments, selectThreadCurrentPage, selectThreadHasMorePages } from './data/selectors'; import { fetchThreadComments } from './data/thunks'; import { Comment, ResponseEditor } from './comment'; import messages from './messages'; ensureConfig(['POST_MARK_AS_READ_DELAY'], 'Comment thread view'); -function CommentsView({ intl }) { - const { postId } = useParams(); +function usePost(postId) { const dispatch = useDispatch(); const thread = useSelector(selectThread(postId)); - const comments = useSelector(selectThreadComments(postId)); - const hasMorePages = useSelector(selectThreadHasMorePages(postId)); - const currentPage = useSelector(selectThreadCurrentPage(postId)); - const handleLoadMoreComments = () => dispatch(fetchThreadComments(postId, { page: currentPage + 1 })); useEffect(() => { - if (!currentPage) { - dispatch(fetchThreadComments(postId, { page: 1 })); - } const markReadTimer = setTimeout(() => { if (thread && !thread.read) { dispatch(markThreadAsRead(postId)); @@ -42,42 +33,119 @@ function CommentsView({ intl }) { clearTimeout(markReadTimer); }; }, [postId]); + return thread; +} + +function usePostComments(postId, endorsed = null) { + const [isLoading, dispatch] = useDispatchWithState(); + const comments = useSelector(selectThreadComments(postId, endorsed)); + const hasMorePages = useSelector(selectThreadHasMorePages(postId, endorsed)); + const currentPage = useSelector(selectThreadCurrentPage(postId, endorsed)); + const handleLoadMoreResponses = async () => dispatch(fetchThreadComments(postId, { + endorsed, + page: currentPage + 1, + })); + useEffect(() => { + dispatch(fetchThreadComments(postId, { + endorsed, + page: 1, + })); + }, [postId]); + return { + comments, + hasMorePages, + isLoading, + handleLoadMoreResponses, + }; +} + +function DiscussionCommentsView({ + postType, + postId, + intl, + endorsed, +}) { + const { + comments, + hasMorePages, + isLoading, + handleLoadMoreResponses, + } = usePostComments(postId, endorsed); + return ( +
+ {comments.map(comment => ( + + ))} + + {hasMorePages && !isLoading && ( + + )} + {isLoading + && ( +
+ +
+ )} +
+ ); +} + +DiscussionCommentsView.propTypes = { + postId: PropTypes.string.isRequired, + postType: PropTypes.string.isRequired, + intl: intlShape.isRequired, + endorsed: PropTypes.oneOf([ + EndorsementStatus.ENDORSED, EndorsementStatus.ENDORSED, EndorsementStatus.DISCUSSION, + ]).isRequired, +}; + +function CommentsView({ intl }) { + const { postId } = useParams(); + const thread = usePost(postId); if (!thread) { return ( ); } return ( -
-
+ <> +
- {comments.length > 0 - && ( -
- -
- )} -
- {comments.map(comment => ( -
- -
- ))} - {hasMorePages && ( -
- -
- )} -
+
- -
+ {thread.type === ThreadType.DISCUSSION + && ( + + )} + {thread.type === ThreadType.QUESTION && ( + <> + + + + )} + ); } diff --git a/src/discussions/comments/CommentsView.test.jsx b/src/discussions/comments/CommentsView.test.jsx index c99f96be..bb55f191 100644 --- a/src/discussions/comments/CommentsView.test.jsx +++ b/src/discussions/comments/CommentsView.test.jsx @@ -3,93 +3,65 @@ */ import React from 'react'; -import { fireEvent, render, screen } from '@testing-library/react'; +import { + act, fireEvent, render, screen, waitFor, +} from '@testing-library/react'; import MockAdapter from 'axios-mock-adapter'; import { IntlProvider } from 'react-intl'; import { MemoryRouter, Route } from 'react-router'; +import { Factory } from 'rosie'; import { initializeMockApp } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { AppProvider } from '@edx/frontend-platform/react'; import { initializeStore } from '../../store'; +import { executeThunk } from '../../test-utils'; +import { threadsApiUrl } from '../posts/data/api'; +import { fetchThreads } from '../posts/data/thunks'; import { commentsApiUrl } from './data/api'; import CommentsView from './CommentsView'; +import messages from './messages'; -const postId = '1'; +import '../posts/data/__factories__'; +import './data/__factories__'; + +const discussionPostId = 'thread-1'; +const questionPostId = 'thread-2'; +const courseId = 'course-v1:edX+TestX+Test_Course'; let store; let axiosMock; -const mockCommentsPaged = [ - [ - { - threadId: postId, - id: '1', - renderedBody: 'test comment 1', - voteCount: 0, - author: 'testauthor', - users: { - testauthor: { - profile: { - image: { - image_url_small: '', - }, - }, - }, - }, - editableFields: [], - }, - ], - [ - { - threadId: postId, - id: '2', - renderedBody: 'test comment 2', - voteCount: 0, - author: 'testauthor', - users: { - testauthor: { - profile: { - image: { - image_url_small: '', - }, - }, - }, - }, - editableFields: [], - }, - ], -]; - function mockAxiosReturnPagedComments() { - const paramsTemplate = { - thread_id: postId, - page: undefined, - page_size: undefined, - requested_fields: 'profile_image', - }; - - const numPages = mockCommentsPaged.length; - for (let page = 1; page <= numPages; page++) { - const comments = mockCommentsPaged[page - 1]; - axiosMock - .onGet(commentsApiUrl, { params: { ...paramsTemplate, page } }) - .reply(200, { - results: comments, - pagination: { + [null, false, true].forEach(endorsed => { + const postId = endorsed === null ? discussionPostId : questionPostId; + [1, 2].forEach(page => { + axiosMock + .onGet(commentsApiUrl, { + params: { + thread_id: postId, + page, + page_size: undefined, + requested_fields: 'profile_image', + endorsed, + }, + }) + .reply(200, Factory.build('commentsResult', null, { + threadId: postId, page, - numPages, - next: page < numPages ? page + 1 : null, - }, - }); - } + pageSize: 1, + count: 2, + endorsed, + })); + }); + }); } -function renderComponent() { +function renderComponent(postId) { render( - + @@ -100,100 +72,127 @@ function renderComponent() { } describe('CommentsView', () => { - beforeEach(() => { + beforeEach(async () => { initializeMockApp({ authenticatedUser: { userId: 3, username: 'abc123', - adminsitrator: true, + administrator: true, roles: [], }, }); - store = initializeStore({ - threads: { - threadsById: { - [postId]: { - id: postId.toString(), - author: 'testauthor', - title: 'test thread', - voteCount: 0, - type: 'discussion', - pinned: false, - abuseFlagged: false, - commentCount: mockCommentsPaged.reduce((acc, cur) => acc + cur.length, 0), - courseId: 'course_id', - following: false, - rawBody: '', - read: true, - topicId: '', - updatedAt: '', - editableFields: [], - }, - }, - avatars: { - testauthor: { - profile: { - image: '', - }, - }, - }, - }, + store = initializeStore(); + Factory.resetAll(); + axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + axiosMock.onGet(threadsApiUrl) + .reply(200, Factory.build('threadsResult')); + + await executeThunk(fetchThreads(courseId), store.dispatch, store.getState); + mockAxiosReturnPagedComments(); + }); + + describe('for discussion thread', () => { + const findLoadMoreCommentsButton = () => screen.findByRole('button', { name: messages.loadMoreResponses.defaultMessage }); + it('initially loads only the first page', async () => { + renderComponent(discussionPostId); + expect(await screen.findByText('comment number 1', { exact: false })) + .toBeInTheDocument(); + expect(screen.queryByText('comment number 2', { exact: false })) + .not + .toBeInTheDocument(); }); - axiosMock = new MockAdapter(getAuthenticatedHttpClient()); - }); + it('pressing load more button will load next page of comments', async () => { + renderComponent(discussionPostId); - // TODO: use test id to prevent breaking from text changes - const findLoadMoreCommentsButton = () => screen.findByRole('button', { name: /load more comments/i }); - - it('initially loads only the first page', async () => { - const firstPageComment = mockCommentsPaged[0][0]; - const secondPageComment = mockCommentsPaged[1][0]; - mockAxiosReturnPagedComments(); - renderComponent(); - - await screen.findByText(firstPageComment.renderedBody); - expect(screen.queryByText(secondPageComment.renderedBody)).not.toBeInTheDocument(); - }); - - it('pressing load more button will load next page of comments', async () => { - const secondPageComment = mockCommentsPaged[1][0]; - mockAxiosReturnPagedComments(); - renderComponent(); - - const loadMoreButton = await findLoadMoreCommentsButton(); - fireEvent.click(loadMoreButton); - - await screen.findByText(secondPageComment.renderedBody); - }); - - it('newly loaded comments are appended to the old ones', async () => { - const firstPageComment = mockCommentsPaged[0][0]; - const secondPageComment = mockCommentsPaged[1][0]; - mockAxiosReturnPagedComments(); - renderComponent(); - - const loadMoreButton = await findLoadMoreCommentsButton(); - fireEvent.click(loadMoreButton); - - await screen.findByText(secondPageComment.renderedBody); - // check that comments from the first pages are also displayed - expect(screen.queryByText(firstPageComment.renderedBody)).toBeInTheDocument(); - }); - - it('load more button is hidden when no more comments pages to load', async () => { - const totalePages = mockCommentsPaged.length; - const lastPageComment = mockCommentsPaged[totalePages - 1][0]; - mockAxiosReturnPagedComments(); - renderComponent(); - - const loadMoreButton = await findLoadMoreCommentsButton(); - for (let page = 1; page < totalePages; page++) { + const loadMoreButton = await findLoadMoreCommentsButton(); fireEvent.click(loadMoreButton); - } - await screen.findByText(lastPageComment.renderedBody); - await expect(findLoadMoreCommentsButton()).rejects.toThrow(); + await screen.findByText('comment number 1', { exact: false }); + await screen.findByText('comment number 2', { exact: false }); + }); + + it('newly loaded comments are appended to the old ones', async () => { + renderComponent(discussionPostId); + + const loadMoreButton = await findLoadMoreCommentsButton(); + fireEvent.click(loadMoreButton); + + await screen.findByText('comment number 1', { exact: false }); + // check that comments from the first pages are also displayed + expect(screen.queryByText('comment number 2', { exact: false })) + .toBeInTheDocument(); + }); + + it('load more button is hidden when no more comments pages to load', async () => { + const totalPages = 2; + renderComponent(discussionPostId); + + const loadMoreButton = await findLoadMoreCommentsButton(); + for (let page = 1; page < totalPages; page++) { + fireEvent.click(loadMoreButton); + } + + await screen.findByText('comment number 2', { exact: false }); + await expect(findLoadMoreCommentsButton()) + .rejects + .toThrow(); + }); + }); + + describe('for question thread', () => { + const findLoadMoreCommentsButtons = () => screen.findAllByRole('button', { name: messages.loadMoreResponses.defaultMessage }); + it('initially loads only the first page', async () => { + act(() => renderComponent(questionPostId)); + expect(await screen.findByText('comment number 3', { exact: false })) + .toBeInTheDocument(); + expect(await screen.findByText('endorsed comment number 5', { exact: false })) + .toBeInTheDocument(); + expect(screen.queryByText('comment number 4', { exact: false })) + .not + .toBeInTheDocument(); + }); + + it('pressing load more button will load next page of comments', async () => { + await act(() => { + renderComponent(questionPostId); + }); + + const [loadMoreButtonEndorsed, loadMoreButtonUnendorsed] = await findLoadMoreCommentsButtons(); + // Both load more buttons should show + expect(await findLoadMoreCommentsButtons()).toHaveLength(2); + expect(await screen.findByText('unendorsed comment number 3', { exact: false })) + .toBeInTheDocument(); + expect(await screen.findByText('endorsed comment number 5', { exact: false })) + .toBeInTheDocument(); + // Comments from next page should not be loaded yet. + expect(await screen.queryByText('endorsed comment number 6', { exact: false })) + .not + .toBeInTheDocument(); + expect(await screen.queryByText('unendorsed comment number 4', { exact: false })) + .not + .toBeInTheDocument(); + + await act(() => { + fireEvent.click(loadMoreButtonEndorsed); + }); + // Endorsed comment from next page should be loaded now. + await waitFor(() => expect(screen.queryByText('endorsed comment number 6', { exact: false })) + .toBeInTheDocument()); + // Unndorsed comment from next page should not be loaded yet. + expect(await screen.queryByText('unendorsed comment number 4', { exact: false })) + .not + .toBeInTheDocument(); + // Now only one load more buttons should show, for unendorsed comments + expect(await findLoadMoreCommentsButtons()).toHaveLength(1); + await act(() => { + fireEvent.click(loadMoreButtonUnendorsed); + }); + // Unndorsed comment from next page should be loaded now. + await waitFor(() => expect(screen.queryByText('unendorsed comment number 4', { exact: false })) + .toBeInTheDocument()); + expect(findLoadMoreCommentsButtons()).rejects.toThrow(); + }); }); }); diff --git a/src/discussions/comments/comment/Comment.jsx b/src/discussions/comments/comment/Comment.jsx index f7ecd7f3..07ca1197 100644 --- a/src/discussions/comments/comment/Comment.jsx +++ b/src/discussions/comments/comment/Comment.jsx @@ -1,11 +1,14 @@ import React, { useEffect, useState } from 'react'; +import PropTypes from 'prop-types'; import { useDispatch, useSelector } from 'react-redux'; +import * as timeago from 'timeago.js'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; -import { Button } from '@edx/paragon'; +import { Alert, Button, Hyperlink } from '@edx/paragon'; +import { CheckCircle, Verified } from '@edx/paragon/icons'; -import { ContentActions } from '../../../data/constants'; +import { ContentActions, ThreadType } from '../../../data/constants'; import CommentIcons from '../comment-icons/CommentIcons'; import { selectCommentResponses } from '../data/selectors'; import { editComment, fetchCommentResponses, removeComment } from '../data/thunks'; @@ -14,7 +17,45 @@ import CommentEditor from './CommentEditor'; import CommentHeader from './CommentHeader'; import { commentShape } from './proptypes'; +function CommentBanner({ + intl, + comment, + postType, +}) { + const isQuestion = postType === ThreadType.QUESTION; + const classes = isQuestion ? 'bg-success-500 text-white' : 'bg-dark-500 text-white'; + const iconClass = isQuestion ? CheckCircle : Verified; + return comment.endorsed ? ( + +
+ {intl.formatMessage( + isQuestion + ? messages.answer + : messages.endorsed, + )} + + + {intl.formatMessage( + isQuestion + ? messages.answeredLabel + : messages.endorsedLabel, + )}  + {comment.endorsedBy}  + {timeago.format(comment.endorsedAt, intl.locale)} + +
+ + ) : null; +} + +CommentBanner.propTypes = { + intl: intlShape.isRequired, + comment: commentShape.isRequired, + postType: PropTypes.string.isRequired, +}; + function Comment({ + postType, comment, intl, }) { @@ -37,10 +78,10 @@ function Comment({ [ContentActions.DELETE]: () => dispatch(removeComment(comment.id)), [ContentActions.REPORT]: () => dispatch(editComment(comment.id, { flagged: !comment.abuseFlagged })), }; - return ( -
-
+
+ +
{isEditing ? ( @@ -55,9 +96,16 @@ function Comment({ voted={comment.voted} />
-
+
{/* Pass along intl since component used here is the one before it's injected with `injectIntl` */} - {inlineReplies.map(inlineReply => )} + {inlineReplies.map(inlineReply => ( + + ))}
{!isNested && ( @@ -82,6 +130,7 @@ function Comment({ } Comment.propTypes = { + postType: PropTypes.oneOf(['discussion', 'question']).isRequired, comment: commentShape.isRequired, intl: intlShape.isRequired, }; diff --git a/src/discussions/comments/comment/proptypes.js b/src/discussions/comments/comment/proptypes.js index 6ed225b7..68a86750 100644 --- a/src/discussions/comments/comment/proptypes.js +++ b/src/discussions/comments/comment/proptypes.js @@ -5,6 +5,9 @@ export const commentShape = PropTypes.shape({ createdAt: PropTypes.string, abuseFlagged: PropTypes.bool, renderedBody: PropTypes.string, + endorsedBy: PropTypes.string, + endorsedAt: PropTypes.string, + endorsed: PropTypes.bool, author: PropTypes.string, authorLabel: PropTypes.string, users: PropTypes.objectOf(PropTypes.shape({ diff --git a/src/discussions/comments/data/__factories__/comments.factory.js b/src/discussions/comments/data/__factories__/comments.factory.js index ec7108d6..d25c9539 100644 --- a/src/discussions/comments/data/__factories__/comments.factory.js +++ b/src/discussions/comments/data/__factories__/comments.factory.js @@ -2,8 +2,8 @@ import { Factory } from 'rosie'; Factory.define('comment') .sequence('id', (idx) => `comment-${idx}`) - .sequence('raw_body', (idx) => `Some contents for **comment number ${idx}**.`) - .sequence('rendered_body', (idx) => `Some contents for comment number ${idx}.`) + .sequence('raw_body', ['endorsed'], (idx, endorsed) => `Some contents for **${endorsed ? 'endorsed ' : 'unendorsed '}comment number ${idx}**.`) + .sequence('rendered_body', ['endorsed'], (idx, endorsed) => `Some contents for ${endorsed ? 'endorsed ' : 'unendorsed '}comment number ${idx}.`) .attr('thread_id', null, 'test-thread') .option('endorsedBy', null, null) .attr('endorsed', ['endorsedBy'], (endorsedBy) => !!endorsedBy) @@ -36,6 +36,7 @@ Factory.define('commentsResult') .option('pageSize', null, 5) .option('threadId', null, 'test-thread') .option('parentId', null, null) + .option('endorsed', null, null) .attr('pagination', ['threadId', 'count', 'page', 'pageSize'], (threadId, count, page, pageSize) => { const numPages = Math.ceil(count / pageSize); const next = (page < numPages) ? `http://test.site/api/discussion/v1/comments/?thread_id=${threadId}&page=${page + 1}` : null; @@ -47,7 +48,12 @@ Factory.define('commentsResult') num_pages: numPages, }; }) - .attr('results', ['count', 'pageSize', 'page', 'threadId', 'parentId'], (count, pageSize, page, threadId, parentId) => { + .attr('results', ['count', 'pageSize', 'page', 'threadId', 'parentId', 'endorsed'], (count, pageSize, page, threadId, parentId, endorsed) => { const len = (pageSize * page <= count) ? pageSize : count % pageSize; - return Factory.buildList('comment', len, { thread_id: threadId, parent_id: parentId }); + return Factory.buildList('comment', len, { + thread_id: threadId, + parent_id: parentId, + }, { + endorsedBy: endorsed ? 'staff' : null, + }); }); diff --git a/src/discussions/comments/data/api.js b/src/discussions/comments/data/api.js index 7d5ac19e..1f0ecb42 100644 --- a/src/discussions/comments/data/api.js +++ b/src/discussions/comments/data/api.js @@ -2,6 +2,8 @@ import { ensureConfig, getConfig, snakeCaseObject } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import { EndorsementValue } from '../../../data/constants'; + ensureConfig([ 'LMS_BASE_URL', ], 'Comments API service'); @@ -13,18 +15,21 @@ export const commentsApiUrl = `${apiBaseUrl}/api/discussion/v1/comments/`; /** * Returns all the comments for the specified thread. * @param {string} threadId + * @param {EndorsementStatus} endorsed * @param {number=} page * @param {number=} pageSize * @returns {Promise<{}>} */ export async function getThreadComments( threadId, { + endorsed, page, pageSize, } = {}, ) { const params = snakeCaseObject({ threadId, + endorsed: EndorsementValue[endorsed], page, pageSize, requestedFields: 'profile_image', diff --git a/src/discussions/comments/data/redux.test.js b/src/discussions/comments/data/redux.test.js index 51fa8c97..c74e8a32 100644 --- a/src/discussions/comments/data/redux.test.js +++ b/src/discussions/comments/data/redux.test.js @@ -4,6 +4,7 @@ import { Factory } from 'rosie'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { initializeMockApp } from '@edx/frontend-platform/testing'; +import { EndorsementStatus } from '../../../data/constants'; import { initializeStore } from '../../../store'; import { executeThunk } from '../../../test-utils'; import { commentsApiUrl } from './api'; @@ -36,17 +37,40 @@ describe('Comments/Responses data layer tests', () => { axiosMock.reset(); }); - test('successfully processes comments', async () => { + test.each([ + { + threadType: 'discussion', + endorsed: EndorsementStatus.DISCUSSION, + }, + { + threadType: 'question', + endorsed: EndorsementStatus.UNENDORSED, + }, + { + threadType: 'question', + endorsed: EndorsementStatus.ENDORSED, + }, + ])('successfully processes comments for \'$threadType\' thread with endorsed=$endorsed', async ({ + endorsed, + }) => { const threadId = 'test-thread'; axiosMock.onGet(commentsApiUrl) .reply(200, Factory.build('commentsResult')); - await executeThunk(fetchThreadComments(threadId), store.dispatch, store.getState); + await executeThunk(fetchThreadComments(threadId, { endorsed }), store.dispatch, store.getState); expect(store.getState().comments.commentsInThreads) - .toEqual({ 'test-thread': ['comment-1', 'comment-2', 'comment-3'] }); + .toEqual({ 'test-thread': { [endorsed]: ['comment-1', 'comment-2', 'comment-3'] } }); expect(store.getState().comments.pagination) - .toEqual({ 'test-thread': { currentPage: 1, totalPages: 1, hasMorePages: false } }); + .toEqual({ + 'test-thread': { + [endorsed]: { + currentPage: 1, + totalPages: 1, + hasMorePages: false, + }, + }, + }); expect(Object.keys(store.getState().comments.commentsById)) .toEqual(['comment-1', 'comment-2', 'comment-3']); expect(store.getState().comments.commentsById['comment-1']) @@ -76,7 +100,7 @@ describe('Comments/Responses data layer tests', () => { .toEqual({ 'comment-1': ['comment-4', 'comment-5', 'comment-6'] }); }); - test('successfully handles comment creation', async () => { + test('successfully handles comment creation for discussion type threads', async () => { const threadId = 'test-thread'; const content = 'Test comment'; axiosMock.onGet(commentsApiUrl) @@ -94,13 +118,68 @@ describe('Comments/Responses data layer tests', () => { await executeThunk(addComment(content, threadId, null), store.dispatch, store.getState); expect(store.getState().comments.commentsInThreads[threadId]) - .toEqual(['comment-1', 'comment-2', 'comment-3', 'comment-4']); + .toEqual({ + [EndorsementStatus.DISCUSSION]: [ + 'comment-1', + 'comment-2', + 'comment-3', + 'comment-4', + ], + }); expect(Object.keys(store.getState().comments.commentsById)) .toEqual(['comment-1', 'comment-2', 'comment-3', 'comment-4']); expect(store.getState().comments.commentsById['comment-4'].threadId) .toEqual(threadId); }); + test('successfully handles comment creation for question type threads', async () => { + const threadId = 'test-thread'; + const content = 'Test comment'; + axiosMock.onGet(commentsApiUrl) + .reply(200, Factory.build('commentsResult', null, { endorsed: false })); + await executeThunk( + fetchThreadComments(threadId, { endorsed: EndorsementStatus.UNENDORSED }), + store.dispatch, + store.getState, + ); + axiosMock.onGet(commentsApiUrl) + .reply(200, Factory.build('commentsResult', null, { endorsed: true })); + await executeThunk( + fetchThreadComments(threadId, { endorsed: EndorsementStatus.ENDORSED }), + store.dispatch, + store.getState, + ); + + axiosMock.onPost(`${commentsApiUrl}`) + .reply(200, Factory.build('comment', { + thread_id: threadId, + raw_body: content, + rendered_body: content, + })); + + await executeThunk(addComment(content, threadId, null), store.dispatch, store.getState); + + expect(store.getState().comments.commentsInThreads[threadId]) + .toEqual({ + [EndorsementStatus.UNENDORSED]: [ + 'comment-1', + 'comment-2', + 'comment-3', + // Newly-added comment + 'comment-7', + ], + [EndorsementStatus.ENDORSED]: [ + 'comment-4', + 'comment-5', + 'comment-6', + ], + }); + expect(Object.keys(store.getState().comments.commentsById)) + .toEqual(['comment-1', 'comment-2', 'comment-3', 'comment-4', 'comment-5', 'comment-6', 'comment-7']); + expect(store.getState().comments.commentsById['comment-7'].threadId) + .toEqual(threadId); + }); + test('successfully handles comment edits', async () => { const threadId = 'test-thread'; const commentId = 'comment-1'; diff --git a/src/discussions/comments/data/selectors.js b/src/discussions/comments/data/selectors.js index e4b8bd92..8aa6edf5 100644 --- a/src/discussions/comments/data/selectors.js +++ b/src/discussions/comments/data/selectors.js @@ -4,9 +4,9 @@ import { createSelector } from '@reduxjs/toolkit'; const selectCommentsById = state => state.comments.commentsById; const mapIdToComment = (ids, comments) => ids.map(id => comments[id]); -export const selectThreadComments = threadId => createSelector( +export const selectThreadComments = (threadId, endorsed = null) => createSelector( [ - state => state.comments.commentsInThreads[threadId] || [], + state => state.comments.commentsInThreads[threadId]?.[endorsed] || [], selectCommentsById, ], mapIdToComment, @@ -20,12 +20,12 @@ export const selectCommentResponses = commentId => createSelector( mapIdToComment, ); -export const selectThreadHasMorePages = threadId => ( - store => store.comments.pagination[threadId]?.hasMorePages || false +export const selectThreadHasMorePages = (threadId, endorsed = null) => ( + store => store.comments.pagination[threadId]?.[endorsed]?.hasMorePages || false ); -export const selectThreadCurrentPage = threadId => ( - store => store.comments.pagination[threadId]?.currentPage || null +export const selectThreadCurrentPage = (threadId, endorsed = null) => ( + store => store.comments.pagination[threadId]?.[endorsed]?.currentPage || null ); export const commentsStatus = state => state.comments.status; diff --git a/src/discussions/comments/data/slices.js b/src/discussions/comments/data/slices.js index 7b42478c..5a4320b9 100644 --- a/src/discussions/comments/data/slices.js +++ b/src/discussions/comments/data/slices.js @@ -1,7 +1,7 @@ /* eslint-disable no-param-reassign,import/prefer-default-export */ import { createSlice } from '@reduxjs/toolkit'; -import { RequestStatus } from '../../../data/constants'; +import { EndorsementStatus, RequestStatus } from '../../../data/constants'; const commentsSlice = createSlice({ name: 'comments', @@ -20,26 +20,29 @@ const commentsSlice = createSlice({ // TODO: save in localstorage so user can continue editing? commentDraft: null, postStatus: RequestStatus.SUCCESSFUL, - pagination: { - }, + pagination: {}, }, reducers: { fetchCommentsRequest: (state) => { state.status = RequestStatus.IN_PROGRESS; }, fetchCommentsSuccess: (state, { payload }) => { + const { threadId, endorsed } = payload; + // force endorsed to be null, true or false state.status = RequestStatus.SUCCESSFUL; - state.commentsInThreads[payload.threadId] = [ - ...(state.commentsInThreads[payload.threadId] || []), - ...(payload.commentsInThreads[payload.threadId] || []), + state.commentsInThreads[threadId] = state.commentsInThreads[threadId] ?? {}; + state.pagination[threadId] = state.pagination[threadId] ?? {}; + state.commentsInThreads[threadId][endorsed] = [ + ...(state.commentsInThreads[threadId][endorsed] || []), + ...(payload.commentsInThreads[threadId] || []), ]; - state.commentsInComments = { ...state.commentsInComments, ...payload.commentsInComments }; - state.commentsById = { ...state.commentsById, ...payload.commentsById }; - state.pagination[payload.threadId] = { + state.pagination[threadId][endorsed] = { currentPage: payload.page, totalPages: payload.pagination.numPages, hasMorePages: Boolean(payload.pagination.next), }; + state.commentsInComments = { ...state.commentsInComments, ...payload.commentsInComments }; + state.commentsById = { ...state.commentsById, ...payload.commentsById }; }, fetchCommentsFailed: (state) => { state.status = RequestStatus.FAILED; @@ -76,7 +79,12 @@ const commentsSlice = createSlice({ if (payload.parentId) { state.commentsInComments[payload.parentId].push(payload.id); } else { - state.commentsInThreads[payload.threadId].push(payload.id); + // The comment should be added to either the discussion or unendorsed + // sections since a new comment won't be endorsed yet. + ( + state.commentsInThreads[payload.threadId][EndorsementStatus.DISCUSSION] + ?? state.commentsInThreads[payload.threadId][EndorsementStatus.UNENDORSED] + ).push(payload.id); } state.commentsById[payload.id] = payload; state.commentDraft = null; @@ -108,8 +116,13 @@ const commentsSlice = createSlice({ deleteCommentSuccess: (state, { payload }) => { const { commentId } = payload; const { threadId, parentId } = state.commentsById[commentId]; + state.postStatus = RequestStatus.SUCCESSFUL; - state.commentsInThreads[threadId] = state.commentsInThreads[threadId].filter(item => item !== commentId); + [EndorsementStatus.DISCUSSION, EndorsementStatus.UNENDORSED, EndorsementStatus.ENDORSED].forEach((endorsed) => { + state.commentsInThreads[threadId][endorsed] = ( + state.commentsInThreads[threadId]?.[endorsed]?.filter(item => item !== commentId) + ); + }); if (parentId) { state.commentsInComments[parentId] = state.commentsInComments[parentId].filter(item => item !== commentId); } diff --git a/src/discussions/comments/data/thunks.js b/src/discussions/comments/data/thunks.js index 5eb8ebd2..27a77f4c 100644 --- a/src/discussions/comments/data/thunks.js +++ b/src/discussions/comments/data/thunks.js @@ -2,6 +2,7 @@ import { camelCaseObject } from '@edx/frontend-platform'; import { logError } from '@edx/frontend-platform/logging'; +import { EndorsementStatus } from '../../../data/constants'; import { getHttpErrorStatus } from '../../utils'; import { deleteComment, getCommentResponses, getThreadComments, postComment, updateComment, @@ -72,13 +73,14 @@ function normaliseComments(data) { }; } -export function fetchThreadComments(threadId, { page = 1 } = {}) { +export function fetchThreadComments(threadId, { page = 1, endorsed = EndorsementStatus.DISCUSSION } = {}) { return async (dispatch) => { try { - dispatch(fetchCommentsRequest({ threadId })); - const data = await getThreadComments(threadId, { page }); + dispatch(fetchCommentsRequest()); + const data = await getThreadComments(threadId, { page, endorsed }); dispatch(fetchCommentsSuccess({ ...normaliseComments(camelCaseObject(data)), + endorsed, page, threadId, })); diff --git a/src/discussions/comments/messages.js b/src/discussions/comments/messages.js index 442fbe41..3d729d8a 100644 --- a/src/discussions/comments/messages.js +++ b/src/discussions/comments/messages.js @@ -11,10 +11,10 @@ const messages = defineMessages({ defaultMessage: 'Add a comment', description: 'Button to add a comment to a response', }, - loadMoreComments: { - id: 'discussions.comments.comment.loadMoreComments', - defaultMessage: 'Load more comments', - description: 'Button to load more comments of forum posts', + loadMoreResponses: { + id: 'discussions.comments.comment.loadMoreResponses', + defaultMessage: 'Load more responses', + description: 'Button to load more responses of forum posts', }, postVisibility: { id: 'discussions.comments.comment.visibility', @@ -37,6 +37,26 @@ const messages = defineMessages({ defaultMessage: 'Posted {relativeTime}', description: 'Message about how long ago a comment was posted. Appears as "username posted 7 minutes ago"', }, + answer: { + id: 'discussions.comments.comment.answer', + defaultMessage: 'Answer', + description: 'Message above a comment that has been marked as the answer.', + }, + answeredLabel: { + id: 'discussions.comments.comment.answeredlabel', + defaultMessage: 'Marked as answered by', + description: 'Message above a comment that has been marked as answered. Appears as "Marked as answered by Username"', + }, + endorsed: { + id: 'discussions.comments.comment.endorsed', + defaultMessage: 'Endorsed', + description: 'Message above a comment that has been endorsed.', + }, + endorsedLabel: { + id: 'discussions.comments.comment.endorsedlabel', + defaultMessage: 'Endorsed by', + description: 'Message above a comment that has been endorsed. Appears as "Endorsed by Username"', + }, actionsAlt: { id: 'discussions.actions.label', defaultMessage: 'Actions menu', @@ -65,6 +85,10 @@ const messages = defineMessages({ id: 'discussions.editor.cancel', defaultMessage: 'Cancel', }, + commentError: { + id: 'discussions.editor.error.empty', + defaultMessage: 'Post content cannot be empty.', + }, }); export default messages; diff --git a/src/discussions/common/ActionsDropdown.jsx b/src/discussions/common/ActionsDropdown.jsx index 9f30b049..8607a5ec 100644 --- a/src/discussions/common/ActionsDropdown.jsx +++ b/src/discussions/common/ActionsDropdown.jsx @@ -9,7 +9,9 @@ import { import { MoreVert } from '@edx/paragon/icons'; import { ContentActions } from '../../data/constants'; +import { commentShape } from '../comments/comment/proptypes'; import messages from '../messages'; +import { postShape } from '../posts/post/proptypes'; import { useActions } from '../utils'; function ActionsDropdown({ @@ -48,13 +50,12 @@ function ActionsDropdown({ >
{actions.map(action => ( - <> + {action.action === ContentActions.DELETE - && } + && } { @@ -65,7 +66,7 @@ function ActionsDropdown({ > {intl.formatMessage(action.label)} - + ))}
@@ -75,7 +76,7 @@ function ActionsDropdown({ ActionsDropdown.propTypes = { intl: intlShape.isRequired, - commentOrPost: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.number])).isRequired, + commentOrPost: PropTypes.oneOfType([commentShape, postShape]).isRequired, disabled: PropTypes.bool, actionHandlers: PropTypes.objectOf(PropTypes.func).isRequired, }; diff --git a/src/discussions/discussions-home/DiscussionsHome.jsx b/src/discussions/discussions-home/DiscussionsHome.jsx index 6659404a..4dca8625 100644 --- a/src/discussions/discussions-home/DiscussionsHome.jsx +++ b/src/discussions/discussions-home/DiscussionsHome.jsx @@ -2,7 +2,7 @@ import React, { useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { - generatePath, Route, Switch, useHistory, useRouteMatch, + generatePath, Redirect, Route, Switch, useHistory, useRouteMatch, } from 'react-router'; import { PostActionsBar } from '../../components'; @@ -19,7 +19,7 @@ export default function DiscussionsHome() { const history = useHistory(); const { params } = useRouteMatch(Routes.DISCUSSIONS.PATH); const postEditorVisible = useSelector(state => state.threads.postEditorVisible); - const { params: { page } } = useRouteMatch(Routes.COMMENTS.PAGE); + const { params: { page } } = useRouteMatch(`${Routes.COMMENTS.PAGE}?`); const { params: { courseId, @@ -48,44 +48,47 @@ export default function DiscussionsHome() { topicId, }} > -
-
+
+
- -
+ +
+ +
+
+
-
-
- - { - postEditorVisible ? ( - - - - ) : ( - - - +
+ { + postEditorVisible ? ( + + - - - - - ) - } + ) : ( + + + + + + + + + ) + } +
diff --git a/src/discussions/posts/data/__factories__/threads.factory.js b/src/discussions/posts/data/__factories__/threads.factory.js index 8b936bad..ac8a322d 100644 --- a/src/discussions/posts/data/__factories__/threads.factory.js +++ b/src/discussions/posts/data/__factories__/threads.factory.js @@ -5,6 +5,7 @@ Factory.define('thread') .sequence('title', (idx) => `This is Thread-${idx}`) .sequence('raw_body', (idx) => `Some contents for **thread number ${idx}**.`) .sequence('rendered_body', (idx) => `Some contents for thread number ${idx}.`) + .sequence('type', (idx) => (idx % 2 === 1 ? 'discussion' : 'question')) .attr('comment_list_url', ['id'], (threadId) => `http://test.site/api/discussion/v1/comments/?thread_id=${threadId}`) .attrs({ created_at: () => (new Date()).toISOString(), @@ -29,7 +30,6 @@ Factory.define('thread') topic_id: 'some-topic', group_id: null, group_name: null, - type: 'discussion', abuse_flagged_count: 0, pinned: false, closed: false, diff --git a/src/discussions/posts/post-actions-bar/PostActionsBar.jsx b/src/discussions/posts/post-actions-bar/PostActionsBar.jsx index b0edfb43..7aa88602 100644 --- a/src/discussions/posts/post-actions-bar/PostActionsBar.jsx +++ b/src/discussions/posts/post-actions-bar/PostActionsBar.jsx @@ -19,7 +19,7 @@ function PostActionsBar({ intl }) { />