Skip to content

Commit 50b2e15

Browse files
authored
feat: Add support for grouping at subsections (#281)
For smaller courses, there is a feature under the new provider for grouping topics at the subsection level so that when navigating the course, all topics under a subsection show up in the sidebar instead of just the current unit. This implements that functionality by checking if the discussion is loaded in-context, and if grouping at subsection is enabled, and if so, displaying the current subsection's topics.
1 parent 947204f commit 50b2e15

File tree

9 files changed

+184
-28
lines changed

9 files changed

+184
-28
lines changed

src/data/selectors.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { createSelector } from '@reduxjs/toolkit';
44

5-
import { selectDiscussionProvider } from '../discussions/data/selectors';
5+
import { selectDiscussionProvider, selectGroupAtSubsection } from '../discussions/data/selectors';
66
import { DiscussionProvider } from './constants';
77

88
export const selectTopicContext = (topicId) => (state) => state.blocks.topics[topicId];
@@ -14,6 +14,18 @@ export const selectorForUnitSubsection = createSelector(
1414
blocks => key => blocks[blocks[key]?.parent],
1515
);
1616

17+
// If subsection grouping is enabled, and the current selection is a unit, then get the current subsection.
18+
export const selectCurrentCategoryGrouping = createSelector(
19+
selectDiscussionProvider,
20+
selectGroupAtSubsection,
21+
selectBlocks,
22+
(provider, groupAtSubsection, blocks) => blockId => (
23+
(provider !== 'openedx' || !groupAtSubsection || blocks[blockId]?.type !== 'vertical')
24+
? blockId
25+
: blocks[blockId].parent
26+
),
27+
);
28+
1729
export const selectChapters = (state) => state.blocks.chapters;
1830
export const selectTopicsUnderCategory = createSelector(
1931
selectDiscussionProvider,

src/discussions/data/hooks.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ import { AppContext } from '@edx/frontend-platform/react';
1111
import { breakpoints, useWindowSize } from '@edx/paragon';
1212

1313
import { Routes } from '../../data/constants';
14+
import { selectTopicsUnderCategory } from '../../data/selectors';
1415
import { fetchCourseBlocks } from '../../data/thunks';
16+
import { DiscussionContext } from '../common/context';
1517
import { clearRedirect } from '../posts/data';
1618
import { selectTopics } from '../topics/data/selectors';
1719
import { fetchCourseTopics } from '../topics/data/thunks';
@@ -158,3 +160,24 @@ export const useShowLearnersTab = () => {
158160
const allowedUsers = isAdmin || IsGroupTA || privileged || (userRoles.includes('Student') && userRoles.length > 1);
159161
return learnersTabEnabled && allowedUsers;
160162
};
163+
164+
/**
165+
* React hook that gets the current topic ID from the current topic or category.
166+
* The topicId in the DiscussionContext only return the direct topicId from the URL.
167+
* If the URL has the current block ID it cannot get the topicID from that. This hook
168+
* gets the topic ID from the URL if available, or from the current category otherwise.
169+
* It only returns an ID if a single ID is available, if navigating a subsection it
170+
* returns null.
171+
* @returns {null|string} A topic ID if a single one available in the current context.
172+
*/
173+
export const useCurrentDiscussionTopic = () => {
174+
const { topicId, category } = useContext(DiscussionContext);
175+
const topics = useSelector(selectTopicsUnderCategory)(category);
176+
if (topicId) {
177+
return topicId;
178+
}
179+
if (topics?.length === 1) {
180+
return topics[0];
181+
}
182+
return null;
183+
};
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import { render } from '@testing-library/react';
2+
import { IntlProvider } from 'react-intl';
3+
4+
import { initializeMockApp } from '@edx/frontend-platform';
5+
import { AppProvider } from '@edx/frontend-platform/react';
6+
7+
import { initializeStore } from '../../store';
8+
import { DiscussionContext } from '../common/context';
9+
import { useCurrentDiscussionTopic } from './hooks';
10+
11+
let store;
12+
initializeMockApp();
13+
describe('Hooks', () => {
14+
describe('useCurrentDiscussionTopic', () => {
15+
function ComponentWithHook() {
16+
const topic = useCurrentDiscussionTopic();
17+
return (
18+
<div>
19+
{String(topic)}
20+
</div>
21+
);
22+
}
23+
24+
function renderComponent({ topicId, category }) {
25+
return render(
26+
<IntlProvider locale="en">
27+
<AppProvider store={store}>
28+
<DiscussionContext.Provider
29+
value={{
30+
topicId,
31+
category,
32+
}}
33+
>
34+
<ComponentWithHook />
35+
</DiscussionContext.Provider>
36+
</AppProvider>
37+
</IntlProvider>,
38+
);
39+
}
40+
41+
beforeEach(() => {
42+
store = initializeStore({
43+
blocks: {
44+
blocks: {
45+
'some-unit-key': { topics: ['some-topic-0'], parent: 'some-sequence-key' },
46+
'some-sequence-key': { topics: ['some-topic-0'] },
47+
'another-sequence-key': { topics: ['some-topic-1', 'some-topic-2'] },
48+
'empty-key': { topics: [] },
49+
},
50+
},
51+
config: { provider: 'openedx' },
52+
});
53+
});
54+
55+
test('when topicId is in context', () => {
56+
const { queryByText } = renderComponent({ topicId: 'some-topic' });
57+
expect(queryByText('some-topic')).toBeInTheDocument();
58+
});
59+
60+
test('when the category is a unit', () => {
61+
const { queryByText } = renderComponent({ category: 'some-unit-key' });
62+
expect(queryByText('some-topic-0')).toBeInTheDocument();
63+
});
64+
65+
test('when the category is a sequence with one unit', () => {
66+
const { queryByText } = renderComponent({ category: 'some-sequence-key' });
67+
expect(queryByText('some-topic-0')).toBeInTheDocument();
68+
});
69+
70+
test('when the category is a sequence with multiple units', () => {
71+
const { queryByText } = renderComponent({ category: 'another-sequence-key' });
72+
expect(queryByText('null')).toBeInTheDocument();
73+
});
74+
75+
test('when the category is invalid', () => {
76+
const { queryByText } = renderComponent({ category: 'invalid-key' });
77+
expect(queryByText('null')).toBeInTheDocument();
78+
});
79+
80+
test('when the category has no topics', () => {
81+
const { queryByText } = renderComponent({ category: 'empty-key' });
82+
expect(queryByText('null')).toBeInTheDocument();
83+
});
84+
});
85+
});

src/discussions/data/selectors.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ export const selectDivisionSettings = state => state.config.settings;
2222

2323
export const selectBlackoutDate = state => state.config.blackouts;
2424

25+
export const selectGroupAtSubsection = state => state.config.groupAtSubsection;
26+
2527
export const selectModerationSettings = state => ({
2628
postCloseReasons: state.config.postCloseReasons,
2729
editReasons: state.config.editReasons,

src/discussions/data/slices.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const configSlice = createSlice({
1111
allowAnonymous: false,
1212
allowAnonymousToPeers: false,
1313
userRoles: [],
14+
groupAtSubsection: false,
1415
hasModerationPrivileges: false,
1516
isGroupTa: false,
1617
isUserAdmin: false,

src/discussions/posts/PostsView.jsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import PropTypes from 'prop-types';
44
import { useDispatch, useSelector } from 'react-redux';
55

66
import SearchInfo from '../../components/SearchInfo';
7-
import { selectTopicsUnderCategory } from '../../data/selectors';
7+
import { selectCurrentCategoryGrouping, selectTopicsUnderCategory } from '../../data/selectors';
88
import { DiscussionContext } from '../common/context';
99
import {
1010
selectAllThreads,
@@ -29,7 +29,10 @@ TopicPostsList.propTypes = {
2929
};
3030

3131
function CategoryPostsList({ category }) {
32-
const topicIds = useSelector(selectTopicsUnderCategory)(category);
32+
const { inContext } = useContext(DiscussionContext);
33+
const groupedCategory = useSelector(selectCurrentCategoryGrouping)(category);
34+
// If grouping at subsection is enabled, only apply it when browsing discussions in context in the learning MFE.
35+
const topicIds = useSelector(selectTopicsUnderCategory)(inContext ? groupedCategory : category);
3336
const posts = useSelector(selectTopicThreads(topicIds));
3437
return <PostsList posts={posts} topics={topicIds} />;
3538
}

src/discussions/posts/PostsView.test.jsx

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ let store;
2929
let axiosMock;
3030

3131
async function renderComponent({
32-
postId, topicId, category, myPosts,
32+
postId, topicId, category, myPosts, inContext = false,
3333
} = { myPosts: false }) {
3434
let path = generatePath(Routes.POSTS.ALL_POSTS, { courseId });
3535
let page;
@@ -56,6 +56,7 @@ async function renderComponent({
5656
topicId,
5757
category,
5858
page,
59+
inContext,
5960
}}
6061
>
6162
<Switch>
@@ -85,12 +86,6 @@ describe('PostsView', () => {
8586
roles: [],
8687
},
8788
});
88-
89-
store = initializeStore({
90-
blocks: { blocks: { 'test-usage-key': { topics: ['some-topic-2', 'some-topic-0'] } } },
91-
config: { hasModerationPrivileges: true },
92-
});
93-
store.dispatch(fetchConfigSuccess({}));
9489
Factory.resetAll();
9590
axiosMock = new MockAdapter(getAuthenticatedHttpClient());
9691
axiosMock.onGet(getCohortsApiUrl(courseId)).reply(200, Factory.buildList('cohort', 1));
@@ -109,36 +104,71 @@ describe('PostsView', () => {
109104
});
110105
});
111106

107+
function setupStore(data = {}) {
108+
const storeData = {
109+
blocks: { blocks: { 'test-usage-key': { topics: ['some-topic-2', 'some-topic-0'] } } },
110+
config: { hasModerationPrivileges: true },
111+
...data,
112+
};
113+
// console.log(storeData);
114+
store = initializeStore(storeData);
115+
store.dispatch(fetchConfigSuccess({}));
116+
}
117+
112118
describe('Basic', () => {
113119
test('displays a list of all posts', async () => {
120+
setupStore();
114121
await act(async () => {
115122
await renderComponent();
116123
});
117124
expect(screen.getAllByText(/this is thread-\d+/i)).toHaveLength(threadCount);
118125
});
119126

120127
test('displays a list of user posts', async () => {
128+
setupStore();
121129
await act(async () => {
122130
await renderComponent({ myPosts: true });
123131
});
124132
expect(screen.getAllByText('abc123')).toHaveLength(threadCount);
125133
});
126134

127135
test('displays a list of posts in a topic', async () => {
136+
setupStore();
128137
await act(async () => {
129138
await renderComponent({ topicId: 'some-topic-1' });
130139
});
131140
expect(screen.getAllByText(/this is thread-\d+ in topic some-topic-1/i)).toHaveLength(Math.ceil(threadCount / 3));
132141
});
133142

134-
test('displays a list of posts in a category', async () => {
135-
await act(async () => {
136-
await renderComponent({ category: 'test-usage-key' });
137-
});
138-
expect(screen.queryAllByText(/this is thread-\d+ in topic some-topic-1}/i)).toHaveLength(0);
139-
expect(screen.queryAllByText(/this is thread-\d+ in topic some-topic-2/i)).toHaveLength(Math.ceil(threadCount / 3));
140-
expect(screen.queryAllByText(/this is thread-\d+ in topic some-topic-0/i)).toHaveLength(Math.ceil(threadCount / 3));
141-
});
143+
test.each([true, false])(
144+
'displays a list of posts in a category with grouping at subsection = %s',
145+
async (grouping) => {
146+
setupStore({
147+
blocks: {
148+
blocks: {
149+
'test-usage-key': {
150+
type: 'vertical',
151+
topics: ['some-topic-2', 'some-topic-0'],
152+
parent: 'test-seq-key',
153+
},
154+
'test-seq-key': { type: 'sequential', topics: ['some-topic-0', 'some-topic-1', 'some-topic-2'] },
155+
},
156+
},
157+
config: { groupAtSubsection: grouping, hasModerationPrivileges: true, provider: 'openedx' },
158+
});
159+
await act(async () => {
160+
await renderComponent({ category: 'test-usage-key', inContext: true, p: true });
161+
});
162+
const topicThreadCount = Math.ceil(threadCount / 3);
163+
expect(screen.queryAllByText(/this is thread-\d+ in topic some-topic-2/i))
164+
.toHaveLength(topicThreadCount);
165+
expect(screen.queryAllByText(/this is thread-\d+ in topic some-topic-0/i))
166+
.toHaveLength(topicThreadCount);
167+
// When grouping is enabled, topic 1 will be shown, but not otherwise.
168+
expect(screen.queryAllByText(/this is thread-\d+ in topic some-topic-1/i))
169+
.toHaveLength(grouping ? topicThreadCount : 0);
170+
},
171+
);
142172
});
143173

144174
describe('Filtering', () => {
@@ -150,6 +180,7 @@ describe('PostsView', () => {
150180
}
151181

152182
beforeEach(async () => {
183+
setupStore();
153184
await act(async () => {
154185
await renderComponent();
155186
});

src/discussions/posts/post-editor/PostEditor.jsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { useDispatch, useSelector } from 'react-redux';
1010
import { useHistory, useLocation, useParams } from 'react-router-dom';
1111
import * as Yup from 'yup';
1212

13-
import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
13+
import { useIntl } from '@edx/frontend-platform/i18n';
1414
import { AppContext } from '@edx/frontend-platform/react';
1515
import {
1616
Button, Card, Form, Spinner, StatefulButton,
@@ -23,6 +23,7 @@ import PostPreviewPane from '../../../components/PostPreviewPane';
2323
import { useDispatchWithState } from '../../../data/hooks';
2424
import { selectCourseCohorts } from '../../cohorts/data/selectors';
2525
import { fetchCourseCohorts } from '../../cohorts/data/thunks';
26+
import { useCurrentDiscussionTopic } from '../../data/hooks';
2627
import {
2728
selectAnonymousPostingConfig,
2829
selectDivisionSettings,
@@ -77,9 +78,9 @@ DiscussionPostType.propTypes = {
7778
};
7879

7980
function PostEditor({
80-
intl,
8181
editExisting,
8282
}) {
83+
const intl = useIntl();
8384
const { authenticatedUser } = useContext(AppContext);
8485
const dispatch = useDispatch();
8586
const editorRef = useRef(null);
@@ -89,9 +90,9 @@ function PostEditor({
8990
const commentsPagePath = useCommentsPagePath();
9091
const {
9192
courseId,
92-
topicId,
9393
postId,
9494
} = useParams();
95+
const topicId = useCurrentDiscussionTopic();
9596

9697
const nonCoursewareTopics = useSelector(selectNonCoursewareTopics);
9798
const nonCoursewareIds = useSelector(selectNonCoursewareIds);
@@ -439,12 +440,11 @@ function PostEditor({
439440
}
440441

441442
PostEditor.propTypes = {
442-
intl: intlShape.isRequired,
443443
editExisting: PropTypes.bool,
444444
};
445445

446446
PostEditor.defaultProps = {
447447
editExisting: false,
448448
};
449449

450-
export default injectIntl(PostEditor);
450+
export default PostEditor;

0 commit comments

Comments
 (0)