diff --git a/package.json b/package.json index 527e60e0..59c9eabb 100644 --- a/package.json +++ b/package.json @@ -60,12 +60,14 @@ }, "devDependencies": { "@edx/frontend-build": "8.0.4", + "axios-mock-adapter": "1.20.0", "codecov": "3.8.3", "es-check": "5.2.3", "eslint-plugin-simple-import-sort": "7.0.0", "glob": "7.1.7", "husky": "7.0.2", "jest": "27.1.0", - "reactifex": "1.1.1" + "reactifex": "1.1.1", + "rosie": "2.1.0" } } diff --git a/src/data/constants.js b/src/data/constants.js index 558f4775..42758eae 100644 --- a/src/data/constants.js +++ b/src/data/constants.js @@ -2,6 +2,16 @@ import { getConfig } from '@edx/frontend-platform'; export const API_BASE_URL = getConfig().LMS_BASE_URL; +/** + * Enum for thread types. + * @readonly + * @enum {string} + */ +export const ThreadType = { + QUESTION: 'question', + DISCUSSION: 'discussion', +}; + /** * Enum for request status. * @readonly diff --git a/src/discussions/comments/data/__factories__/comments.factory.js b/src/discussions/comments/data/__factories__/comments.factory.js new file mode 100644 index 00000000..ec7108d6 --- /dev/null +++ b/src/discussions/comments/data/__factories__/comments.factory.js @@ -0,0 +1,53 @@ +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}.`) + .attr('thread_id', null, 'test-thread') + .option('endorsedBy', null, null) + .attr('endorsed', ['endorsedBy'], (endorsedBy) => !!endorsedBy) + .attr('endorsed_by', ['endorsedBy'], (endorsedBy) => endorsedBy) + .attr('endorsed_by_label', ['endorsedBy'], (endorsedBy) => (endorsedBy ? 'Staff' : null)) + .attr('endorsed_at', ['endorsedBy'], (endorsedBy) => (endorsedBy ? (new Date()).toISOString() : null)) + .attrs({ + author: 'edx', + author_label: 'Staff', + created_at: () => (new Date()).toISOString(), + updated_at: () => (new Date()).toISOString(), + abuse_flagged: false, + voted: false, + vote_count: 0, + editable_fields: [ + 'abuse_flagged', + 'endorsed', + 'raw_body', + 'voted', + ], + parent_id: null, + child_count: 0, + children: [], + abuse_flagged_any_user: false, + }); + +Factory.define('commentsResult') + .option('count', null, 3) + .option('page', null, 1) + .option('pageSize', null, 5) + .option('threadId', null, 'test-thread') + .option('parentId', 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; + const prev = (page > 1) ? `http://test.site/api/discussion/v1/comments/?thread_id=${threadId}&page=${page - 1}` : null; + return { + next, + prev, + count, + num_pages: numPages, + }; + }) + .attr('results', ['count', 'pageSize', 'page', 'threadId', 'parentId'], (count, pageSize, page, threadId, parentId) => { + const len = (pageSize * page <= count) ? pageSize : count % pageSize; + return Factory.buildList('comment', len, { thread_id: threadId, parent_id: parentId }); + }); diff --git a/src/discussions/comments/data/__factories__/index.js b/src/discussions/comments/data/__factories__/index.js new file mode 100644 index 00000000..a311a46c --- /dev/null +++ b/src/discussions/comments/data/__factories__/index.js @@ -0,0 +1 @@ +import './comments.factory'; diff --git a/src/discussions/comments/data/api.js b/src/discussions/comments/data/api.js index 32ba2132..762f4252 100644 --- a/src/discussions/comments/data/api.js +++ b/src/discussions/comments/data/api.js @@ -8,7 +8,7 @@ ensureConfig([ const apiBaseUrl = getConfig().LMS_BASE_URL; -const commentsApiUrl = `${apiBaseUrl}/api/discussion/v1/comments/`; +export const commentsApiUrl = `${apiBaseUrl}/api/discussion/v1/comments/`; /** * Returns all the comments for the specified thread. diff --git a/src/discussions/comments/data/redux.test.js b/src/discussions/comments/data/redux.test.js new file mode 100644 index 00000000..626ad714 --- /dev/null +++ b/src/discussions/comments/data/redux.test.js @@ -0,0 +1,158 @@ +import MockAdapter from 'axios-mock-adapter'; +import { Factory } from 'rosie'; + +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import { initializeMockApp } from '@edx/frontend-platform/testing'; + +import { initializeStore } from '../../../store'; +import { executeThunk } from '../../../test-utils'; +import { commentsApiUrl } from './api'; +import { + addComment, editComment, fetchCommentResponses, fetchThreadComments, removeComment, +} from './thunks'; + +import './__factories__'; + +let axiosMock; +let store; + +describe('Comments/Responses data layer tests', () => { + beforeEach(() => { + initializeMockApp({ + authenticatedUser: { + userId: 3, + username: 'abc123', + administrator: true, + roles: [], + }, + }); + + axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + Factory.resetAll(); + store = initializeStore(); + }); + + afterEach(() => { + axiosMock.reset(); + }); + + test('successfully processes comments', async () => { + const threadId = 'test-thread'; + axiosMock.onGet(commentsApiUrl) + .reply(200, Factory.build('commentsResult')); + + await executeThunk(fetchThreadComments(threadId), store.dispatch, store.getState); + + expect(store.getState().comments.commentsInThreads) + .toEqual({ 'test-thread': ['comment-1', 'comment-2', 'comment-3'] }); + expect(store.getState().comments.pages) + .toEqual({ 1: ['comment-1', 'comment-2', 'comment-3'] }); + expect(Object.keys(store.getState().comments.commentsById)) + .toEqual(['comment-1', 'comment-2', 'comment-3']); + expect(store.getState().comments.commentsById['comment-1']) + .toHaveProperty('threadId'); + expect(store.getState().comments.commentsById['comment-1']) + .toHaveProperty('parentId'); + expect(store.getState().comments.commentsById['comment-1'].threadId) + .toEqual('test-thread'); + }); + + test('successfully processes comment responses', async () => { + const threadId = 'test-thread'; + const commentId = 'comment-1'; + axiosMock.onGet(commentsApiUrl) + .reply(200, Factory.build('commentsResult')); + + await executeThunk(fetchThreadComments(threadId), store.dispatch, store.getState); + + axiosMock.onGet(`${commentsApiUrl}${commentId}/`) + .reply(200, Factory.build('commentsResult', null, { parentId: commentId })); + // get some comments into the store first + await executeThunk(fetchCommentResponses(commentId), store.dispatch, store.getState); + + expect(Object.keys(store.getState().comments.commentsById)) + .toHaveLength(6); + expect(store.getState().comments.commentsInComments) + .toEqual({ 'comment-1': ['comment-4', 'comment-5', 'comment-6'] }); + }); + + test('successfully handles comment creation', async () => { + const threadId = 'test-thread'; + const content = 'Test comment'; + axiosMock.onGet(commentsApiUrl) + .reply(200, Factory.build('commentsResult')); + + await executeThunk(fetchThreadComments(threadId), 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(['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 edits', async () => { + const threadId = 'test-thread'; + const commentId = 'comment-1'; + const newComment = 'Edited comment'; + axiosMock.onGet(commentsApiUrl) + .reply(200, Factory.build('commentsResult')); + + await executeThunk(fetchThreadComments(threadId), store.dispatch, store.getState); + + expect(store.getState().comments.commentsById[commentId].rawBody) + .not + .toEqual(newComment); + + axiosMock.onPatch(`${commentsApiUrl}${commentId}/`) + .reply(200, Factory.build('comment', { + id: commentId, + raw_body: newComment, + rendered_body: newComment, + })); + + await executeThunk(editComment(commentId, newComment), store.dispatch, store.getState); + + expect(store.getState().comments.commentsById[commentId].rawBody) + .toEqual(newComment); + }); + + test('successfully handles comment deletion', async () => { + const threadId = 'test-thread'; + const commentId = 'comment-1'; + axiosMock.onGet(commentsApiUrl) + .reply(200, Factory.build('commentsResult')); + + await executeThunk(fetchThreadComments(threadId), store.dispatch, store.getState); + + expect(store.getState().comments.commentsById) + .toHaveProperty(commentId); + expect(store.getState().comments.pages[1]) + .toContain(commentId); + + axiosMock.onDelete(`${commentsApiUrl}${commentId}/`) + .reply(201); + + await executeThunk(removeComment(commentId, threadId), store.dispatch, store.getState); + + expect(store.getState().comments.commentsById) + .not + .toHaveProperty(commentId); + expect(store.getState().comments.pages[1]) + .not + .toContain(commentId); + expect(store.getState().comments.commentsInThreads[threadId]) + .not + .toContain(commentId); + }); +}); diff --git a/src/discussions/comments/data/selectors.js b/src/discussions/comments/data/selectors.js index e4802501..a0f77f42 100644 --- a/src/discussions/comments/data/selectors.js +++ b/src/discussions/comments/data/selectors.js @@ -1,10 +1,23 @@ /* eslint-disable import/prefer-default-export */ -export const selectThreadComments = threadId => state => (state.comments.threadCommentMap[threadId] || []).map( - commentId => state.comments.comments[commentId], +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( + [ + state => state.comments.commentsInThreads[threadId] || [], + selectCommentsById, + ], + mapIdToComment, ); -export const selectCommentResponses = commentId => state => (state.comments.commentResponsesMap[commentId] || []).map( - cId => state.comments.comments[cId], +export const selectCommentResponses = commentId => createSelector( + [ + state => state.comments.commentsInComments[commentId] || [], + selectCommentsById, + ], + mapIdToComment, ); export const commentsStatus = state => state.comments.status; diff --git a/src/discussions/comments/data/slices.js b/src/discussions/comments/data/slices.js index f57162fe..2b286227 100644 --- a/src/discussions/comments/data/slices.js +++ b/src/discussions/comments/data/slices.js @@ -3,49 +3,20 @@ import { createSlice } from '@reduxjs/toolkit'; import { RequestStatus } from '../../../data/constants'; -function normaliseComments(state, rawCommentData) { - const { - threadCommentMap: threads, - commentResponsesMap: responses, - comments, - } = state; - rawCommentData.forEach( - comment => { - if (comment.parentId) { - if (!responses[comment.parentId]) { - responses[comment.parentId] = []; - } - if (!responses[comment.parentId].includes(comment.id)) { - responses[comment.parentId].push(comment.id); - } - } else { - if (!threads[comment.threadId]) { - threads[comment.threadId] = []; - } - - if (!threads[comment.threadId].includes(comment.id)) { - threads[comment.threadId].push(comment.id); - } - } - comments[comment.id] = comment; - }, - ); -} - const commentsSlice = createSlice({ name: 'comments', initialState: { status: RequestStatus.IN_PROGRESS, - page: null, - threadCommentMap: { + commentsInThreads: { // Maps threads to comment ids in them. }, - commentResponsesMap: { + commentsInComments: { // Maps comments to response comments in them. }, - comments: { + commentsById: { // Map comment ids to comments. }, + pages: {}, // Stores the comment being posted in case it needs to be reposted due to network failure. // TODO: save in localstorage so user can continue editing? commentDraft: null, @@ -59,8 +30,10 @@ const commentsSlice = createSlice({ }, fetchCommentsSuccess: (state, { payload }) => { state.status = RequestStatus.SUCCESSFUL; - normaliseComments(state, payload.results); - state.page = payload.pagination.page; + state.pages[payload.page] = payload.ids; + Object.assign(state.commentsInThreads, payload.commentsInThreads); + Object.assign(state.commentsInComments, payload.commentsInComments); + Object.assign(state.commentsById, payload.commentsById); state.totalPages = payload.pagination.numPages; state.totalThreads = payload.pagination.count; }, @@ -81,7 +54,8 @@ const commentsSlice = createSlice({ }, fetchCommentResponsesSuccess: (state, { payload }) => { state.status = RequestStatus.SUCCESSFUL; - normaliseComments(state, payload.results); + Object.assign(state.commentsInComments, payload.commentsInComments); + Object.assign(state.commentsById, payload.commentsById); }, postCommentRequest: (state, { payload }) => { state.postStatus = RequestStatus.IN_PROGRESS; @@ -95,7 +69,11 @@ const commentsSlice = createSlice({ }, postCommentSuccess: (state, { payload }) => { state.postStatus = RequestStatus.SUCCESSFUL; - normaliseComments(state, [payload]); + state.commentsInThreads[payload.threadId].push(payload.id); + if (payload.parentId) { + state.commentsInComments[payload.parentId].push(payload.id); + } + state.commentsById[payload.id] = payload; state.commentDraft = null; }, updateCommentRequest: (state, { payload }) => { @@ -110,7 +88,7 @@ const commentsSlice = createSlice({ }, updateCommentSuccess: (state, { payload }) => { state.status = RequestStatus.SUCCESSFUL; - normaliseComments(state, [payload]); + state.commentsById[payload.id] = payload; state.commentDraft = null; }, deleteCommentRequest: (state) => { @@ -124,10 +102,16 @@ const commentsSlice = createSlice({ }, deleteCommentSuccess: (state, { payload }) => { const { commentId } = payload; + const { threadId, parentId } = state.commentsById[commentId]; state.postStatus = RequestStatus.SUCCESSFUL; - const { threadId } = state.comments[commentId]; - state.threadCommentMap[threadId] = state.threadCommentMap[threadId].filter(item => item !== commentId); - delete state.comments[commentId]; + state.commentsInThreads[threadId] = state.commentsInThreads[threadId].filter(item => item !== commentId); + if (parentId) { + state.commentsInComments[parentId] = state.commentsInComments[parentId].filter(item => item !== commentId); + } + Object.keys(state.pages).forEach(page => { + state.pages[page] = state.pages[page].filter(item => item !== commentId); + }); + delete state.commentsById[commentId]; }, }, }); diff --git a/src/discussions/comments/data/thunks.js b/src/discussions/comments/data/thunks.js index 761ee813..827b362c 100644 --- a/src/discussions/comments/data/thunks.js +++ b/src/discussions/comments/data/thunks.js @@ -29,12 +29,58 @@ import { updateCommentSuccess, } from './slices'; -export function fetchThreadComments(threadId) { +/** + * Normalises comment data by mapping comments to ids, and grouping them by their + * parent thread and comment. + * @param data + * @returns {{commentsInComments: {}, pagination, commentsById: {}, commentsInThreads: {}}} + */ +function normaliseComments(data) { + const { pagination, results } = data; + const commentsInThreads = {}; + const commentsInComments = {}; + const commentsById = {}; + const ids = []; + results.forEach( + comment => { + const { parentId, threadId, id } = comment; + ids.push(id); + if (parentId) { + if (!commentsInComments[parentId]) { + commentsInComments[parentId] = []; + } + if (!commentsInComments[parentId].includes(id)) { + commentsInComments[parentId].push(id); + } + } else { + if (!commentsInThreads[threadId]) { + commentsInThreads[threadId] = []; + } + if (!commentsInThreads[threadId].includes(id)) { + commentsInThreads[threadId].push(id); + } + } + commentsById[id] = comment; + }, + ); + return { + ids, + commentsInThreads, + commentsInComments, + commentsById, + pagination, + }; +} + +export function fetchThreadComments(threadId, { page } = {}) { return async (dispatch) => { try { dispatch(fetchCommentsRequest({ threadId })); - const data = await getThreadComments(threadId); - dispatch(fetchCommentsSuccess(camelCaseObject(data))); + const data = await getThreadComments(threadId, { page }); + dispatch(fetchCommentsSuccess({ + ...normaliseComments(camelCaseObject(data)), + page: page || 1, + })); } catch (error) { if (getHttpErrorStatus(error) === 403) { dispatch(fetchCommentsDenied()); @@ -51,7 +97,7 @@ export function fetchCommentResponses(commentId) { try { dispatch(fetchCommentResponsesRequest({ commentId })); const data = await getCommentResponses(commentId); - dispatch(fetchCommentResponsesSuccess(camelCaseObject(data))); + dispatch(fetchCommentResponsesSuccess(normaliseComments(camelCaseObject(data)))); } catch (error) { if (getHttpErrorStatus(error) === 403) { dispatch(fetchCommentResponsesDenied()); diff --git a/src/discussions/posts/data/__factories__/index.js b/src/discussions/posts/data/__factories__/index.js new file mode 100644 index 00000000..c494f1f6 --- /dev/null +++ b/src/discussions/posts/data/__factories__/index.js @@ -0,0 +1 @@ +import './threads.factory'; diff --git a/src/discussions/posts/data/__factories__/threads.factory.js b/src/discussions/posts/data/__factories__/threads.factory.js new file mode 100644 index 00000000..8b936bad --- /dev/null +++ b/src/discussions/posts/data/__factories__/threads.factory.js @@ -0,0 +1,65 @@ +import { Factory } from 'rosie'; + +Factory.define('thread') + .sequence('id', (idx) => `thread-${idx}`) + .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}.`) + .attr('comment_list_url', ['id'], (threadId) => `http://test.site/api/discussion/v1/comments/?thread_id=${threadId}`) + .attrs({ + created_at: () => (new Date()).toISOString(), + updated_at: () => (new Date()).toISOString(), + editable_fields: [ + 'abuse_flagged', + 'following', + 'group_id', + 'raw_body', + 'read', + 'title', + 'topic_id', + 'type', + 'voted', + ], + author: 'test_user', + author_label: 'Staff', + abuse_flagged: false, + voted: false, + vote_count: 1, + course_id: 'course-v1:Test+TestX+Test_Course', + topic_id: 'some-topic', + group_id: null, + group_name: null, + type: 'discussion', + abuse_flagged_count: 0, + pinned: false, + closed: false, + following: false, + comment_count: 8, + unread_comment_count: 0, + endorsed_comment_list_url: null, + non_endorsed_comment_list_url: null, + read: false, + has_endorsed: false, + }); + +Factory.define('threadsResult') + .option('count', null, 3) + .option('page', null, 1) + .option('pageSize', null, 5) + .option('courseId', null, 'course-v1:Test+TestX+Test_Course') + .option('topicId', null, 'test-topic') + .attr('pagination', ['courseId', 'count', 'page', 'pageSize'], (courseId, count, page, pageSize) => { + const numPages = Math.ceil(count / pageSize); + const next = (page < numPages) ? `http://test.site/api/discussion/v1/threads/?course_id=${courseId}&page=${page + 1}` : null; + const prev = (page > 1) ? `http://test.site/api/discussion/v1/threads/?course_id=${courseId}&page=${page - 1}` : null; + return { + next, + prev, + count, + num_pages: numPages, + }; + }) + .attr('results', ['count', 'pageSize', 'page', 'courseId', 'topicId'], (count, pageSize, page, courseId, topicId) => { + const len = (pageSize * page <= count) ? pageSize : count % pageSize; + return Factory.buildList('thread', len, { course_id: courseId, topic_id: topicId }); + }); diff --git a/src/discussions/posts/data/api.js b/src/discussions/posts/data/api.js index a801c32b..ac6e74ee 100644 --- a/src/discussions/posts/data/api.js +++ b/src/discussions/posts/data/api.js @@ -10,7 +10,7 @@ ensureConfig([ const apiBaseUrl = getConfig().LMS_BASE_URL; -const threadsApiUrl = `${apiBaseUrl}/api/discussion/v1/threads/`; +export const threadsApiUrl = `${apiBaseUrl}/api/discussion/v1/threads/`; /** * Fetches all the threads in the given course and topic. @@ -67,7 +67,7 @@ export async function getThread(threadId) { * Posts a new thread. * @param {string} courseId * @param {string} topicId - * @param {string} type The thread's type (either "question" or "discussion") + * @param {ThreadType} type The thread's type (either "question" or "discussion") * @param {string} title * @param {string} content * @param {boolean} following Follow the thread after creating @@ -91,7 +91,7 @@ export async function postThread(courseId, topicId, type, title, content, follow * Updates an existing thread. * @param {string} threadId * @param {string} topicId - * @param {string} type The thread's type (either "question" or "discussion") + * @param {ThreadType} type The thread's type (either "question" or "discussion") * @param {string} title * @param {string} content * @param {boolean} flagged @@ -121,7 +121,6 @@ export async function updateThread(threadId, { raw_body: content, following, }); - const { data } = await getAuthenticatedHttpClient() .patch(url, patchData, { headers: { 'Content-Type': 'application/merge-patch+json' } }); return data; diff --git a/src/discussions/posts/data/redux.test.js b/src/discussions/posts/data/redux.test.js new file mode 100644 index 00000000..0bd3fc44 --- /dev/null +++ b/src/discussions/posts/data/redux.test.js @@ -0,0 +1,143 @@ +import MockAdapter from 'axios-mock-adapter'; +import { Factory } from 'rosie'; + +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import { initializeMockApp } from '@edx/frontend-platform/testing'; + +import { initializeStore } from '../../../store'; +import { executeThunk } from '../../../test-utils'; +import { threadsApiUrl } from './api'; +import { + createNewThread, fetchThread, fetchThreads, removeThread, updateExistingThread, +} from './thunks'; + +import './__factories__'; + +const courseId = 'course-v1:edX+TestX+Test_Course'; + +let axiosMock; +let store; + +describe('Threads/Posts data layer tests', () => { + beforeEach(() => { + initializeMockApp({ + authenticatedUser: { + userId: 3, + username: 'abc123', + administrator: true, + roles: [], + }, + }); + + axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + Factory.resetAll(); + store = initializeStore(); + }); + + afterEach(() => { + axiosMock.reset(); + }); + + test('successfully processes threads', async () => { + axiosMock.onGet(threadsApiUrl) + .reply(200, Factory.build('threadsResult')); + + await executeThunk(fetchThreads(courseId), store.dispatch, store.getState); + + expect(store.getState().threads.threadsInTopic) + .toEqual({ 'test-topic': ['thread-1', 'thread-2', 'thread-3'] }); + expect(store.getState().threads.pages) + .toEqual({ 1: ['thread-1', 'thread-2', 'thread-3'] }); + expect(Object.keys(store.getState().threads.threadsById)) + .toEqual(['thread-1', 'thread-2', 'thread-3']); + expect(store.getState().threads.threadsById['thread-1']) + .toHaveProperty('courseId'); + expect(store.getState().threads.threadsById['thread-1']) + .toHaveProperty('topicId'); + expect(store.getState().threads.threadsById['thread-1'].topicId) + .toEqual('test-topic'); + }); + + test('successfully processes single thread', async () => { + const threadId = 'thread-1'; + axiosMock.onGet(`${threadsApiUrl}${threadId}/`) + .reply(200, Factory.build('thread')); + + await executeThunk(fetchThread(threadId), store.dispatch, store.getState); + + expect(Object.keys(store.getState().threads.threadsById)) + .toEqual(['thread-1']); + expect(store.getState().threads.threadsById['thread-1']) + .toHaveProperty('courseId'); + expect(store.getState().threads.threadsById['thread-1']) + .toHaveProperty('topicId'); + expect(store.getState().threads.threadsById['thread-1'].topicId) + .toEqual('some-topic'); + }); + + test('successfully handles thread creation', async () => { + const topicId = 'test-topic'; + const title = 'A Test Thread'; + const content = 'Some test content'; + // pre-load thread results + axiosMock.onGet(threadsApiUrl) + .reply(200, Factory.build('threadsResult')); + await executeThunk(fetchThreads(courseId), store.dispatch, store.getState); + + axiosMock.onPost(`${threadsApiUrl}`) + .reply(200, Factory.build('thread', { + course_id: courseId, topic_id: topicId, title, raw_body: content, rendered_body: content, + })); + + await executeThunk(createNewThread(courseId, topicId, 'discussion', title, content), store.dispatch, store.getState); + + expect(store.getState().threads.threadsInTopic) + .toEqual({ [topicId]: ['thread-1', 'thread-2', 'thread-3', 'thread-4'] }); + expect(Object.keys(store.getState().threads.threadsById)) + .toEqual(['thread-1', 'thread-2', 'thread-3', 'thread-4']); + expect(store.getState().threads.threadsById['thread-1']) + .toHaveProperty('courseId'); + expect(store.getState().threads.threadsById['thread-1']) + .toHaveProperty('topicId'); + expect(store.getState().threads.threadsById['thread-1'].topicId) + .toEqual(topicId); + }); + + test('successfully handles thread updates', async () => { + const threadId = 'thread-2'; + axiosMock.onGet(threadsApiUrl) + .reply(200, Factory.build('threadsResult')); + await executeThunk(fetchThreads(courseId), store.dispatch, store.getState); + + expect(store.getState().threads.threadsById[threadId].voted) + .toEqual(false); + + axiosMock.onPatch(`${threadsApiUrl}${threadId}/`) + .reply(200, Factory.build('thread', { voted: true, id: threadId })); + await executeThunk(updateExistingThread(threadId, { voted: true }), store.dispatch, store.getState); + + expect(store.getState().threads.threadsById[threadId].voted) + .toEqual(true); + }); + + test('successfully handles thread deletion', async () => { + const threadId = 'thread-2'; + axiosMock.onGet(threadsApiUrl) + .reply(200, Factory.build('threadsResult')); + await executeThunk(fetchThreads(courseId), store.dispatch, store.getState); + + axiosMock.onDelete(`${threadsApiUrl}${threadId}/`) + .reply(201); + await executeThunk(removeThread(threadId), store.dispatch, store.getState); + + expect(store.getState().threads.threadsById) + .not + .toHaveProperty(threadId); + expect(store.getState().threads.pages[1]) + .not + .toContain(threadId); + expect(store.getState().threads.threadsInTopic['test-topic']) + .not + .toContain(threadId); + }); +}); diff --git a/src/discussions/posts/data/selectors.js b/src/discussions/posts/data/selectors.js index 01648b29..8a6e8e66 100644 --- a/src/discussions/posts/data/selectors.js +++ b/src/discussions/posts/data/selectors.js @@ -1,20 +1,54 @@ /* eslint-disable import/prefer-default-export */ -export const selectTopicThreads = topicId => state => (state.threads.topicThreadMap[topicId] || []).map( - threadId => state.threads.threads[threadId], +import { createSelector } from '@reduxjs/toolkit'; + +const selectThreads = state => state.threads.threadsById; + +const mapIdsToThreads = (ids, threads) => ids.map(id => threads?.[id]); + +export const selectTopicThreads = topicId => createSelector( + [ + state => state.threads.threadsInTopic[topicId] || [], + selectThreads, + ], + mapIdsToThreads, ); -export const selectThread = threadId => state => (state.threads.threads?.[threadId]); +export const selectThread = threadId => createSelector( + [selectThreads], + (threads) => threads?.[threadId], +); -export const selectAllThreads = () => state => Object.values(state.threads.threads); +export const selectAllThreadsOnPage = (page) => createSelector( + [ + state => state.threads.pages[page] || [], + selectThreads, + ], + mapIdsToThreads, +); + +export const selectAllThreads = () => state => { + let threads = []; + let page = 1; + while (state.threads.pages[page]?.length) { + threads = threads.concat(selectAllThreadsOnPage(page)(state)); + page += 1; + } + return threads; +}; export const threadsLoadingStatus = () => state => state.threads.status; -export const selectUserThreads = author => state => ( - Object.values(state.threads.threads) - .filter(thread => thread.author === author) +export const selectUserThreads = author => createSelector( + [selectThreads], + (threads) => Object.values(threads) + .filter(thread => thread.author === author), ); export const selectThreadSorting = () => state => state.threads.sortedBy; export const selectThreadFilters = () => state => state.threads.filters; + +export const selectAuthorAvatars = author => state => ( + state.threads.avatars?.[author].profile.image +); diff --git a/src/discussions/posts/data/slices.js b/src/discussions/posts/data/slices.js index b6676627..297f59d7 100644 --- a/src/discussions/posts/data/slices.js +++ b/src/discussions/posts/data/slices.js @@ -38,13 +38,16 @@ const threadsSlice = createSlice({ name: 'thread', initialState: { status: RequestStatus.IN_PROGRESS, - page: null, - topicThreadMap: { + avatars: { + // Mapping users to avatars + }, + threadsInTopic: { // Mapping of topic ids to thread ids in them }, - threads: { + threadsById: { // Mapping of threads ids to threads in them }, + pages: {}, threadDraft: null, totalPages: null, totalThreads: null, @@ -65,10 +68,10 @@ const threadsSlice = createSlice({ }, fetchThreadsSuccess: (state, { payload }) => { state.status = RequestStatus.SUCCESSFUL; - state.topicThreadMap = {}; - state.threads = {}; - normaliseThreads(state, payload.results); - state.page = payload.pagination.page; + state.pages[payload.page] = payload.ids; + Object.assign(state.threadsById, payload.threadsById); + Object.assign(state.threadsInTopic, payload.threadsInTopic); + Object.assign(state.avatars, payload.avatars); state.totalPages = payload.pagination.numPages; state.totalThreads = payload.pagination.count; }, @@ -83,7 +86,8 @@ const threadsSlice = createSlice({ }, fetchThreadSuccess: (state, { payload }) => { state.status = RequestStatus.SUCCESSFUL; - normaliseThreads(state, [payload]); + Object.assign(state.threadsById, payload.threadsById); + Object.assign(state.avatars, payload.avatars); }, fetchThreadFailed: (state) => { state.status = RequestStatus.FAILED; @@ -97,7 +101,11 @@ const threadsSlice = createSlice({ }, postThreadSuccess: (state, { payload }) => { state.postStatus = RequestStatus.SUCCESSFUL; - normaliseThreads(state, [payload]); + state.threadsById[payload.id] = payload; + state.threadsInTopic[payload.topicId].push(payload.id); + // Temporarily add it to the top of the list so it's visible + state.pages[1] = [payload.id] + (state.pages[1] || []); + Object.assign(state.avatars, payload.users); state.redirectToThread = { topicId: payload.topicId, threadId: payload.id }; state.threadDraft = null; }, @@ -113,7 +121,8 @@ const threadsSlice = createSlice({ }, updateThreadSuccess: (state, { payload }) => { state.postStatus = RequestStatus.SUCCESSFUL; - normaliseThreads(state, [payload]); + Object.assign(state.threadsById[payload.id], payload); + Object.assign(state.avatars, payload.avatars); state.threadDraft = null; }, updateThreadFailed: (state) => { @@ -127,10 +136,14 @@ const threadsSlice = createSlice({ }, deleteThreadSuccess: (state, { payload }) => { const { threadId } = payload; - const { topicId } = state.threads[threadId]; + const { topicId } = state.threadsById[threadId]; state.postStatus = RequestStatus.SUCCESSFUL; - state.topicThreadMap[topicId] = state.topicThreadMap[topicId].filter(item => item !== threadId); - delete state.threads[threadId]; + state.threadsInTopic[topicId] = state.threadsInTopic[topicId].filter(item => item !== threadId); + Object.keys(state.pages) + .forEach(page => { + state.pages[page] = state.pages[page].filter(item => item !== threadId); + }); + delete state.threadsById[threadId]; }, deleteThreadFailed: (state) => { state.postStatus = RequestStatus.FAILED; @@ -140,18 +153,23 @@ const threadsSlice = createSlice({ }, setSortedBy: (state, { payload }) => { state.sortedBy = payload; + state.pages = {}; }, setStatusFilter: (state, { payload }) => { state.filters.status = payload; + state.pages = {}; }, setAllPostsTypeFilter: (state, { payload }) => { state.filters.allPosts = payload; + state.pages = {}; }, setMyPostsTypeFilter: (state, { payload }) => { state.filters.myPosts = payload; + state.pages = {}; }, setSearchQuery: (state, { payload }) => { state.filters.search = payload; + state.pages = {}; }, showPostEditor: (state) => { state.postEditorVisible = true; diff --git a/src/discussions/posts/data/thunks.js b/src/discussions/posts/data/thunks.js index ca194bfc..95e3b157 100644 --- a/src/discussions/posts/data/thunks.js +++ b/src/discussions/posts/data/thunks.js @@ -37,6 +37,44 @@ import { * @property {AllPostsFilter} allPosts */ +/** + * Normalises raw data returned by threads API by mapping threads to id and + * mapping topic ids to threads in them. + * @param data + * @returns {{pagination, threadsById: {}, threadsInTopic: {}, avatars: {}}} + */ +function normaliseThreads(data) { + const normalized = {}; + let threads; + if ('results' in data) { + threads = data.results; + normalized.pagination = data.pagination; + } else { + threads = [data]; + } + const threadsInTopic = {}; + const threadsById = {}; + const avatars = {}; + const ids = []; + threads.forEach( + thread => { + const { topicId, id } = thread; + ids.push(id); + if (!threadsInTopic[topicId]) { + threadsInTopic[topicId] = []; + } + if (!threadsInTopic[topicId].includes(id)) { + threadsInTopic[topicId].push(id); + } + threadsById[id] = thread; + Object.assign(avatars, thread.users); + }, + ); + return { + ids, threadsById, threadsInTopic, avatars, ...normalized, + }; +} + /** * Fetches the threads for the course specified va the threadIds. * @param {string} courseId The course ID for the course to fetch data for. @@ -49,6 +87,7 @@ export function fetchThreads(courseId, { topicIds, orderBy, filters = {}, + page, } = {}) { const options = { orderBy, @@ -67,7 +106,8 @@ export function fetchThreads(courseId, { try { dispatch(fetchThreadsRequest({ courseId })); const data = await getThreads(courseId, options); - dispatch(fetchThreadsSuccess(camelCaseObject(data))); + const normalisedData = normaliseThreads(camelCaseObject(data)); + dispatch(fetchThreadsSuccess({ ...normalisedData, page: page || 1 })); } catch (error) { if (getHttpErrorStatus(error) === 403) { dispatch(fetchThreadsDenied()); @@ -84,7 +124,7 @@ export function fetchThread(threadId) { try { dispatch(fetchThreadRequest({ threadId })); const data = await getThread(threadId); - dispatch(fetchThreadSuccess(camelCaseObject(data))); + dispatch(fetchThreadSuccess(normaliseThreads(camelCaseObject(data)))); } catch (error) { if (getHttpErrorStatus(error) === 403) { dispatch(fetchThreadDenied()); diff --git a/src/discussions/posts/post/Post.jsx b/src/discussions/posts/post/Post.jsx index 60a21fd0..54a16a1a 100644 --- a/src/discussions/posts/post/Post.jsx +++ b/src/discussions/posts/post/Post.jsx @@ -1,7 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { useDispatch } from 'react-redux'; +import { useDispatch, useSelector } from 'react-redux'; import * as timeago from 'timeago.js'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; @@ -9,9 +9,16 @@ import { Avatar, Icon, IconButton, OverlayTrigger, Tooltip, } from '@edx/paragon'; import { - Pin, QuestionAnswer, StarFilled, StarOutline, + Help, + Pin, + Post as PostIcon, + QuestionAnswer, + StarFilled, + StarOutline, } from '@edx/paragon/icons'; +import { ThreadType } from '../../../data/constants'; +import { selectAuthorAvatars } from '../data/selectors'; import { updateExistingThread } from '../data/thunks'; import LikeButton from './LikeButton'; import messages from './messages'; @@ -35,8 +42,8 @@ export const postShape = PropTypes.shape({ function PostTypeIcon(props) { return (
- {props.type === 'question' && } - {props.type === 'discussion' && } + {props.type === ThreadType.QUESTION && } + {props.type === ThreadType.DISCUSSION && } {props.pinned && ( - +
diff --git a/src/store.js b/src/store.js index b30a8e36..d22ac18c 100644 --- a/src/store.js +++ b/src/store.js @@ -4,12 +4,16 @@ import { commentsReducer } from './discussions/comments/data'; import { threadsReducer } from './discussions/posts/data'; import { topicsReducer } from './discussions/topics/data'; -const store = configureStore({ - reducer: { - topics: topicsReducer, - threads: threadsReducer, - comments: commentsReducer, - }, -}); +export function initializeStore() { + return configureStore({ + reducer: { + topics: topicsReducer, + threads: threadsReducer, + comments: commentsReducer, + }, + }); +} + +const store = initializeStore(); export default store; diff --git a/src/test-utils.js b/src/test-utils.js new file mode 100644 index 00000000..46db3aa3 --- /dev/null +++ b/src/test-utils.js @@ -0,0 +1,5 @@ +/* eslint-disable import/prefer-default-export */ +export const executeThunk = async (thunk, dispatch, getState) => { + await thunk(dispatch, getState); + await new Promise(setImmediate); +};