diff --git a/src/components/FilterBar.jsx b/src/components/FilterBar.jsx index 05dc4b0a..b86e45bf 100644 --- a/src/components/FilterBar.jsx +++ b/src/components/FilterBar.jsx @@ -120,28 +120,27 @@ function FilterBar({
{filters.map((value) => ( - { - value.filters.map(filterName => { - const element = allFilters.find(obj => obj.id === filterName); - if (element) { - return ( - - ); - } - return false; - }) - } - + {value.filters.map(filterName => { + const element = allFilters.find(obj => obj.id === filterName); + if (element) { + return ( + + ); + } + return false; + })} ))}
diff --git a/src/components/NavigationBar/data/selectors.js b/src/components/NavigationBar/data/selectors.js new file mode 100644 index 00000000..0c371c51 --- /dev/null +++ b/src/components/NavigationBar/data/selectors.js @@ -0,0 +1,3 @@ +/* eslint-disable import/prefer-default-export */ + +export const selectCourseTabs = state => state.courseTabs; diff --git a/src/data/constants.js b/src/data/constants.js index ddc35fbf..7420940a 100644 --- a/src/data/constants.js +++ b/src/data/constants.js @@ -190,6 +190,8 @@ export const Routes = { CATEGORY: `${BASE_PATH}/category/:category`, CATEGORY_POST: `${BASE_PATH}/category/:category/posts/:postId`, TOPIC: `${BASE_PATH}/topics/:topicId`, + TOPIC_POST: `${BASE_PATH}/topics/:topicId/posts/:postId`, + TOPIC_POST_EDIT: `${BASE_PATH}/topics/:topicId/posts/:postId/edit`, }, }; diff --git a/src/data/selectors.js b/src/data/selectors.js index aea0b5bf..afc4eb2e 100644 --- a/src/data/selectors.js +++ b/src/data/selectors.js @@ -38,12 +38,6 @@ export const selectTopicsUnderCategory = createSelector( ), ); -export const selectSequences = createSelector( - selectChapters, - selectBlocks, - (chapterIds, blocks) => chapterIds?.flatMap(cId => blocks[cId].children.map(seqId => blocks[seqId])) || [], -); - export const selectArchivedTopics = createSelector( state => state.topics.topics, state => state.topics.archivedIds || [], diff --git a/src/discussions/comments/CommentsView.jsx b/src/discussions/comments/CommentsView.jsx index 79ccf805..431eb55e 100644 --- a/src/discussions/comments/CommentsView.jsx +++ b/src/discussions/comments/CommentsView.jsx @@ -159,7 +159,7 @@ function CommentsView({ intl }) { const location = useLocation(); const isOnDesktop = useIsOnDesktop(); const { - courseId, learnerUsername, category, topicId, page, inContext, + courseId, learnerUsername, category, topicId, page, enableInContextSidebar, } = useContext(DiscussionContext); useEffect(() => { @@ -180,7 +180,7 @@ function CommentsView({ intl }) { return ( <> {!isOnDesktop && ( - inContext ? ( + enableInContextSidebar ? ( <>
)} - {inContext && ( + {enableInContextSidebar && ( <>
( + `${section.displayName} / ${subsection.displayName}` || intl.formatMessage(messages.unnamedTopics) + ); + return ( {nonCoursewareTopics.map(topic => ( ))} - {coursewareTopics.map(categoryObj => ( - - {categoryObj.topics.map(subtopic => ( - - ))} - - ))} + {enableInContext ? ( + coursewareTopics?.map(section => ( + section?.children?.map(subsection => ( + + {subsection?.children?.map(unit => ( + + ))} + + )) + )) + ) : ( + coursewareTopics.map(categoryObj => ( + + {categoryObj.topics.map(subtopic => ( + + ))} + + )) + )} {canSelectCohort(values.topic) && ( diff --git a/src/discussions/posts/post/Post.jsx b/src/discussions/posts/post/Post.jsx index 3a5a43fa..7cf2159c 100644 --- a/src/discussions/posts/post/Post.jsx +++ b/src/discussions/posts/post/Post.jsx @@ -58,7 +58,7 @@ function Post({ hideReportConfirmation(); }; - const { inContext } = useContext(DiscussionContext); + const { enableInContextSidebar } = useContext(DiscussionContext); const actionHandlers = { [ContentActions.EDIT_CONTENT]: () => history.push({ ...location, @@ -111,14 +111,14 @@ function Post({
{topicContext && topic && (
{intl.formatMessage(messages.relatedTo)}{' '} - {inContext + {enableInContextSidebar ? ( <> {topicContext.chapterName} diff --git a/src/discussions/posts/post/PostLink.jsx b/src/discussions/posts/post/PostLink.jsx index 9aedd4e6..172ef2e9 100644 --- a/src/discussions/posts/post/PostLink.jsx +++ b/src/discussions/posts/post/PostLink.jsx @@ -29,12 +29,12 @@ function PostLink({ const { page, postId, - inContext, + enableInContextSidebar, category, learnerUsername, } = useContext(DiscussionContext); const linkUrl = discussionsPath(Routes.COMMENTS.PAGES[page], { - 0: inContext ? 'in-context' : undefined, + 0: enableInContextSidebar ? 'in-context' : undefined, courseId: post.courseId, topicId: post.topicId, postId: post.id, diff --git a/src/discussions/topics/TopicsView.jsx b/src/discussions/topics/TopicsView.jsx index ce4a2ef8..fd0fbe62 100644 --- a/src/discussions/topics/TopicsView.jsx +++ b/src/discussions/topics/TopicsView.jsx @@ -4,17 +4,15 @@ import { useDispatch, useSelector } from 'react-redux'; import { useParams } from 'react-router'; import SearchInfo from '../../components/SearchInfo'; -import { DiscussionProvider, RequestStatus } from '../../data/constants'; -import { selectSequences } from '../../data/selectors'; +import { RequestStatus } from '../../data/constants'; import { DiscussionContext } from '../common/context'; import { selectDiscussionProvider } from '../data/selectors'; import NoResults from '../posts/NoResults'; +import { handleKeyDown } from '../utils'; import { selectCategories, selectNonCoursewareTopics, selectTopicFilter } from './data/selectors'; import { setFilter, setTopicsCount } from './data/slices'; import { fetchCourseTopics } from './data/thunks'; -import ArchivedTopicGroup from './topic-group/ArchivedTopicGroup'; import LegacyTopicGroup from './topic-group/LegacyTopicGroup'; -import SequenceTopicGroup from './topic-group/SequenceTopicGroup'; import Topic from './topic-group/topic/Topic'; import countFilteredTopics from './utils'; @@ -38,24 +36,6 @@ function CourseWideTopics() { )); } -function CoursewareTopics() { - const sequences = useSelector(selectSequences); - - return ( - <> - { sequences?.map( - sequence => ( - - ), - )} - - - ); -} - function LegacyCoursewareTopics() { const { category } = useParams(); const categories = useSelector(selectCategories) @@ -80,20 +60,6 @@ function TopicsView() { const { courseId } = useContext(DiscussionContext); const dispatch = useDispatch(); - const handleKeyDown = (event) => { - const { key } = event; - if (key !== 'ArrowDown' && key !== 'ArrowUp') { return; } - const option = event.target; - - let selectedOption; - if (key === 'ArrowDown') { selectedOption = option.nextElementSibling; } - if (key === 'ArrowUp') { selectedOption = option.previousElementSibling; } - - if (selectedOption) { - selectedOption.focus(); - } - }; - useEffect(() => { // Don't load till the provider information is available if (provider) { @@ -118,8 +84,7 @@ function TopicsView() { )}
handleKeyDown(e)}> - {provider === DiscussionProvider.OPEN_EDX && } - {provider === DiscussionProvider.LEGACY && } +
{ filteredTopicsCount === 0 diff --git a/src/discussions/topics/TopicsView.test.jsx b/src/discussions/topics/TopicsView.test.jsx index 4874a03a..f50070df 100644 --- a/src/discussions/topics/TopicsView.test.jsx +++ b/src/discussions/topics/TopicsView.test.jsx @@ -1,5 +1,5 @@ import { - fireEvent, render, screen, within, + fireEvent, render, screen, } from '@testing-library/react'; import MockAdapter from 'axios-mock-adapter'; import { IntlProvider } from 'react-intl'; @@ -10,11 +10,7 @@ import { initializeMockApp } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { AppProvider } from '@edx/frontend-platform/react'; -import { getBlocksAPIResponse } from '../../data/__factories__'; -import { getBlocksAPIURL } from '../../data/api'; -import { DiscussionProvider, getApiBaseUrl } from '../../data/constants'; -import { selectSequences } from '../../data/selectors'; -import { fetchCourseBlocks } from '../../data/thunks'; +import { getApiBaseUrl } from '../../data/constants'; import { initializeStore } from '../../store'; import { executeThunk } from '../../test-utils'; import { DiscussionContext } from '../common/context'; @@ -27,7 +23,6 @@ import './data/__factories__'; const courseId = 'course-v1:edX+DemoX+Demo_Course'; const topicsApiUrl = `${getApiBaseUrl()}/api/discussion/v1/course_topics/${courseId}`; -const topicsv2ApiUrl = `${getApiBaseUrl()}/api/discussion/v2/course_topics/${courseId}`; let store; let axiosMock; let lastLocation; @@ -57,131 +52,86 @@ function renderComponent() { ); } -describe('TopicsView', () => { - describe.each(['legacy', 'openedx'])('%s provider', (provider) => { - let inContextTopics; - let globalTopics; - let categories; - beforeEach(() => { - initializeMockApp({ - authenticatedUser: { - userId: 3, - username: 'abc123', - administrator: true, - roles: [], - }, - }); - - store = initializeStore({ - config: { provider }, - blocks: { - topics: {}, - }, - }); - Factory.resetAll(); - axiosMock = new MockAdapter(getAuthenticatedHttpClient()); - lastLocation = undefined; +describe('Legacy Topics View', () => { + let inContextTopics; + let globalTopics; + let categories; + beforeEach(() => { + initializeMockApp({ + authenticatedUser: { + userId: 3, + username: 'abc123', + administrator: true, + roles: [], + }, }); - async function setupMockResponse() { - if (provider === 'legacy') { - axiosMock - .onGet(topicsApiUrl) - .reply(200, { - courseware_topics: Factory.buildList('category', 2), - non_courseware_topics: Factory.buildList('topic', 3, {}, { topicPrefix: 'ncw' }), - }); - await executeThunk(fetchCourseTopics(courseId), store.dispatch, store.getState); - const state = store.getState(); - categories = state.topics.categoryIds; - globalTopics = selectNonCoursewareTopics(state); - inContextTopics = selectCoursewareTopics(state); - } else { - const blocksAPIResponse = getBlocksAPIResponse(true); - const ids = Object.values(blocksAPIResponse.blocks).filter(block => block.type === 'vertical') - .map(block => block.block_id); - const deletedIds = [ - 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@deleted-vertical-1', - 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@deleted-vertical-2', - ]; - const data = [ - ...Factory.buildList('topic.v2', 2, { usage_key: null }, { topicPrefix: 'ncw' }), - ...ids.map(id => Factory.build('topic.v2', { id })), - ...deletedIds.map(id => Factory.build('topic.v2', { id, enabled_in_context: false }, { topicPrefix: 'archived ' })), - ]; - - axiosMock - .onGet(topicsv2ApiUrl) - .reply(200, data); - axiosMock.onGet(getBlocksAPIURL()) - .reply(200, getBlocksAPIResponse(true)); - axiosMock.onAny().networkError(); - await executeThunk(fetchCourseBlocks(courseId, 'abc123'), store.dispatch, store.getState); - await executeThunk(fetchCourseTopics(courseId), store.dispatch, store.getState); - const state = store.getState(); - categories = selectSequences(state); - globalTopics = selectNonCoursewareTopics(state); - inContextTopics = selectCoursewareTopics(state); - } - } - - it('displays non-courseware topics', async () => { - await setupMockResponse(); - renderComponent(); - - globalTopics.forEach(topic => { - expect(screen.queryByText(topic.name)).toBeInTheDocument(); - }); + store = initializeStore({ + config: { provider: 'legacy' }, + blocks: { + topics: {}, + }, }); + Factory.resetAll(); + axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + lastLocation = undefined; + }); - it('displays non-courseware outside of a topic group', async () => { - await setupMockResponse(); - renderComponent(); - - categories.forEach(category => { - // For the new provider categories are blocks so use the display name - // otherwise use the category itself which is a string - expect(screen.queryByText(category.displayName || category)).toBeInTheDocument(); + async function setupMockResponse() { + axiosMock + .onGet(topicsApiUrl) + .reply(200, { + courseware_topics: Factory.buildList('category', 2), + non_courseware_topics: Factory.buildList('topic', 3, {}, { topicPrefix: 'ncw' }), }); + await executeThunk(fetchCourseTopics(courseId), store.dispatch, store.getState); + const state = store.getState(); + categories = state.topics.categoryIds; + globalTopics = selectNonCoursewareTopics(state); + inContextTopics = selectCoursewareTopics(state); + } - const topicGroups = screen.queryAllByTestId('topic-group'); - // For the new provider there should be a section for archived topics - expect(topicGroups).toHaveLength( - provider === DiscussionProvider.LEGACY - ? categories.length - : categories.length + 1, - ); - }); + it('displays non-courseware topics', async () => { + await setupMockResponse(); + renderComponent(); - if (provider === DiscussionProvider.OPEN_EDX) { - it('displays archived topics', async () => { - await setupMockResponse(); - renderComponent(); - const archivedTopicGroup = screen.queryAllByTestId('topic-group').pop(); - expect(archivedTopicGroup).toHaveTextContent(/archived/i); - const archivedTopicLinks = within(archivedTopicGroup).queryAllByRole('option'); - expect(archivedTopicLinks).toHaveLength(2); - }); - } - - it('displays courseware topics', async () => { - await setupMockResponse(); - renderComponent(); - - inContextTopics.forEach(topic => { - expect(screen.queryByText(topic.name)).toBeInTheDocument(); - }); - }); - - it('clicking on courseware topic (category) takes to category page', async () => { - await setupMockResponse(); - renderComponent(); - - const categoryName = categories[0].displayName || categories[0]; - const categoryPath = provider === 'legacy' ? categoryName : categories[0].id; - const topic = await screen.findByText(categoryName); - fireEvent.click(topic); - expect(lastLocation.pathname.endsWith(`/category/${categoryPath}`)).toBeTruthy(); + globalTopics.forEach(topic => { + expect(screen.queryByText(topic.name)).toBeInTheDocument(); }); }); + + it('displays non-courseware outside of a topic group', async () => { + await setupMockResponse(); + renderComponent(); + + categories.forEach(category => { + // For the new provider categories are blocks so use the display name + // otherwise use the category itself which is a string + expect(screen.queryByText(category.displayName || category)).toBeInTheDocument(); + }); + + const topicGroups = screen.queryAllByTestId('topic-group'); + // For the new provider there should be a section for archived topics + expect(topicGroups).toHaveLength(categories.length); + }); + + it('displays courseware topics', async () => { + await setupMockResponse(); + renderComponent(); + + inContextTopics.forEach(topic => { + expect(screen.queryByText(topic.name)).toBeInTheDocument(); + }); + }); + + it('clicking on courseware topic (category) takes to category page', async () => { + await setupMockResponse(); + renderComponent(); + + const categoryName = categories[0].displayName || categories[0]; + const categoryPath = categoryName; + const topic = await screen.findByText(categoryName); + fireEvent.click(topic); + expect(lastLocation.pathname.endsWith(`/category/${categoryPath}`)).toBeTruthy(); + }); }); diff --git a/src/discussions/topics/data/api.js b/src/discussions/topics/data/api.js index ec0fecbf..b297e2d9 100644 --- a/src/discussions/topics/data/api.js +++ b/src/discussions/topics/data/api.js @@ -13,14 +13,3 @@ export async function getCourseTopics(courseId, topicIds) { .get(url); return data; } - -export async function getCourseTopicsV2(courseId, topicIds) { - const url = `${getApiBaseUrl()}/api/discussion/v2/course_topics/${courseId}`; - const params = {}; - if (topicIds) { - params.topic_id = topicIds.join(','); - } - const { data } = await getAuthenticatedHttpClient() - .get(url); - return data; -} diff --git a/src/discussions/topics/data/selectors.js b/src/discussions/topics/data/selectors.js index 7b81ce44..d5c636e5 100644 --- a/src/discussions/topics/data/selectors.js +++ b/src/discussions/topics/data/selectors.js @@ -2,10 +2,6 @@ import { createSelector } from '@reduxjs/toolkit'; -import { DiscussionProvider } from '../../../data/constants'; -import { selectSequences } from '../../../data/selectors'; -import { selectDiscussionProvider } from '../../data/selectors'; - export const selectTopicFilter = state => state.topics.filter.trim() .toLowerCase(); @@ -19,29 +15,22 @@ export const selectTopicsInCategory = (categoryId) => state => ( export const selectTopics = state => state.topics.topics; export const selectCoursewareTopics = createSelector( - selectDiscussionProvider, selectCategories, selectTopicCategoryMap, selectTopics, - selectSequences, - (provider, categoryIds, topicsInCategory, topics, sequences) => ( - provider === DiscussionProvider.LEGACY - ? categoryIds.map(category => ({ - id: category, - name: category, - topics: topicsInCategory[category].map(id => topics[id]), - })) - : sequences.map(sequence => ({ - id: sequence.id, - name: sequence.displayName, - topics: sequence.topics.map(topicId => ({ id: topicId, name: topics[topicId]?.name || 'unnamed' })), - })) + (categoryIds, topicsInCategory, topics) => ( + categoryIds.map(category => ({ + id: category, + name: category, + topics: topicsInCategory[category].map(id => topics[id]), + })) ), ); export const selectNonCoursewareIds = state => state.topics.nonCoursewareIds; -export const selectNonCoursewareTopics = state => state.topics.nonCoursewareIds.map(id => state.topics.topics[id]); +export const selectNonCoursewareTopics = state => state.topics.nonCoursewareIds?.map(id => state.topics.topics[id]) + || []; export const selectTopic = topicId => state => state.topics.topics[topicId]; diff --git a/src/discussions/topics/data/slices.js b/src/discussions/topics/data/slices.js index 4456a0c0..1efe0008 100644 --- a/src/discussions/topics/data/slices.js +++ b/src/discussions/topics/data/slices.js @@ -11,8 +11,6 @@ const topicsSlice = createSlice({ categoryIds: [], // List of all non-courseware topics nonCoursewareIds: [], - // Topics that have been archived - archivedIds: [], // Mapping of all topics in each category topicsInCategory: {}, // Map of topics ids to topic data @@ -32,7 +30,6 @@ const topicsSlice = createSlice({ state.topics = payload.topics; state.nonCoursewareIds = payload.nonCoursewareIds; state.categoryIds = payload.categoryIds; - state.archivedIds = payload.archivedIds; state.topicsInCategory = payload.topicsInCategory; }, fetchCourseTopicsFailed: (state) => { diff --git a/src/discussions/topics/data/thunks.js b/src/discussions/topics/data/thunks.js index d52568c4..212da426 100644 --- a/src/discussions/topics/data/thunks.js +++ b/src/discussions/topics/data/thunks.js @@ -2,8 +2,7 @@ import { camelCaseObject } from '@edx/frontend-platform'; import { logError } from '@edx/frontend-platform/logging'; -import { DiscussionProvider } from '../../../data/constants'; -import { getCourseTopics, getCourseTopicsV2 } from './api'; +import { getCourseTopics } from './api'; import { fetchCourseTopicsFailed, fetchCourseTopicsRequest, fetchCourseTopicsSuccess } from './slices'; function normaliseTopics(data) { @@ -26,34 +25,12 @@ function normaliseTopics(data) { }; } -function normaliseTopicsV2(data) { - const nonCoursewareIds = []; - const topics = {}; - const archivedIds = []; - data.forEach(topic => { - if (!topic.enabledInContext) { - archivedIds.push(topic.id); - } else if (topic.usageKey === null) { - nonCoursewareIds.push(topic.id); - } - topics[topic.id] = topic; - }); - return { - topics, nonCoursewareIds, archivedIds, - }; -} - export function fetchCourseTopics(courseId) { - return async (dispatch, getState) => { + return async (dispatch) => { try { - const { config } = getState(); dispatch(fetchCourseTopicsRequest({ courseId })); - let data = {}; - if (config.provider === DiscussionProvider.LEGACY) { - data = normaliseTopics(camelCaseObject(await getCourseTopics(courseId))); - } else if (config.provider === DiscussionProvider.OPEN_EDX) { - data = normaliseTopicsV2(camelCaseObject(await getCourseTopicsV2(courseId))); - } + + const data = normaliseTopics(camelCaseObject(await getCourseTopics(courseId))); dispatch(fetchCourseTopicsSuccess(data)); } catch (error) { dispatch(fetchCourseTopicsFailed()); @@ -61,16 +38,3 @@ export function fetchCourseTopics(courseId) { } }; } - -export function fetchCourseTopicsV2(courseId) { - return async (dispatch) => { - try { - dispatch(fetchCourseTopicsRequest({ courseId })); - const data = await getCourseTopicsV2(courseId); - dispatch(fetchCourseTopicsSuccess(normaliseTopicsV2(camelCaseObject(data)))); - } catch (error) { - dispatch(fetchCourseTopicsFailed()); - logError(error); - } - }; -} diff --git a/src/discussions/topics/utils.js b/src/discussions/topics/utils.js index 6499cb6e..fcc3f961 100644 --- a/src/discussions/topics/utils.js +++ b/src/discussions/topics/utils.js @@ -10,7 +10,7 @@ export default function countFilteredTopics(topicsSelector, provider) { ? item.name.toLowerCase().includes(query) : true )); - count += nonCoursewareTopicsList.length; + count += nonCoursewareTopicsList?.length; // Counting legacy topics if (provider === DiscussionProvider.LEGACY) { const categories = topicsSelector?.categoryIds; diff --git a/src/discussions/utils.js b/src/discussions/utils.js index ff84c9d1..bb1c5587 100644 --- a/src/discussions/utils.js +++ b/src/discussions/utils.js @@ -281,3 +281,17 @@ export function inBlackoutDateRange(blackoutDateRanges) { (blackoutDateRange) => dateInDateRange(now, new Date(blackoutDateRange.start), new Date(blackoutDateRange.end)), ); } + +export function handleKeyDown(event) { + const { key } = event; + if (key !== 'ArrowDown' && key !== 'ArrowUp') { return; } + const option = event.target; + + let selectedOption; + if (key === 'ArrowDown') { selectedOption = option.nextElementSibling; } + if (key === 'ArrowUp') { selectedOption = option.previousElementSibling; } + + if (selectedOption) { + selectedOption.focus(); + } +} diff --git a/src/store.js b/src/store.js index 6be5e82b..3159f288 100644 --- a/src/store.js +++ b/src/store.js @@ -5,6 +5,7 @@ import { blocksReducer } from './data/slices'; import { cohortsReducer } from './discussions/cohorts/data'; import { commentsReducer } from './discussions/comments/data'; import { configReducer } from './discussions/data/slices'; +import { inContextTopicsReducer } from './discussions/in-context-topics/data'; import { learnersReducer } from './discussions/learners/data'; import { threadsReducer } from './discussions/posts/data'; import { topicsReducer } from './discussions/topics/data'; @@ -17,6 +18,7 @@ export function initializeStore(preloadedState = undefined) { comments: commentsReducer, cohorts: cohortsReducer, config: configReducer, + inContextTopics: inContextTopicsReducer, blocks: blocksReducer, learners: learnersReducer, courseTabs: courseTabsReducer,