From c57dfc1fc57a8aa859f1d64b03649372837adba7 Mon Sep 17 00:00:00 2001
From: Kshitij Sobti <kshitij@sobti.in>
Date: Wed, 20 Apr 2022 17:56:03 +0530
Subject: [PATCH] fix: Fix constant reloading on topics page and posts links on
 category page [BD-38] [TNL-9868] [TNL-9846] (#128)

This fixes two issues. The first is that the topics page can cause constant requests to threads, and the second, that clicking on a post link when browsing a category can cause the application to crash.
---
 src/data/constants.js                         |  8 +-
 src/discussions/common/context.js             |  7 +-
 .../discussions-home/DiscussionSidebar.jsx    |  5 +-
 .../navigation-bar/NavigationBar.jsx          | 10 +-
 src/discussions/posts/PostsView.jsx           | 98 +++++++++----------
 src/discussions/posts/PostsView.test.jsx      | 10 +-
 src/discussions/posts/post/PostLink.jsx       |  2 +
 7 files changed, 77 insertions(+), 63 deletions(-)

diff --git a/src/data/constants.js b/src/data/constants.js
index 26e7f566..e3646279 100644
--- a/src/data/constants.js
+++ b/src/data/constants.js
@@ -179,6 +179,7 @@ export const Routes = {
       `${BASE_PATH}`,
     ],
     EDIT_POST: [
+      `${BASE_PATH}/category/:category/posts/:postId/edit`,
       `${BASE_PATH}/topics/:topicId/posts/:postId/edit`,
       `${BASE_PATH}/posts/:postId/edit`,
       `${BASE_PATH}/my-posts/:postId/edit`,
@@ -186,12 +187,14 @@ export const Routes = {
   },
   COMMENTS: {
     PATH: [
+      `${BASE_PATH}/category/:category/posts/:postId`,
       `${BASE_PATH}/topics/:topicId/posts/:postId`,
       `${BASE_PATH}/posts/:postId`,
       `${BASE_PATH}/my-posts/:postId`,
     ],
     PAGE: `${BASE_PATH}/:page`,
     PAGES: {
+      category: `${BASE_PATH}/category/:category/posts/:postId`,
       topics: `${BASE_PATH}/topics/:topicId/posts/:postId`,
       posts: `${BASE_PATH}/posts/:postId`,
       'my-posts': `${BASE_PATH}/my-posts/:postId`,
@@ -200,15 +203,18 @@ export const Routes = {
   TOPICS: {
     PATH: [
       `${BASE_PATH}/topics/:topicId?`,
+      `${BASE_PATH}/category/:category`,
+      `${BASE_PATH}/topics`,
     ],
     ALL: `${BASE_PATH}/topics`,
     CATEGORY: `${BASE_PATH}/category/:category`,
+    CATEGORY_POST: `${BASE_PATH}/category/:category/posts/:postId`,
     TOPIC: `${BASE_PATH}/topics/:topicId`,
   },
 };
 
 export const ALL_ROUTES = []
-  .concat([Routes.TOPICS.CATEGORY])
+  .concat([Routes.TOPICS.CATEGORY_POST, Routes.TOPICS.CATEGORY])
   .concat(Routes.COMMENTS.PATH)
   .concat(Routes.TOPICS.PATH)
   .concat([Routes.POSTS.ALL_POSTS, Routes.POSTS.MY_POSTS])
diff --git a/src/discussions/common/context.js b/src/discussions/common/context.js
index 19003713..df63df36 100644
--- a/src/discussions/common/context.js
+++ b/src/discussions/common/context.js
@@ -2,10 +2,11 @@
 import React from 'react';
 
 export const DiscussionContext = React.createContext({
+  page: null,
   courseId: null,
   postId: null,
-  category: null,
-  commentId: null,
-  learnerUsername: null,
+  topicId: null,
   inContext: false,
+  category: null,
+  learnerUsername: null,
 });
diff --git a/src/discussions/discussions-home/DiscussionSidebar.jsx b/src/discussions/discussions-home/DiscussionSidebar.jsx
index c1825047..40e4bcd0 100644
--- a/src/discussions/discussions-home/DiscussionSidebar.jsx
+++ b/src/discussions/discussions-home/DiscussionSidebar.jsx
@@ -24,11 +24,8 @@ export default function DiscussionSidebar({ displaySidebar }) {
       data-testid="sidebar"
     >
       <Switch>
-        <Route path={Routes.POSTS.MY_POSTS}>
-          <PostsView showOwnPosts />
-        </Route>
         <Route
-          path={[Routes.POSTS.PATH, Routes.POSTS.ALL_POSTS, Routes.TOPICS.CATEGORY]}
+          path={[Routes.POSTS.PATH, Routes.POSTS.ALL_POSTS, Routes.TOPICS.CATEGORY, Routes.POSTS.MY_POSTS]}
           component={PostsView}
         />
         <Route path={Routes.TOPICS.PATH} component={TopicsView} />
diff --git a/src/discussions/navigation/navigation-bar/NavigationBar.jsx b/src/discussions/navigation/navigation-bar/NavigationBar.jsx
index c45effc3..5ec94178 100644
--- a/src/discussions/navigation/navigation-bar/NavigationBar.jsx
+++ b/src/discussions/navigation/navigation-bar/NavigationBar.jsx
@@ -1,7 +1,7 @@
 import React from 'react';
 
 import { useSelector } from 'react-redux';
-import { useParams } from 'react-router';
+import { matchPath, useParams } from 'react-router';
 import { NavLink } from 'react-router-dom';
 
 import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
@@ -27,6 +27,7 @@ function NavigationBar({ intl }) {
     },
     {
       route: Routes.TOPICS.ALL,
+      isActive: (match, location) => Boolean(matchPath(location.pathname, { path: Routes.TOPICS.PATH })),
       labelMessage: messages.allTopics,
     },
   ];
@@ -41,7 +42,12 @@ function NavigationBar({ intl }) {
     <Nav variant="pills" className="py-2">
       {navLinks.map(link => (
         <Nav.Item key={link.route}>
-          <Nav.Link as={NavLink} to={discussionsPath(link.route, { courseId })} className="border">
+          <Nav.Link
+            as={NavLink}
+            to={discussionsPath(link.route, { courseId })}
+            className="border"
+            isActive={link.isActive}
+          >
             {intl.formatMessage(link.labelMessage)}
           </Nav.Link>
         </Nav.Item>
diff --git a/src/discussions/posts/PostsView.jsx b/src/discussions/posts/PostsView.jsx
index 84cff296..52c1d60f 100644
--- a/src/discussions/posts/PostsView.jsx
+++ b/src/discussions/posts/PostsView.jsx
@@ -23,7 +23,32 @@ import PostFilterBar from './post-filter-bar/PostFilterBar';
 import NoResults from './NoResults';
 import { PostLink } from './post';
 
-function PostsList({ posts }) {
+function PostsList({ posts, topics }) {
+  const dispatch = useDispatch();
+  const {
+    courseId,
+    page,
+  } = useContext(DiscussionContext);
+  const loadingStatus = useSelector(threadsLoadingStatus());
+  const { authenticatedUser } = useContext(AppContext);
+  const orderBy = useSelector(selectThreadSorting());
+  const filters = useSelector(selectThreadFilters());
+  const nextPage = useSelector(selectThreadNextPage());
+  const showOwnPosts = page === 'my-posts';
+
+  const loadThreads = (topicIds, pageNum = undefined) => dispatch(fetchThreads(courseId, {
+    topicIds,
+    orderBy,
+    filters,
+    page: pageNum,
+    author: showOwnPosts ? authenticatedUser.username : null,
+  }));
+
+  useEffect(() => {
+    if (topics !== undefined) {
+      loadThreads(topics);
+    }
+  }, [courseId, orderBy, filters, page, JSON.stringify(topics)]);
   let lastPinnedIdx = null;
   const postInstances = posts && posts.map((post, idx) => {
     if (post.pinned && lastPinnedIdx !== false) {
@@ -44,6 +69,18 @@ function PostsList({ posts }) {
     <>
       {postInstances}
       {posts && posts.length === 0 && <NoResults />}
+      {loadingStatus === RequestStatus.IN_PROGRESS ? (
+        <div className="d-flex justify-content-center p-4">
+          <Spinner animation="border" variant="primary" size="lg" />
+        </div>
+      ) : (
+        nextPage && (
+          <ScrollThreshold onScroll={() => {
+            loadThreads(topics, nextPage);
+          }}
+          />
+        )
+      )}
     </>
   );
 }
@@ -53,20 +90,22 @@ PostsList.propTypes = {
     pinned: PropTypes.bool.isRequired,
     id: PropTypes.string.isRequired,
   })),
+  topics: PropTypes.arrayOf(PropTypes.string),
 };
 
 PostsList.defaultProps = {
   posts: [],
+  topics: undefined,
 };
 
 function AllPostsList() {
   const posts = useSelector(selectAllThreads);
-  return <PostsList posts={posts} />;
+  return <PostsList posts={posts} topics={null} />;
 }
 
 function TopicPostsList({ topicId }) {
   const posts = useSelector(selectTopicThreads([topicId]));
-  return <PostsList posts={posts} />;
+  return <PostsList posts={posts} topics={[topicId]} />;
 }
 
 TopicPostsList.propTypes = {
@@ -76,27 +115,22 @@ TopicPostsList.propTypes = {
 function CategoryPostsList({ category }) {
   const topicIds = useSelector(selectTopicsUnderCategory)(category);
   const posts = useSelector(selectTopicThreads(topicIds));
-  return <PostsList posts={posts} />;
+  return <PostsList posts={posts} topics={topicIds} />;
 }
 
 CategoryPostsList.propTypes = {
   category: PropTypes.string.isRequired,
 };
 
-function PostsView({ showOwnPosts }) {
+function PostsView() {
   const {
-    courseId,
     topicId,
     category,
+    page,
   } = useContext(DiscussionContext);
-  const dispatch = useDispatch();
-  const { authenticatedUser } = useContext(AppContext);
-  const orderBy = useSelector(selectThreadSorting());
-  const filters = useSelector(selectThreadFilters());
-  const nextPage = useSelector(selectThreadNextPage());
-  const loadingStatus = useSelector(threadsLoadingStatus());
-  const topicIds = null;
-  let postsListComponent = null;
+
+  let postsListComponent;
+  const showOwnPosts = page === 'my-posts';
 
   if (topicId) {
     postsListComponent = <TopicPostsList topicId={topicId} />;
@@ -106,53 +140,17 @@ function PostsView({ showOwnPosts }) {
     postsListComponent = <AllPostsList />;
   }
 
-  useEffect(() => {
-    // The courseId from the URL is the course we WANT to load.
-    dispatch(fetchThreads(courseId, {
-      topicIds,
-      orderBy,
-      filters,
-      author: showOwnPosts ? authenticatedUser.username : null,
-    }));
-  }, [courseId, orderBy, filters, showOwnPosts, topicId, category]);
-
-  const loadMorePosts = async () => {
-    if (nextPage) {
-      dispatch(fetchThreads(courseId, {
-        topicIds,
-        orderBy,
-        filters,
-        page: nextPage,
-        author: showOwnPosts ? authenticatedUser.username : null,
-      }));
-    }
-  };
-
   return (
     <div className="discussion-posts d-flex flex-column">
       <PostFilterBar filterSelfPosts={showOwnPosts} />
       <div className="list-group list-group-flush">
         {postsListComponent}
-        {loadingStatus === RequestStatus.IN_PROGRESS ? (
-          <div className="d-flex justify-content-center p-4">
-            <Spinner animation="border" variant="primary" size="lg" />
-          </div>
-        ) : (
-          nextPage && (
-            <ScrollThreshold onScroll={loadMorePosts} />
-          )
-        )}
       </div>
     </div>
   );
 }
 
 PostsView.propTypes = {
-  showOwnPosts: PropTypes.bool,
-};
-
-PostsView.defaultProps = {
-  showOwnPosts: false,
 };
 
 export default PostsView;
diff --git a/src/discussions/posts/PostsView.test.jsx b/src/discussions/posts/PostsView.test.jsx
index 5e73ad94..1f626c06 100644
--- a/src/discussions/posts/PostsView.test.jsx
+++ b/src/discussions/posts/PostsView.test.jsx
@@ -29,16 +29,19 @@ async function renderComponent({
   postId, topicId, category, myPosts,
 } = { myPosts: false }) {
   let path = generatePath(Routes.POSTS.ALL_POSTS, { courseId });
-  let showOwnPosts = false;
+  let page;
   if (postId) {
     path = generatePath(Routes.POSTS.ALL_POSTS, { courseId, postId });
+    page = 'posts';
   } else if (topicId) {
     path = generatePath(Routes.POSTS.PATH, { courseId, topicId });
+    page = 'posts';
   } else if (category) {
     path = generatePath(Routes.TOPICS.CATEGORY, { courseId, category });
+    page = 'category';
   } else if (myPosts) {
     path = generatePath(Routes.POSTS.MY_POSTS, { courseId });
-    showOwnPosts = myPosts;
+    page = 'my-posts';
   }
   await render(
     <IntlProvider locale="en">
@@ -49,11 +52,12 @@ async function renderComponent({
             postId,
             topicId,
             category,
+            page,
           }}
           >
             <Switch>
               <Route path={Routes.POSTS.MY_POSTS}>
-                <PostsView showOwnPosts={showOwnPosts} />
+                <PostsView />
               </Route>
               <Route
                 path={[Routes.POSTS.PATH, Routes.POSTS.ALL_POSTS, Routes.TOPICS.CATEGORY]}
diff --git a/src/discussions/posts/post/PostLink.jsx b/src/discussions/posts/post/PostLink.jsx
index 43e3d0b7..bc8454d9 100644
--- a/src/discussions/posts/post/PostLink.jsx
+++ b/src/discussions/posts/post/PostLink.jsx
@@ -24,12 +24,14 @@ function PostLink({
     page,
     postId,
     inContext,
+    category,
   } = useContext(DiscussionContext);
   const linkUrl = discussionsPath(Routes.COMMENTS.PAGES[page], {
     0: inContext ? 'in-context' : undefined,
     courseId: post.courseId,
     topicId: post.topicId,
     postId: post.id,
+    category,
   });
   const showAnsweredBadge = post.hasEndorsed && post.type === ThreadType.QUESTION;
   const authorLabelColor = AvatarBorderAndLabelColors[post.authorLabel];