refactor: update normalisation code to work in thunks instead of reducers

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"
   }
 }

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

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 <b>comment number ${idx}</b>.`)
+  .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 });
+  });

src/discussions/comments/data/__factories__/index.js

@@ -0,0 +1 @@
+import './comments.factory';

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.

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);
+  });
+});

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;

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];
     },
   },
 });

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());

src/discussions/posts/data/__factories__/index.js

@@ -0,0 +1 @@
+import './threads.factory';

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 <b>thread number ${idx}</b>.`)
+  .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 });
+  });

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;

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);
+  });
+});

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
+);

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;

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());

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 (
     <div className="m-1">
-      {props.type === 'question' && <Icon src={QuestionAnswer} size="lg" />}
-      {props.type === 'discussion' && <Icon src={QuestionAnswer} size="lg" />}
+      {props.type === ThreadType.QUESTION && <Icon src={Help} size="lg" />}
+      {props.type === ThreadType.DISCUSSION && <Icon src={PostIcon} size="lg" />}
       {props.pinned && (
         <Icon
           src={Pin}
@@ -55,9 +62,10 @@ function PostHeader({
   intl,
   post,
 }) {
+  const authorAvatars = useSelector(selectAuthorAvatars(post.author));
   return (
     <div className="d-flex flex-fill justify-content-between">
-      <Avatar className="m-2" alt={post.author} src={post?.authorAvatars?.imageUrlSmall} />
+      <Avatar className="m-2" alt={post.author} src={authorAvatars.imageUrlSmall} />
       <PostTypeIcon type={post.type} pinned={post.pinned} />
       <div className="align-items-center d-flex flex-row flex-fill">
         <div className="d-flex flex-column flex-fill">

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;

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);
+};