diff --git a/src/data/selectors.js b/src/data/selectors.js index fdb36957..7681b850 100644 --- a/src/data/selectors.js +++ b/src/data/selectors.js @@ -1,3 +1,14 @@ /* eslint-disable import/prefer-default-export */ export const selectTopicContext = (topicId) => (state) => state.blocks.topics[topicId]; + +export const selectBlocks = (state) => state.blocks.blocks; +export const selectChapters = (state) => state.blocks.chapters; + +export const selectCurrentSelection = (state) => ({ + currentChapter: state.blocks.currentChapter, + currentSequential: state.blocks.currentSequential, + currentVertical: state.blocks.currentVertical, +}); + +export const selectTopicIds = () => (state) => state.blocks.chapters; diff --git a/src/data/slices.js b/src/data/slices.js index 08a2d8f3..305b3108 100644 --- a/src/data/slices.js +++ b/src/data/slices.js @@ -14,6 +14,9 @@ const blocksSlice = createSlice({ chapters: [], // Mapping of block keys to block data blocks: {}, + currentChapter: null, + currentSequential: null, + currentVertical: null, }, reducers: { fetchCourseBlocksRequest: (state) => { @@ -29,6 +32,18 @@ const blocksSlice = createSlice({ fetchCourseBlocksDenied: (state) => { state.status = RequestStatus.DENIED; }, + setCurrentChapter: (state, { payload }) => { + state.currentChapter = payload; + state.currentSequential = null; + state.currentVertical = null; + }, + setCurrentSequential: (state, { payload }) => { + state.currentSequential = payload; + state.currentVertical = null; + }, + setCurrentVertical: (state, { payload }) => { + state.currentVertical = payload; + }, }, }); @@ -37,6 +52,9 @@ export const { fetchCourseBlocksSuccess, fetchCourseBlocksFailed, fetchCourseBlocksDenied, + setCurrentVertical, + setCurrentChapter, + setCurrentSequential, } = blocksSlice.actions; export const blocksReducer = blocksSlice.reducer; diff --git a/src/data/thunks.js b/src/data/thunks.js index 86343231..577591b8 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -15,6 +15,16 @@ function normaliseCourseBlocks({ root, blocks, }) { + // This normalisation code is goes throught the block structure and converts it + // to a format that's easier for the app to use. + // It does a couple of things: + // 1. It creates record of all topic ids, and their position in the course + // structure, i.e. their section, sub-section, and unit. + // 2. It keeps a record of all the topic ids under each part of the course. So + // the app can easily query all the topic ids under any section, subsection + // so it can display posts from all of them if needed. + // 3. It creates a list of chapters/sections so there is a place to start + // navigating the course structure. const topics = {}; const chapters = []; const blockData = {}; @@ -22,13 +32,24 @@ function normaliseCourseBlocks({ const chapterData = camelCaseObject(blocks[chapterId]); chapters.push(chapterData); blockData[chapterId] = chapterData; + chapterData.topics = []; + blocks[chapterId].children?.forEach(sequentialId => { blockData[sequentialId] = camelCaseObject(blocks[sequentialId]); + blockData[sequentialId].topics = []; + blocks[sequentialId].children?.forEach(verticalId => { blockData[verticalId] = camelCaseObject(blocks[verticalId]); + blockData[verticalId].topics = []; + blocks[verticalId].children?.forEach(discussionId => { const discussion = camelCaseObject(blocks[discussionId]); blockData[discussionId] = discussion; + // Add this topic id to the list of topics for the current chapter, sequential, and vertical + chapterData.topics.push(discussion.studentViewData.topicId); + blockData[sequentialId].topics.push(discussion.studentViewData.topicId); + blockData[verticalId].topics.push(discussion.studentViewData.topicId); + // Store the topic's context in the course in a map topics[discussion.studentViewData.topicId] = { chapterName: blockData[chapterId].displayName, verticalName: blockData[sequentialId].displayName, diff --git a/src/discussions/comments/CommentsView.jsx b/src/discussions/comments/CommentsView.jsx index 5d7bcc6d..6037060b 100644 --- a/src/discussions/comments/CommentsView.jsx +++ b/src/discussions/comments/CommentsView.jsx @@ -108,7 +108,7 @@ DiscussionCommentsView.propTypes = { postType: PropTypes.string.isRequired, intl: intlShape.isRequired, endorsed: PropTypes.oneOf([ - EndorsementStatus.ENDORSED, EndorsementStatus.ENDORSED, EndorsementStatus.DISCUSSION, + EndorsementStatus.ENDORSED, EndorsementStatus.UNENDORSED, EndorsementStatus.DISCUSSION, ]).isRequired, }; diff --git a/src/discussions/discussions-home/DiscussionsHome.jsx b/src/discussions/discussions-home/DiscussionsHome.jsx index ddfc8789..02e3da10 100644 --- a/src/discussions/discussions-home/DiscussionsHome.jsx +++ b/src/discussions/discussions-home/DiscussionsHome.jsx @@ -13,7 +13,7 @@ import { fetchCourseBlocks } from '../../data/thunks'; import { CommentsView } from '../comments'; import { DiscussionContext } from '../common/context'; import { fetchCourseConfig } from '../data/thunks'; -import { BreadcrumbMenu, NavigationBar } from '../navigation'; +import { LegacyBreadcrumbMenu, NavigationBar } from '../navigation'; import { PostEditor, PostsView } from '../posts'; import { clearRedirect } from '../posts/data'; import { TopicsView } from '../topics'; @@ -69,7 +69,7 @@ export default function DiscussionsHome() { Routes.POSTS.PATH, Routes.TOPICS.CATEGORY, ]} - component={BreadcrumbMenu} + component={LegacyBreadcrumbMenu} />
diff --git a/src/discussions/navigation/breadcrumb-menu/BreadcrumbMenu.jsx b/src/discussions/navigation/breadcrumb-menu/BreadcrumbMenu.jsx index a4ace026..2a2a6b0a 100644 --- a/src/discussions/navigation/breadcrumb-menu/BreadcrumbMenu.jsx +++ b/src/discussions/navigation/breadcrumb-menu/BreadcrumbMenu.jsx @@ -1,66 +1,91 @@ import React from 'react'; -import { useSelector } from 'react-redux'; -import { generatePath, useHistory, useRouteMatch } from 'react-router'; +import { useDispatch, useSelector } from 'react-redux'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; -import { Breadcrumb } from '@edx/paragon'; +import { Dropdown, DropdownButton } from '@edx/paragon'; -import { Routes } from '../../../data/constants'; -import { selectTopic } from '../../topics/data/selectors'; +import { selectBlocks, selectChapters, selectCurrentSelection } from '../../../data/selectors'; +import { setCurrentChapter, setCurrentSequential, setCurrentVertical } from '../../../data/slices'; import messages from './messages'; function BreadcrumbMenu({ intl }) { + const dispatch = useDispatch(); + const blocks = useSelector(selectBlocks); + const chapters = useSelector(selectChapters); const { - params: { - courseId, - category, - topicId, - }, - } = useRouteMatch([Routes.TOPICS.CATEGORY, Routes.TOPICS.TOPIC]); - const topic = useSelector(selectTopic(topicId)); - const history = useHistory(); + currentChapter, + currentVertical, + currentSequential, + } = useSelector(selectCurrentSelection); - const crumbs = [ - { - url: () => generatePath(Routes.TOPICS.ALL, { courseId }), - label: intl.formatMessage(messages.allTopics), - }, - { - url: () => generatePath(Routes.TOPICS.CATEGORY, { - courseId, - category: category || topic?.categoryId, - }), - label: category || topic?.categoryId, - }, - { - url: () => generatePath(Routes.TOPICS.TOPIC, { - courseId, - topicId, - }), - label: topic?.name, - }, - ].filter(crumb => Boolean(crumb.label)) - .map(({ - url, - label, - }) => ({ - url: url(), - label, - })); - - const activeLabel = crumbs.pop().label; + const showAllMsg = intl.formatMessage(messages.showAll); return (
- { - e.preventDefault(); - history.push(e.target.pathname); - }} - /> + dispatch(setCurrentChapter(cId))} + > + + {showAllMsg} + + {chapters.map(chapter => ( + chapter.topics.length > 0 + && ( + + {chapter.displayName} + + ) + ))} + + {currentChapter + && ( + <> +
/
+ dispatch(setCurrentSequential(sId))} + > + + {showAllMsg} + + {blocks[currentChapter].children.map(seqId => ( + blocks[seqId].topics.length > 0 + && ( + + {blocks[seqId].displayName} + + ) + ))} + + + )} + {currentSequential + && ( + <> +
/
+ dispatch(setCurrentVertical(vId))} + > + + {showAllMsg} + + {blocks[currentSequential]?.children?.map(vertId => ( + blocks[vertId].topics.length > 0 + && ( + + {blocks[vertId].displayName} + + ) + ))} + + + )}
); } diff --git a/src/discussions/navigation/breadcrumb-menu/LegacyBreadcrumbMenu.jsx b/src/discussions/navigation/breadcrumb-menu/LegacyBreadcrumbMenu.jsx new file mode 100644 index 00000000..7399b846 --- /dev/null +++ b/src/discussions/navigation/breadcrumb-menu/LegacyBreadcrumbMenu.jsx @@ -0,0 +1,120 @@ +import React from 'react'; + +import { useSelector } from 'react-redux'; +import { generatePath, useHistory, useRouteMatch } from 'react-router'; + +import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; +import { Dropdown, DropdownButton } from '@edx/paragon'; + +import { Routes } from '../../../data/constants'; +import { + selectCategories, + selectNonCoursewareTopics, + selectTopic, + selectTopicsInCategory, +} from '../../topics/data/selectors'; +import messages from './messages'; + +function LegacyBreadcrumbMenu({ intl }) { + const history = useHistory(); + const { + params: { + courseId, + category, + topicId: currentTopicId, + }, + } = useRouteMatch([Routes.TOPICS.CATEGORY, Routes.TOPICS.TOPIC]); + + const currentTopic = useSelector(selectTopic(currentTopicId)); + const currentCategory = category || currentTopic?.categoryId; + const topicsInCategory = useSelector(selectTopicsInCategory(currentCategory)); + const nonCoursewareTopics = useSelector(selectNonCoursewareTopics); + const categories = useSelector(selectCategories); + const showAllMsg = intl.formatMessage(messages.showAll); + const isNonCoursewareTopic = currentTopic && !currentCategory; + + const navigateToCategory = (categoryId) => { + if (!categoryId) { + history.push(generatePath(Routes.TOPICS.ALL, { + courseId, + category: categoryId, + })); + } else { + history.push(generatePath(Routes.TOPICS.CATEGORY, { + courseId, + category: categoryId, + })); + } + }; + const navigateToTopic = (topicId) => { + if (!topicId) { + navigateToCategory(currentCategory); + } else { + history.push(generatePath(Routes.TOPICS.TOPIC, { + courseId, + topicId, + })); + } + }; + + return ( +
+ {isNonCoursewareTopic ? ( + + + {showAllMsg} + + {nonCoursewareTopics.map(topic => ( + + {topic.name} + + ))} + + ) : ( + + + {showAllMsg} + + {categories.map(categoryId => ( + + {categoryId} + + ))} + + )} + {currentCategory && ( + <> +
/
+ + + {showAllMsg} + + {topicsInCategory?.map(topic => ( + + {topic.name} + + ))} + + + )} +
+ ); +} + +LegacyBreadcrumbMenu.propTypes = { + intl: intlShape.isRequired, +}; + +export default injectIntl(LegacyBreadcrumbMenu); diff --git a/src/discussions/navigation/breadcrumb-menu/LegacyBreadcrumbMenu.test.jsx b/src/discussions/navigation/breadcrumb-menu/LegacyBreadcrumbMenu.test.jsx new file mode 100644 index 00000000..51b100e3 --- /dev/null +++ b/src/discussions/navigation/breadcrumb-menu/LegacyBreadcrumbMenu.test.jsx @@ -0,0 +1,112 @@ +import React from 'react'; + +import { + act, fireEvent, render, screen, +} from '@testing-library/react'; +import MockAdapter from 'axios-mock-adapter'; +import { IntlProvider } from 'react-intl'; +import { MemoryRouter, Route } from 'react-router'; +import { Factory } from 'rosie'; + +import { initializeMockApp } from '@edx/frontend-platform'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import { AppProvider } from '@edx/frontend-platform/react'; + +import { API_BASE_URL, Routes } from '../../../data/constants'; +import { initializeStore } from '../../../store'; +import { executeThunk } from '../../../test-utils'; +import { fetchCourseTopics } from '../../topics/data/thunks'; +import { LegacyBreadcrumbMenu } from '../index'; + +import '../../topics/data/__factories__'; + +const courseId = 'course-v1:edX+TestX+Test_Course'; +const topicsApiUrl = `${API_BASE_URL}/api/discussion/v1/course_topics/${courseId}`; +let store; +let axiosMock; + +function renderComponent(path) { + render( + + + + + + + , + ); +} + +describe('LegacyBreadcrumbMenu', () => { + beforeEach(async () => { + initializeMockApp({ + authenticatedUser: { + userId: 3, + username: 'abc123', + administrator: true, + roles: [], + }, + }); + + store = initializeStore({ + blocks: { + topics: {}, + }, + }); + Factory.resetAll(); + axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + const data = { + courseware_topics: Factory.buildList('category', 3), + non_courseware_topics: Factory.buildList('topic', 3), + }; + axiosMock + .onGet(topicsApiUrl) + .reply(200, data); + await executeThunk(fetchCourseTopics(courseId), store.dispatch, store.getState); + }); + + it('shows the category dropdown with a category selected', async () => { + renderComponent(`/discussions/${courseId}/category/category-1`); + + // The current category should be visible on the page + const categoryDropdown = await screen.findByText('category-1'); + // Since a category is selected a subcategory dropdown should also be visible with "show all" selected by default + const topicsDropdown = screen.queryByText('Show all'); + // A show all button should show up that lists topics in the current category + expect(topicsDropdown).toBeInTheDocument(); + // Other categories should not be visible. + expect(screen.queryByText('category-2')).not.toBeInTheDocument(); + + // Click on the category dropdown. + act(() => { + fireEvent.click(categoryDropdown); + }); + // Now other categories should be visible in the dropdown. + expect(screen.queryByText('category-2')).toBeInTheDocument(); + // There are three categories but this has a length of 4 since the selected category name appears twice. + expect(screen.queryAllByText('category-', { exact: false })).toHaveLength(4); + + // Now click on the topics dropdown + act(() => { + fireEvent.click(topicsDropdown); + }); + // Topics in the category should be visible. + expect(screen.queryAllByText('category-1-topic', { exact: false })).toHaveLength(4); + }); + + it('shows the category correct dropdown labels with a topic selected', async () => { + renderComponent(`/discussions/${courseId}/topics/category-2-topic-1`); + + // Since a topic is selected, we have both a category and topic, so "show all shouldn't be visible" + expect(screen.queryByText('Show all')).not.toBeInTheDocument(); + // The name of the category and topic should be visible. + expect(screen.queryByText('category-2')).toBeInTheDocument(); + expect(screen.queryByText('category-2-topic 1')).toBeInTheDocument(); + }); +}); diff --git a/src/discussions/navigation/breadcrumb-menu/messages.js b/src/discussions/navigation/breadcrumb-menu/messages.js index ecfc254f..71da8eed 100644 --- a/src/discussions/navigation/breadcrumb-menu/messages.js +++ b/src/discussions/navigation/breadcrumb-menu/messages.js @@ -6,6 +6,11 @@ const messages = defineMessages({ defaultMessage: 'Topics', description: 'Topics from Breadcrumb Menu item', }, + showAll: { + id: 'discussions.navigation.breadcrumbMenu.showAll', + defaultMessage: 'Show all', + description: 'Option to show all items in a section of the breadcrumb', + }, }); export default messages; diff --git a/src/discussions/navigation/index.js b/src/discussions/navigation/index.js index 1532211d..9ea64da0 100644 --- a/src/discussions/navigation/index.js +++ b/src/discussions/navigation/index.js @@ -1,3 +1,4 @@ /* eslint-disable import/prefer-default-export */ export { default as BreadcrumbMenu } from './breadcrumb-menu/BreadcrumbMenu'; +export { default as LegacyBreadcrumbMenu } from './breadcrumb-menu/LegacyBreadcrumbMenu'; export { default as NavigationBar } from './navigation-bar/NavigationBar'; diff --git a/src/discussions/posts/PostsView.jsx b/src/discussions/posts/PostsView.jsx index 549784f5..b682f4c6 100644 --- a/src/discussions/posts/PostsView.jsx +++ b/src/discussions/posts/PostsView.jsx @@ -74,10 +74,10 @@ function PostsView({ showOwnPosts }) { lastPinnedIdx = false; // Add a spacing after the group of pinned posts return ( - <> +
- + ); } return (); diff --git a/src/discussions/topics/data/__factories__/topics.factory.js b/src/discussions/topics/data/__factories__/topics.factory.js index 68a8eaaa..cfcfe40b 100644 --- a/src/discussions/topics/data/__factories__/topics.factory.js +++ b/src/discussions/topics/data/__factories__/topics.factory.js @@ -1,6 +1,23 @@ import { Factory } from 'rosie'; Factory.define('topic') - .sequence('id', (idx) => `topic-${idx}`) - .sequence('name', (idx) => `topic ${idx}`) + .option('topicPrefix', null, '') + .sequence('id', ['topicPrefix'], (idx, topicPrefix) => `${topicPrefix}topic-${idx}`) + .sequence('name', ['topicPrefix'], (idx, topicPrefix) => `${topicPrefix}topic ${idx}`) + .attr('thread_counts', [], { + discussion: 0, + question: 0, + }) .attr('children', []); + +Factory.define('category') + .attr('id', [], null) + .sequence('name', (idx) => `category-${idx}`) + .attr('thread_counts', [], { + discussion: 0, + question: 0, + }) + .attr('children', ['name'], (name) => { + Factory.reset('topic'); + return Factory.buildList('topic', 4, null, { topicPrefix: `${name}-` }); + }); diff --git a/src/discussions/topics/data/selectors.js b/src/discussions/topics/data/selectors.js index 09047238..99c30ebd 100644 --- a/src/discussions/topics/data/selectors.js +++ b/src/discussions/topics/data/selectors.js @@ -5,7 +5,7 @@ export const selectTopicFilter = state => state.topics.filter.trim().toLowerCase export const selectCategories = state => state.topics.categoryIds; export const selectTopicsInCategory = (categoryId) => state => ( - state.topics.topicsInCategory[categoryId].map(id => state.topics.topics[id]) + state.topics.topicsInCategory[categoryId]?.map(id => state.topics.topics[id]) || [] ); export const selectTopics = state => state.topics.topics;