Merge pull request #39 from edx/kshitij/cohort-support

feat: Fully implements support for cohorts [BD-38] [TNL-9304]
This commit is contained in:
Kshitij Sobti
2021-12-08 13:00:37 +00:00
committed by GitHub
15 changed files with 427 additions and 49 deletions

20
package-lock.json generated
View File

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

View File

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

View File

@@ -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 (
<Editor
init={{
@@ -83,7 +91,7 @@ export default function TinyMCEEditor(props) {
+ ' | openedx_html'
+ ' | undo redo',
content_css: false,
content_style: [contentCss, contentUiCss, edxBrandCss].join('\n'),
content_style: contentStyle,
images_upload_handler: uploadHandler,
setup,
}}

View File

@@ -0,0 +1,9 @@
import { Factory } from 'rosie';
Factory.define('config')
.attrs({
allow_anonymous: false,
allow_anonymous_to_peers: false,
user_is_privileged: false,
})
.attr('user_roles', ['user_is_privileged'], (userIsPrivileged) => (userIsPrivileged ? ['Student', 'Moderator'] : ['Student']));

View File

@@ -0,0 +1,2 @@
import './config.factory';
import './settings.factory';

View File

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

View File

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

View File

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

View File

@@ -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) => {

View File

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

View File

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

View File

@@ -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 (
<div className="m-4 card p-4 align-items-center">
<Spinner animation="border" variant="primary" />
</div>
);
}
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 (
<Formik
enableReinitialize
@@ -172,9 +193,8 @@ function PostEditor({
anonymousToPeers: Yup.bool()
.default(false)
.nullable(),
cohort: Yup.string(),
cohort: Yup.string().nullable().default(null),
})}
initialErrors={{}}
onSubmit={submitForm}
>{
({
@@ -242,7 +262,7 @@ function PostEditor({
))}
</Form.Control>
</Form.Group>
{canSelectCohort
{canSelectCohort(values.topic)
&& (
<Form.Group className="w-50 d-inline-block pl-2">
<Form.Control
@@ -251,7 +271,6 @@ function PostEditor({
value={values.cohort}
onChange={handleChange}
onBlur={handleBlur}
aria-describedby="cohortVisiblityInput"
floatingLabel={intl.formatMessage(messages.cohortVisibility)}
>
<option value="">{intl.formatMessage(messages.cohortVisibilityAllLearners)}</option>
@@ -334,17 +353,17 @@ function PostEditor({
)}
<div className="d-flex justify-content-end">
<StatefulButton
labels={{
default: intl.formatMessage(messages.cancel),
}}
<Button
variant="outline-primary"
onClick={hideEditor}
/>
>{intl.formatMessage(messages.cancel)}
</Button>
<StatefulButton
labels={{
default: intl.formatMessage(messages.submit),
pending: intl.formatMessage(messages.submitting),
}}
state={submitting ? 'pending' : 'default'}
className="ml-2"
variant="primary"
onClick={handleSubmit}

View File

@@ -0,0 +1,279 @@
import React from 'react';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
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 { threadsApiUrl } from '../data/api';
import { fetchThread } from '../data/thunks';
import { PostEditor } from '../index';
import '../../cohorts/data/__factories__';
import '../../data/__factories__';
import '../../topics/data/__factories__';
import '../data/__factories__';
const courseId = 'course-v1:edX+DemoX+Demo_Course';
const topicsApiUrl = `${API_BASE_URL}/api/discussion/v1/course_topics/${courseId}`;
let store;
let axiosMock;
async function renderComponent(editExisting = false, location = `/discussions/${courseId}/posts/`) {
const path = editExisting ? Routes.POSTS.EDIT_POST : Routes.POSTS.NEW_POSTS;
await render(
<IntlProvider locale="en">
<AppProvider store={store}>
<MemoryRouter initialEntries={[location]}>
<Route path={path}>
<PostEditor editExisting={editExisting} />
</Route>
</MemoryRouter>
</AppProvider>
</IntlProvider>,
);
}
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();
});
});
});
});

View File

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

View File

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