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.
This commit is contained in:
Kshitij Sobti
2022-04-20 17:56:03 +05:30
committed by GitHub
parent d1dce4f2ea
commit c57dfc1fc5
7 changed files with 77 additions and 63 deletions

View File

@@ -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])

View File

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

View File

@@ -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} />

View File

@@ -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>

View File

@@ -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;

View File

@@ -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]}

View File

@@ -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];