diff --git a/package-lock.json b/package-lock.json index 5af9a9d3..41cef0ff 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5588,6 +5588,26 @@ } } }, + "@testing-library/user-event": { + "version": "13.5.0", + "resolved": "https://registry.npmjs.org/@testing-library/user-event/-/user-event-13.5.0.tgz", + "integrity": "sha512-5Kwtbo3Y/NowpkbRuSepbyMFkZmHgD+vPzYB/RJ4oxt5Gj/avFFBYjhw27cqSVPVw/3a67NK1PbiIr9k4Gwmdg==", + "dev": true, + "requires": { + "@babel/runtime": "^7.12.5" + }, + "dependencies": { + "@babel/runtime": { + "version": "7.16.3", + "resolved": "https://registry.npmjs.org/@babel/runtime/-/runtime-7.16.3.tgz", + "integrity": "sha512-WBwekcqacdY2e9AF/Q7WLFUWmdJGJTkbjqTjoMDgXkVZ3ZRUvOPsLb5KdwISoQVsbP+DQzVZW4Zhci0DvpbNTQ==", + "dev": true, + "requires": { + "regenerator-runtime": "^0.13.4" + } + } + } + }, "@tinymce/tinymce-react": { "version": "3.13.0", "resolved": "https://registry.npmjs.org/@tinymce/tinymce-react/-/tinymce-react-3.13.0.tgz", diff --git a/package.json b/package.json index d42580aa..0df0f0bb 100644 --- a/package.json +++ b/package.json @@ -64,6 +64,7 @@ "@edx/frontend-build": "8.1.6", "@testing-library/jest-dom": "5.15.0", "@testing-library/react": "12.1.2", + "@testing-library/user-event": "13.5.0", "axios-mock-adapter": "1.20.0", "codecov": "3.8.3", "es-check": "6.1.1", diff --git a/src/components/TinyMCEEditor.jsx b/src/components/TinyMCEEditor.jsx index 8a521deb..1b5ab655 100644 --- a/src/components/TinyMCEEditor.jsx +++ b/src/components/TinyMCEEditor.jsx @@ -63,6 +63,14 @@ export default function TinyMCEEditor(props) { } }; + let contentStyle; + // In the test environment this causes an error so set styles to empty since they aren't needed for testing. + try { + contentStyle = [contentCss, contentUiCss, edxBrandCss].join('\n'); + } catch (err) { + contentStyle = ''; + } + return ( (userIsPrivileged ? ['Student', 'Moderator'] : ['Student'])); diff --git a/src/discussions/data/__factories__/index.js b/src/discussions/data/__factories__/index.js new file mode 100644 index 00000000..c7ed34c2 --- /dev/null +++ b/src/discussions/data/__factories__/index.js @@ -0,0 +1,2 @@ +import './config.factory'; +import './settings.factory'; diff --git a/src/discussions/data/__factories__/settings.factory.js b/src/discussions/data/__factories__/settings.factory.js new file mode 100644 index 00000000..b6f9d848 --- /dev/null +++ b/src/discussions/data/__factories__/settings.factory.js @@ -0,0 +1,7 @@ +import { Factory } from 'rosie'; + +Factory.define('settings') + .attr('division_scheme', null, 'none') + .attr('always_divide_inline_discussions', null, false) + .attr('divided_inline_discussions', null, []) + .attr('divided_course_wide_discussions', null, []); diff --git a/src/discussions/data/api.js b/src/discussions/data/api.js index eb966727..6dcd3460 100644 --- a/src/discussions/data/api.js +++ b/src/discussions/data/api.js @@ -20,3 +20,13 @@ export async function getDiscussionsConfig(courseId) { const { data } = await getAuthenticatedHttpClient().get(url); return data; } + +/** + * Get discussions course config + * @param {string} courseId + */ +export async function getDiscussionsSettings(courseId) { + const url = `${courseConfigApiUrl}${courseId}/settings`; + const { data } = await getAuthenticatedHttpClient().get(url); + return data; +} diff --git a/src/discussions/data/selectors.js b/src/discussions/data/selectors.js index 6284bb23..f6ac95ec 100644 --- a/src/discussions/data/selectors.js +++ b/src/discussions/data/selectors.js @@ -3,5 +3,8 @@ export const selectAnonymousPostingConfig = state => ({ allowAnonymous: state.config.allowAnonymous, allowAnonymousToPeers: state.config.allowAnonymousToPeers, -} -); +}); + +export const selectUserIsPrivileged = state => state.config.userIsPrivileged; + +export const selectDivisionSettings = state => state.config.settings; diff --git a/src/discussions/data/slices.js b/src/discussions/data/slices.js index e573603e..95fc0bb5 100644 --- a/src/discussions/data/slices.js +++ b/src/discussions/data/slices.js @@ -10,6 +10,14 @@ const configSlice = createSlice({ blackouts: [], allowAnonymous: false, allowAnonymousToPeers: false, + userRoles: [], + userIsPrivileged: false, + settings: { + divisionScheme: 'none', + alwaysDivideInlineDiscussions: false, + dividedInlineDiscussions: [], + dividedCourseWideDiscussions: [], + }, }, reducers: { fetchConfigRequest: (state) => { diff --git a/src/discussions/data/thunks.js b/src/discussions/data/thunks.js index bec4a0d9..3b102c73 100644 --- a/src/discussions/data/thunks.js +++ b/src/discussions/data/thunks.js @@ -3,7 +3,7 @@ import { camelCaseObject } from '@edx/frontend-platform'; import { logError } from '@edx/frontend-platform/logging'; import { getHttpErrorStatus } from '../utils'; -import { getDiscussionsConfig } from './api'; +import { getDiscussionsConfig, getDiscussionsSettings } from './api'; import { fetchConfigDenied, fetchConfigFailed, fetchConfigRequest, fetchConfigSuccess, } from './slices'; @@ -17,8 +17,12 @@ export function fetchCourseConfig(courseId) { return async (dispatch) => { try { dispatch(fetchConfigRequest()); - const data = await getDiscussionsConfig(courseId); - dispatch(fetchConfigSuccess(camelCaseObject(data))); + const config = await getDiscussionsConfig(courseId); + if (config.user_is_privileged) { + const settings = await getDiscussionsSettings(courseId); + Object.assign(config, { settings }); + } + dispatch(fetchConfigSuccess(camelCaseObject(config))); } catch (error) { if (getHttpErrorStatus(error) === 403) { dispatch(fetchConfigDenied()); diff --git a/src/discussions/posts/data/api.js b/src/discussions/posts/data/api.js index 4f600332..6df5a3d5 100644 --- a/src/discussions/posts/data/api.js +++ b/src/discussions/posts/data/api.js @@ -77,6 +77,7 @@ export async function getThread(threadId) { * @param {ThreadType} type The thread's type (either "question" or "discussion") * @param {string} title * @param {string} content + * @param {number} cohort * @param {boolean} following Follow the thread after creating * @param {boolean} anonymous Should the thread be anonymous to all users * @param {boolean} anonymousToPeers Should the thread be anonymous to peers diff --git a/src/discussions/posts/post-editor/PostEditor.jsx b/src/discussions/posts/post-editor/PostEditor.jsx index 52258af9..2e033d32 100644 --- a/src/discussions/posts/post-editor/PostEditor.jsx +++ b/src/discussions/posts/post-editor/PostEditor.jsx @@ -1,4 +1,4 @@ -import React, { useContext, useEffect } from 'react'; +import React, { useEffect } from 'react'; import PropTypes from 'prop-types'; import { Formik } from 'formik'; @@ -7,17 +7,18 @@ import { useHistory, useParams } from 'react-router'; import * as Yup from 'yup'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; -import { AppContext } from '@edx/frontend-platform/react'; -import { Card, Form, StatefulButton } from '@edx/paragon'; +import { + Button, Card, Form, Spinner, StatefulButton, +} from '@edx/paragon'; import { Help, Post } from '@edx/paragon/icons'; import { TinyMCEEditor } from '../../../components'; import FormikErrorFeedback from '../../../components/FormikErrorFeedback'; +import { useDispatchWithState } from '../../../data/hooks'; import { selectCourseCohorts } from '../../cohorts/data/selectors'; import { fetchCourseCohorts } from '../../cohorts/data/thunks'; -import { selectAnonymousPostingConfig } from '../../data/selectors'; -import { selectCoursewareTopics, selectNonCoursewareTopics } from '../../topics/data/selectors'; -import { fetchCourseTopics } from '../../topics/data/thunks'; +import { selectAnonymousPostingConfig, selectDivisionSettings, selectUserIsPrivileged } from '../../data/selectors'; +import { selectCoursewareTopics, selectNonCoursewareIds, selectNonCoursewareTopics } from '../../topics/data/selectors'; import { discussionsPath, formikCompatibleHandler, isFormikFieldInvalid, useCommentsPagePath, } from '../../utils'; @@ -62,8 +63,8 @@ function PostEditor({ intl, editExisting, }) { - const { authenticatedUser } = useContext(AppContext); const dispatch = useDispatch(); + const [submitting, dispatchSubmit] = useDispatchWithState(); const history = useHistory(); const commentsPagePath = useCommentsPagePath(); const { @@ -73,33 +74,26 @@ function PostEditor({ } = useParams(); const coursewareTopics = useSelector(selectCoursewareTopics); const nonCoursewareTopics = useSelector(selectNonCoursewareTopics); + const nonCoursewareIds = useSelector(selectNonCoursewareIds); const { allowAnonymous, allowAnonymousToPeers, } = useSelector(selectAnonymousPostingConfig); const cohorts = useSelector(selectCourseCohorts); const post = useSelector(selectThread(postId)); - let initialValues = { - postType: 'discussion', - topic: topicId || nonCoursewareTopics?.[0]?.id, - title: '', - comment: '', - follow: true, - anonymous: false, - anonymousToPeers: false, + const userIsPrivileged = useSelector(selectUserIsPrivileged); + const settings = useSelector(selectDivisionSettings); + const canSelectCohort = (tId) => { + // If the user isn't privileged, they can't edit the cohort. + // If the topic is being edited the cohort can't be changed. + if (!userIsPrivileged || editExisting) { + return false; + } + if (nonCoursewareIds.includes(tId)) { + return settings.dividedCourseWideDiscussions.includes(tId); + } + return settings.alwaysDivideInlineDiscussions || settings.dividedInlineDiscussions.includes(tId); }; - if (editExisting) { - initialValues = { - postType: post.type || 'discussion', - topic: post.topicId || topicId || nonCoursewareTopics?.[0]?.id, - title: post.title || '', - comment: post.rawBody || '', - follow: (post.following === null || post.following === undefined) ? true : post.following, - anonymous: allowAnonymous ? false : undefined, - anonymousToPeers: allowAnonymousToPeers ? false : undefined, - }; - } - const canSelectCohort = authenticatedUser.administrator && !editExisting; const hideEditor = () => { if (editExisting) { history.push(discussionsPath(commentsPagePath, { @@ -113,27 +107,27 @@ function PostEditor({ const submitForm = async (values) => { if (editExisting) { - dispatch(updateExistingThread(postId, { + await dispatchSubmit(updateExistingThread(postId, { topicId: values.topic, type: values.postType, title: values.title, content: values.comment, })); } else { - const cohort = canSelectCohort + const cohort = canSelectCohort(values.topic) // null stands for no cohort restriction ("All learners" option) ? (values.cohort || null) // if not allowed to set cohort, always undefined, so no value is sent to backend : undefined; - dispatch(createNewThread({ + await dispatchSubmit(createNewThread({ courseId, topicId: values.topic, type: values.postType, title: values.title, content: values.comment, following: values.following, - anonymous: values.anonymous, - anonymousToPeers: values.anonymousToPeers, + anonymous: allowAnonymous ? values.anonymous : undefined, + anonymousToPeers: allowAnonymousToPeers ? values.anonymousToPeers : undefined, cohort, })); } @@ -141,8 +135,7 @@ function PostEditor({ }; useEffect(() => { - dispatch(fetchCourseTopics(courseId)); - if (canSelectCohort) { + if (userIsPrivileged) { dispatch(fetchCourseCohorts(courseId)); } if (editExisting) { @@ -150,6 +143,34 @@ function PostEditor({ } }, [courseId, editExisting]); + if (editExisting && !post) { + return ( +
+ +
+ ); + } + let initialValues = { + postType: 'discussion', + topic: topicId || nonCoursewareTopics?.[0]?.id, + title: '', + comment: '', + follow: true, + anonymous: false, + anonymousToPeers: false, + }; + if (editExisting) { + initialValues = { + postType: post.type, + topic: post.topicId, + title: post.title, + comment: post.rawBody, + follow: (post.following === null || post.following === undefined) ? true : post.following, + anonymous: allowAnonymous ? false : undefined, + anonymousToPeers: allowAnonymousToPeers ? false : undefined, + }; + } + return ( { ({ @@ -242,7 +262,7 @@ function PostEditor({ ))} - {canSelectCohort + {canSelectCohort(values.topic) && ( @@ -334,17 +353,17 @@ function PostEditor({ )}
- + >{intl.formatMessage(messages.cancel)} + + + + + + + + + , + ); +} + +describe('PostEditor', () => { + beforeEach(async () => { + initializeMockApp({ + authenticatedUser: { + userId: 3, + username: 'abc123', + administrator: true, + roles: [], + }, + }); + + Factory.resetAll(); + axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + const cwtopics = Factory.buildList('category', 2); + Factory.reset('topic'); + axiosMock + .onGet(topicsApiUrl) + .reply(200, { + courseware_topics: cwtopics, + non_courseware_topics: Factory.buildList('topic', 3, {}, { topicPrefix: 'ncw-' }), + }); + }); + describe.each([ + { + allowAnonymous: false, + allowAnonymousToPeers: false, + }, + { + allowAnonymous: false, + allowAnonymousToPeers: true, + }, + { + allowAnonymous: true, + allowAnonymousToPeers: false, + }, + { + allowAnonymous: true, + allowAnonymousToPeers: true, + }, + ])('Non-Cohorted', ({ + allowAnonymous, + allowAnonymousToPeers, + }) => { + beforeEach(async () => { + store = initializeStore({ + config: { + allowAnonymous, + allowAnonymousToPeers, + }, + }); + await executeThunk(fetchCourseTopics(courseId), store.dispatch, store.getState); + }); + test( + `new post when anonymous posts are ${allowAnonymous ? '' : 'not '}allowed and anonymous posts to peers are ${allowAnonymousToPeers ? '' : 'not '}allowed`, + async () => { + await renderComponent(); + + expect(screen.queryByRole('heading')) + .toHaveTextContent('Add a post'); + expect(screen.queryAllByRole('radio')) + .toHaveLength(2); + // 2 categories with 4 subcategories each + expect(screen.queryAllByText(/category-\d-topic \d/)) + .toHaveLength(8); + // 3 non courseare topics + expect(screen.queryAllByText(/ncw-topic \d/)) + .toHaveLength(3); + + expect(screen.queryByText('cohort', { exact: false })) + .not + .toBeInTheDocument(); + if (allowAnonymous) { + expect(screen.queryByText('Post anonymously')) + .toBeInTheDocument(); + } else { + expect(screen.queryByText('Post anonymously')) + .not + .toBeInTheDocument(); + } + if (allowAnonymousToPeers) { + expect(screen.queryByText('Post anonymously to peers')) + .toBeInTheDocument(); + } else { + expect(screen.queryByText('Post anonymously to peers')) + .not + .toBeInTheDocument(); + } + }, + ); + }); + + describe('Cohorted', () => { + const dividedncw = ['ncw-topic-2']; + const dividedcw = ['category-1-topic-2', 'category-2-topic-1', 'category-2-topic-2']; + + async function setupData(config = {}, settings = {}) { + store = initializeStore({ + config: { + userRoles: ['Student', 'Moderator'], + userIsPrivileged: true, + settings: { + dividedInlineDiscussions: dividedcw, + dividedCourseWideDiscussions: dividedncw, + ...settings, + }, + ...config, + }, + }); + await executeThunk(fetchCourseTopics(courseId), store.dispatch, store.getState); + } + + test('test privileged user', async () => { + await setupData(); + renderComponent(); + // Initially the user can't select a cohort + expect(screen.queryByRole('combobox', { + name: /cohort visibility/i, + })) + .not + .toBeInTheDocument(); + // If a cohorted topic is selected, the cohort visibility selector is displayed + ['ncw-topic 2', 'category-1-topic 2', 'category-2-topic 1', 'category-2-topic 2'].forEach((topicName) => { + userEvent.selectOptions( + screen.getByRole('combobox', { + name: /topic area/i, + }), + screen.getByRole('option', { name: topicName }), + ); + + expect(screen.queryByRole('combobox', { + name: /cohort visibility/i, + })) + .toBeInTheDocument(); + }); + // Now if a non-cohorted topic is selected, the cohort visibility selector is hidden + ['ncw-topic 1', 'category-1-topic 1', 'category-2-topic 4'].forEach((topicName) => { + userEvent.selectOptions( + screen.getByRole('combobox', { + name: /topic area/i, + }), + screen.getByRole('option', { name: topicName }), + ); + expect(screen.queryByRole('combobox', { + name: /cohort visibility/i, + })) + .not + .toBeInTheDocument(); + }); + }); + test('test always divided inline', async () => { + await setupData({}, { alwaysDivideInlineDiscussions: true }); + renderComponent(); + // Initially the user can't select a cohort + expect(screen.queryByRole('combobox', { + name: /cohort visibility/i, + })) + .not + .toBeInTheDocument(); + // All coursweare topics are divided + [1, 2].forEach(catId => { + [1, 2, 3, 4].forEach((topicId) => { + userEvent.selectOptions( + screen.getByRole('combobox', { + name: /topic area/i, + }), + screen.getByRole('option', { name: `category-${catId}-topic ${topicId}` }), + ); + + expect(screen.queryByRole('combobox', { + name: /cohort visibility/i, + })) + .toBeInTheDocument(); + }); + }); + + // Non-courseware topics can still have cohort visibility hidden + ['ncw-topic 1', 'ncw-topic 3'].forEach((topicName) => { + userEvent.selectOptions( + screen.getByRole('combobox', { + name: /topic area/i, + }), + screen.getByRole('option', { name: topicName }), + ); + expect(screen.queryByRole('combobox', { + name: /cohort visibility/i, + })) + .not + .toBeInTheDocument(); + }); + }); + test('test unprivileged user', async () => { + await setupData({ userIsPrivileged: false }); + renderComponent(); + ['ncw-topic 1', 'ncw-topic 2', 'category-1-topic 1', 'category-2-topic 1'].forEach((topicName) => { + userEvent.selectOptions( + screen.getByRole('combobox', { + name: /topic area/i, + }), + screen.getByRole('option', { name: topicName }), + ); + // If a cohorted topic is selected, the cohort visibility selector is displayed + expect(screen.queryByRole('combobox', { + name: /cohort visibility/i, + })) + .not + .toBeInTheDocument(); + }); + }); + test('edit existing post should not show cohort selector', async () => { + const threadId = 'thread-1'; + await setupData(); + axiosMock.onGet(`${threadsApiUrl}${threadId}/`) + .reply(200, Factory.build('thread')); + await executeThunk(fetchThread(threadId), store.dispatch, store.getState); + await renderComponent(true, `/discussions/${courseId}/posts/${threadId}/edit`); + + ['ncw-topic 1', 'ncw-topic 2', 'category-1-topic 1', 'category-2-topic 1'].forEach((topicName) => { + userEvent.selectOptions( + screen.getByRole('combobox', { + name: /topic area/i, + }), + screen.getByRole('option', { name: topicName }), + ); + // If a cohorted topic is selected, the cohort visibility selector is displayed + expect(screen.queryByRole('combobox', { + name: /cohort visibility/i, + })) + .not + .toBeInTheDocument(); + }); + }); + }); +}); diff --git a/src/discussions/posts/post-editor/messages.js b/src/discussions/posts/post-editor/messages.js index 4f0f498c..7fa6ec5a 100644 --- a/src/discussions/posts/post-editor/messages.js +++ b/src/discussions/posts/post-editor/messages.js @@ -93,6 +93,10 @@ const messages = defineMessages({ id: 'discussions.editor.submit', defaultMessage: 'Submit', }, + submitting: { + id: 'discussions.editor.submitting', + defaultMessage: 'Submitting', + }, cancel: { id: 'discussions.editor.cancel', defaultMessage: 'Cancel', diff --git a/src/discussions/topics/data/selectors.js b/src/discussions/topics/data/selectors.js index 99c30ebd..c906f4de 100644 --- a/src/discussions/topics/data/selectors.js +++ b/src/discussions/topics/data/selectors.js @@ -14,6 +14,9 @@ export const selectCoursewareTopics = state => state.topics.categoryIds.map(cate name: category, children: state.topics.topicsInCategory[category].map(id => state.topics.topics[id]), })); + +export const selectNonCoursewareIds = state => state.topics.nonCoursewareIds; + export const selectNonCoursewareTopics = state => state.topics.nonCoursewareIds.map(id => state.topics.topics[id]); export const selectTopic = topicId => state => state.topics.topics[topicId];