From 7f4c28f4bafbbfa42b68b7df26492779d48f1468 Mon Sep 17 00:00:00 2001
From: Awais Ansari <79941147+awais-ansari@users.noreply.github.com>
Date: Thu, 3 Jun 2021 17:32:18 +0500
Subject: [PATCH] feat: Add Divide discussions by cohorts funtionality (#122)
* feat: Add Divide discussions by cohorts funtionality
* feat: Destructure values in DivisionByGroupField and remove unnecessary async calls from thunks
* feat: update checkbox component in DivisionByGroupFields
* refactor: change variable names
---
src/i18n/messages/ar.json | 2 +-
src/i18n/messages/es_419.json | 2 +-
src/i18n/messages/fr.json | 2 +-
src/i18n/messages/zh_CN.json | 2 +-
.../discussions/DiscussionsSettings.test.jsx | 4 +-
.../app-config-form/AppConfigForm.jsx | 4 +-
.../apps/legacy/LegacyConfigForm.jsx | 13 +-
.../apps/legacy/LegacyConfigForm.test.jsx | 70 +++++---
.../apps/shared/DivisionByGroupFields.jsx | 154 ++++++++++++++----
.../discussion-topics/DiscussionTopics.jsx | 23 ++-
.../app-config-form/apps/shared/messages.js | 8 +-
.../discussions/data/api.js | 26 +--
.../discussions/data/redux.test.js | 55 +++++--
.../discussions/data/slice.js | 10 +-
.../discussions/data/thunks.js | 19 +--
.../discussions/factories/mockApiResponses.js | 7 +-
16 files changed, 265 insertions(+), 136 deletions(-)
diff --git a/src/i18n/messages/ar.json b/src/i18n/messages/ar.json
index 0899e37df..c34acdc77 100644
--- a/src/i18n/messages/ar.json
+++ b/src/i18n/messages/ar.json
@@ -171,4 +171,4 @@
"header.label.main.header": "الرئيسية",
"header.label.secondary.nav": "ثانوي",
"header.label.courseOutline": "العودة إلى مخطط المساق في استوديو"
-}
\ No newline at end of file
+}
diff --git a/src/i18n/messages/es_419.json b/src/i18n/messages/es_419.json
index b7d4d67cc..45ea0d5bd 100644
--- a/src/i18n/messages/es_419.json
+++ b/src/i18n/messages/es_419.json
@@ -171,4 +171,4 @@
"header.label.main.header": "Main",
"header.label.secondary.nav": "Secondary",
"header.label.courseOutline": "Back to course outline in Studio"
-}
\ No newline at end of file
+}
diff --git a/src/i18n/messages/fr.json b/src/i18n/messages/fr.json
index b7d4d67cc..45ea0d5bd 100644
--- a/src/i18n/messages/fr.json
+++ b/src/i18n/messages/fr.json
@@ -171,4 +171,4 @@
"header.label.main.header": "Main",
"header.label.secondary.nav": "Secondary",
"header.label.courseOutline": "Back to course outline in Studio"
-}
\ No newline at end of file
+}
diff --git a/src/i18n/messages/zh_CN.json b/src/i18n/messages/zh_CN.json
index b7d4d67cc..45ea0d5bd 100644
--- a/src/i18n/messages/zh_CN.json
+++ b/src/i18n/messages/zh_CN.json
@@ -171,4 +171,4 @@
"header.label.main.header": "Main",
"header.label.secondary.nav": "Secondary",
"header.label.courseOutline": "Back to course outline in Studio"
-}
\ No newline at end of file
+}
diff --git a/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx b/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx
index 4a80501ef..1961c134d 100644
--- a/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx
+++ b/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx
@@ -24,7 +24,7 @@ import DiscussionsSettings from './DiscussionsSettings';
import PagesAndResourcesProvider from '../PagesAndResourcesProvider';
import initializeStore from '../../store';
import { getAppsUrl } from './data/api';
-import { piazzaApiResponse } from './factories/mockApiResponses';
+import { piazzaApiResponse, legacyApiResponse } from './factories/mockApiResponses';
import appMessages from './app-config-form/messages';
import appListMessages from './app-list/messages';
@@ -117,6 +117,8 @@ describe('DiscussionsSettings', () => {
});
test('successfully advances to settings step for legacy', async () => {
+ axiosMock.onGet(getAppsUrl(courseId)).reply(200, legacyApiResponse);
+ renderComponent();
history.push(`/course/${courseId}/pages-and-resources/discussions`);
// This is an important line that ensures the spinner has been removed - and thus our main
diff --git a/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx
index 682f36905..bd1333d6e 100644
--- a/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx
@@ -30,7 +30,7 @@ function AppConfigForm({
const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext);
const { params: { appId: routeAppId } } = useRouteMatch();
const {
- selectedAppId, status, saveStatus, discussionTopicIds,
+ selectedAppId, status, saveStatus, discussionTopicIds, divideDiscussionIds,
} = useSelector(state => state.discussions);
const app = useModel('apps', selectedAppId);
// appConfigs have no ID of their own, so we use the active app ID to reference them.
@@ -38,7 +38,7 @@ function AppConfigForm({
// if we're configuring a new app.
const appConfigObj = useModel('appConfigs', selectedAppId);
const discussionTopics = useModels('discussionTopics', discussionTopicIds);
- const appConfig = { ...appConfigObj, discussionTopics };
+ const appConfig = { ...appConfigObj, discussionTopics, divideDiscussionIds };
useEffect(() => {
if (status === LOADED) {
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx
index a7cbd1f8c..8e2edd2c9 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx
@@ -75,7 +75,7 @@ function LegacyConfigForm({
@@ -96,14 +96,13 @@ function LegacyConfigForm({
LegacyConfigForm.propTypes = {
appConfig: PropTypes.shape({
divideByCohorts: PropTypes.bool.isRequired,
- divideCourseWideTopics: PropTypes.bool.isRequired,
- divideGeneralTopic: PropTypes.bool.isRequired,
- divideQuestionsForTAsTopic: PropTypes.bool.isRequired,
+ divideCourseTopicsByCohorts: PropTypes.bool.isRequired,
allowAnonymousPosts: PropTypes.bool.isRequired,
allowAnonymousPostsPeers: PropTypes.bool.isRequired,
blackoutDates: PropTypes.string.isRequired,
discussionTopics: PropTypes.arrayOf(PropTypes.shape({
- name: PropTypes.string.isRequired,
+ name: PropTypes.string,
+ id: PropTypes.string,
})),
}),
onSubmit: PropTypes.func.isRequired,
@@ -117,9 +116,7 @@ LegacyConfigForm.defaultProps = {
appConfig: {
divideByCohorts: false,
allowDivisionByUnit: false,
- divideCourseWideTopics: false,
- divideGeneralTopic: false,
- divideQuestionsForTAsTopic: false,
+ divideCourseTopicsByCohorts: false,
allowAnonymousPosts: false,
allowAnonymousPostsPeers: false,
blackoutDates: '[]',
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx
index 429a621a3..bf8c9da66 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx
@@ -20,15 +20,18 @@ const courseId = 'course-v1:edX+TestX+Test_Course';
const defaultAppConfig = {
id: 'legacy',
divideByCohorts: false,
- divideCourseWideTopics: false,
- divideGeneralTopic: false,
- divideQuestionsForTAsTopic: false,
+ divideCourseTopicsByCohorts: false,
discussionTopics: [
- { name: 'General', id: 'course-generated-id-123-client-made-this-up' },
+ { name: 'General', id: 'course' },
{ name: 'Edx', id: '13f106c6-6735-4e84-b097-0456cff55960' },
],
+ divideDiscussionIds: [
+ 'course',
+ '13f106c6-6735-4e84-b097-0456cff55960',
+ ],
allowAnonymousPosts: false,
allowAnonymousPostsPeers: false,
+ allowDivisionByUnit: false,
blackoutDates: '[]',
};
@@ -89,7 +92,10 @@ describe('LegacyConfigForm', () => {
const handleSubmit = jest.fn();
await mockStore(legacyApiResponse);
- createComponent(defaultAppConfig, handleSubmit, formRef);
+ createComponent({
+ ...defaultAppConfig,
+ divideByCohorts: true,
+ }, handleSubmit, formRef);
await act(async () => {
formRef.current.submit();
@@ -99,7 +105,10 @@ describe('LegacyConfigForm', () => {
// Because we use defaultAppConfig as the initialValues of the form, and we haven't changed
// any of the form inputs, this exact object shape is returned back to us, so we're reusing
// it here. It's not supposed to be 'the same object', it just happens to be.
- defaultAppConfig,
+ {
+ ...defaultAppConfig,
+ divideByCohorts: true,
+ },
);
});
@@ -111,15 +120,13 @@ describe('LegacyConfigForm', () => {
expect(container.querySelector('#divideByCohorts')).toBeInTheDocument();
expect(container.querySelector('#divideByCohorts')).not.toBeChecked();
expect(
- container.querySelector('#divideCourseWideTopics'),
- ).not.toBeInTheDocument();
- expect(
- container.querySelector('#divideGeneralTopic'),
- ).not.toBeInTheDocument();
- expect(
- container.querySelector('#divideQuestionsForTAsTopic'),
+ container.querySelector('#divideCourseTopicsByCohorts'),
).not.toBeInTheDocument();
+ defaultAppConfig.divideDiscussionIds.forEach(id => expect(
+ container.querySelector(`#checkbox-${id}`),
+ ).not.toBeInTheDocument());
+
// AnonymousPostingFields
expect(container.querySelector('#allowAnonymousPosts')).toBeInTheDocument();
expect(container.querySelector('#allowAnonymousPosts')).not.toBeChecked();
@@ -144,20 +151,16 @@ describe('LegacyConfigForm', () => {
expect(container.querySelector('#divideByCohorts')).toBeInTheDocument();
expect(container.querySelector('#divideByCohorts')).toBeChecked();
expect(
- container.querySelector('#divideCourseWideTopics'),
+ container.querySelector('#divideCourseTopicsByCohorts'),
).toBeInTheDocument();
expect(
- container.querySelector('#divideCourseWideTopics'),
- ).not.toBeChecked();
- expect(container.querySelector('#divideGeneralTopic')).toBeInTheDocument();
- expect(container.querySelector('#divideGeneralTopic')).not.toBeChecked();
- expect(
- container.querySelector('#divideQuestionsForTAsTopic'),
- ).toBeInTheDocument();
- expect(
- container.querySelector('#divideQuestionsForTAsTopic'),
+ container.querySelector('#divideCourseTopicsByCohorts'),
).not.toBeChecked();
+ defaultAppConfig.divideDiscussionIds.forEach(id => expect(
+ container.querySelector(`#checkbox-${id}`),
+ ).not.toBeInTheDocument());
+
// AnonymousPostingFields
expect(container.querySelector('#allowAnonymousPosts')).toBeInTheDocument();
expect(container.querySelector('#allowAnonymousPosts')).toBeChecked();
@@ -168,4 +171,25 @@ describe('LegacyConfigForm', () => {
container.querySelector('#allowAnonymousPostsPeers'),
).not.toBeChecked();
});
+
+ test('folded discussion topics are in the DOM when divideByCohorts and divideCourseWideTopicsare enabled',
+ async () => {
+ await mockStore(legacyApiResponse);
+ createComponent({
+ ...defaultAppConfig,
+ divideByCohorts: true,
+ divideCourseTopicsByCohorts: true,
+ });
+
+ // DivisionByGroupFields
+ expect(container.querySelector('#divideByCohorts')).toBeInTheDocument();
+ expect(container.querySelector('#divideByCohorts')).toBeChecked();
+ expect(container.querySelector('#divideCourseTopicsByCohorts')).toBeInTheDocument();
+ expect(container.querySelector('#divideCourseTopicsByCohorts')).toBeChecked();
+
+ defaultAppConfig.divideDiscussionIds.forEach(id => {
+ expect(container.querySelector(`#checkbox-${id}`)).toBeInTheDocument();
+ expect(container.querySelector(`#checkbox-${id}`)).toBeChecked();
+ });
+ });
});
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/DivisionByGroupFields.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/DivisionByGroupFields.jsx
index 61f40fe44..faaaa3f1d 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/shared/DivisionByGroupFields.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/DivisionByGroupFields.jsx
@@ -1,61 +1,142 @@
-import React from 'react';
+import React, { useEffect } from 'react';
+import { useDispatch } from 'react-redux';
import PropTypes from 'prop-types';
import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { Form, TransitionReplace } from '@edx/paragon';
+import { FieldArray, useFormikContext } from 'formik';
import FormSwitchGroup from '../../../../../generic/FormSwitchGroup';
import messages from './messages';
import AppConfigFormDivider from './AppConfigFormDivider';
+import { updateDividedDiscussionsIds } from '../../../data/slice';
+import { updateModel } from '../../../../../generic/model-store';
function DivisionByGroupFields({
- onBlur,
- onChange,
- intl,
- values,
+ onBlur, onChange, intl, appConfig,
}) {
+ const dispatch = useDispatch();
+ const { setFieldValue } = useFormikContext();
+ const {
+ divideDiscussionIds,
+ discussionTopics,
+ divideByCohorts,
+ divideCourseTopicsByCohorts,
+ } = appConfig;
+
+ useEffect(() => {
+ dispatch(updateDividedDiscussionsIds({ divideDiscussionIds }));
+ }, [divideDiscussionIds]);
+
+ useEffect(() => {
+ const discussionTopicIds = discussionTopics.map(
+ (topic) => topic.id,
+ );
+ const divideCourseTopicsByCohortsOff = (
+ discussionTopicIds.length === divideDiscussionIds.length
+ && discussionTopicIds.every((topicId) => divideDiscussionIds.includes(topicId))
+ ) || !divideDiscussionIds.length;
+
+ if (divideByCohorts) {
+ if (divideCourseTopicsByCohortsOff && !divideCourseTopicsByCohorts) {
+ setFieldValue('divideCourseTopicsByCohorts', false);
+ setFieldValue('divideDiscussionIds', discussionTopicIds);
+ } else {
+ setFieldValue('divideCourseTopicsByCohorts', true);
+ }
+ } else {
+ setFieldValue('divideDiscussionIds', []);
+ setFieldValue('divideCourseTopicsByCohorts', false);
+ }
+
+ const {
+ divideDiscussionIds: courseDividedDiscussionsIds,
+ discussionTopics: courseDiscussionTopics,
+ ...payload
+ } = appConfig;
+ dispatch(updateModel({ modelType: 'appConfigs', model: payload }));
+ }, [
+ divideByCohorts,
+ divideCourseTopicsByCohorts,
+ discussionTopics,
+ ]);
+
+ const handleCheckBoxToggle = (event, push, remove) => {
+ const { checked, value } = event.target;
+ if (checked) {
+ push(value);
+ } else {
+ remove(divideDiscussionIds.indexOf(value));
+ }
+ };
+
+ const handleDivideCourseTopicsByCohortsToggle = (event) => {
+ const { checked } = event.target;
+ if (!checked) {
+ setFieldValue('divideDiscussionIds', []);
+ }
+ onChange(event);
+ };
+
return (
<>
- {intl.formatMessage(messages.divisionByGroup)}
-
+
+ {intl.formatMessage(messages.divisionByGroup)}
+
- {values.divideByCohorts ? (
+ {divideByCohorts ? (
handleDivideCourseTopicsByCohortsToggle(event)}
onBlur={onBlur}
className="ml-4 mt-3"
- id="divideCourseWideTopics"
- checked={values.divideCourseWideTopics}
- label={intl.formatMessage(messages.divideCourseWideTopicsLabel)}
- helpText={intl.formatMessage(messages.divideCourseWideTopicsHelp)}
+ id="divideCourseTopicsByCohorts"
+ checked={divideCourseTopicsByCohorts}
+ label={intl.formatMessage(messages.divideCourseTopicsByCohortsLabel)}
+ helpText={intl.formatMessage(messages.divideCourseTopicsByCohortsHelp)}
/>
-
-
-
-
+
+ {divideCourseTopicsByCohorts ? (
+
+ (
+
+ handleCheckBoxToggle(event, push, remove)}
+ onBlur={onBlur}
+ defaultValue={divideDiscussionIds}
+ >
+ {discussionTopics.map((topic) => (
+
+ {topic.name}
+
+ ))}
+
+
+ )}
+ />
+
+ ) : (
+
+ )}
+
- ) : }
+ ) : (
+
+ )}
);
@@ -65,11 +146,14 @@ DivisionByGroupFields.propTypes = {
onBlur: PropTypes.func.isRequired,
onChange: PropTypes.func.isRequired,
intl: intlShape.isRequired,
- values: PropTypes.shape({
+ appConfig: PropTypes.shape({
divideByCohorts: PropTypes.bool,
- divideCourseWideTopics: PropTypes.bool,
- divideGeneralTopic: PropTypes.bool,
- divideQuestionsForTAsTopic: PropTypes.bool,
+ divideCourseTopicsByCohorts: PropTypes.bool,
+ discussionTopics: PropTypes.arrayOf(PropTypes.shape({
+ name: PropTypes.string,
+ id: PropTypes.string,
+ })),
+ divideDiscussionIds: PropTypes.arrayOf(PropTypes.string),
}).isRequired,
};
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/DiscussionTopics.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/DiscussionTopics.jsx
index 090132d9c..dc1f02e98 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/DiscussionTopics.jsx
+++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/DiscussionTopics.jsx
@@ -8,31 +8,36 @@ import { v4 as uuid } from 'uuid';
import messages from '../messages';
import TopicItem from './TopicItem';
-import { removeModel } from '../../../../../../generic/model-store';
+import { removeModel, updateModels } from '../../../../../../generic/model-store';
import { updateDiscussionTopicIds } from '../../../../data/slice';
-import { updatedDiscussionTopics } from '../../../../data/thunks';
const DiscussionTopics = ({ intl }) => {
const dispatch = useDispatch();
- const { values } = useFormikContext();
- const [topics, setTopics] = useState(values.discussionTopics);
+ const { values: appConfig, setFieldValue } = useFormikContext();
+ const { discussionTopics, divideDiscussionIds } = appConfig;
+ const [topics, setTopics] = useState(discussionTopics);
useEffect(() => {
- const updatedDiscussionTopicIds = values.discussionTopics?.map(topic => topic.id);
+ const updatedDiscussionTopicIds = discussionTopics.map(topic => topic.id);
- setTopics(values.discussionTopics);
- dispatch(updateDiscussionTopicIds(updatedDiscussionTopicIds));
- dispatch(updatedDiscussionTopics(values.discussionTopics));
- }, [values.discussionTopics]);
+ setTopics(discussionTopics);
+ dispatch(updateDiscussionTopicIds({ updatedDiscussionTopicIds }));
+ dispatch(updateModels({ modelType: 'discussionTopics', models: discussionTopics }));
+ }, [discussionTopics]);
const handleTopicDelete = (topicIndex, topicId, remove) => {
remove(topicIndex);
dispatch(removeModel({ modelType: 'discussionTopics', id: topicId }));
+ const updatedDividedDiscussionsIds = divideDiscussionIds.filter(
+ (id) => id !== topicId,
+ );
+ setFieldValue('divideDiscussionIds', updatedDividedDiscussionsIds);
};
const addNewTopic = (push) => {
const payload = { name: '', id: uuid() };
push(payload);
+ setFieldValue('divideDiscussionIds', [...divideDiscussionIds, payload.id]);
};
return (
diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/messages.js b/src/pages-and-resources/discussions/app-config-form/apps/shared/messages.js
index 99c5f08b2..1b3c109fb 100644
--- a/src/pages-and-resources/discussions/app-config-form/apps/shared/messages.js
+++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/messages.js
@@ -16,13 +16,13 @@ const messages = defineMessages({
defaultMessage: 'Learners will only be able to view and respond to discussions posted by members of their cohort.',
description: 'Help text for a switch that enables dividing discussions by cohorts.',
},
- divideCourseWideTopicsLabel: {
- id: 'authoring.discussions.builtIn.divideCourseWideTopics.label',
+ divideCourseTopicsByCohortsLabel: {
+ id: 'authoring.discussions.builtIn.divideCourseTopicsByCohorts.label',
defaultMessage: 'Divide course wide discussion topics',
description: 'Label for a switch that enables dividing course wide topics by cohorts.',
},
- divideCourseWideTopicsHelp: {
- id: 'authoring.discussions.builtIn.divideCourseWideTopics.help',
+ divideCourseTopicsByCohortsHelp: {
+ id: 'authoring.discussions.builtIn.divideCourseTopicsByCohorts.help',
defaultMessage: 'Choose which of your general course wide discussion topics you would like to divide.',
description: 'Help text asking the user to pick course-wide topics that should be divided by cohort.',
},
diff --git a/src/pages-and-resources/discussions/data/api.js b/src/pages-and-resources/discussions/data/api.js
index 56466138c..e27337159 100644
--- a/src/pages-and-resources/discussions/data/api.js
+++ b/src/pages-and-resources/discussions/data/api.js
@@ -33,22 +33,13 @@ function normalizePluginConfig(data) {
if (!data || Object.keys(data).length < 1) {
return {};
}
-
return {
allowAnonymousPosts: data.allow_anonymous,
allowAnonymousPostsPeers: data.allow_anonymous_to_peers,
blackoutDates: JSON.stringify(data.discussion_blackouts),
- // TODO: We need all these added to the API. ... default them for now
- divideByCohorts: false,
allowDivisionByUnit: false,
- divideCourseWideTopics: false,
- // TODO: Note that these last two are in the `discussion_topics` data, but we haven't been able
- // to add them properly here yet. I'm not sure the data is in a usable state as is, since it
- // only seems to include the topics for which these would be "true". Assuming its only these
- // two, I think we could check for the existence of "General" for the first, but I'm not sure
- // what the topic title is for the second. "Questions for TAs" maybe?
- divideGeneralTopic: false,
- divideQuestionsForTAsTopic: false,
+ divideByCohorts: data.divided_course_wide_discussions.length > 0,
+ divideCourseTopicsByCohorts: false,
};
}
@@ -77,19 +68,11 @@ function normalizeApps(data) {
? extractDiscussionTopicIds(data.plugin_configuration.discussion_topics) : [],
discussionTopics: data.plugin_configuration.discussion_topics
? normalizeDiscussionTopic(data.plugin_configuration.discussion_topics) : [],
+ divideDiscussionIds: data.plugin_configuration.divided_course_wide_discussions,
};
}
function denormalizeData(courseId, appId, data) {
- /*
- TODO: What about these? Some are from the instructor dashboard, presumably... but I don't think they all are.
-
- divideByCohorts
- allowDivisionByUnit
- divideCourseWideTopics
- divideGeneralTopic
- divideQuestionsForTAsTopic
- */
const pluginConfiguration = {};
if (data.allowAnonymousPosts) {
@@ -108,6 +91,9 @@ function denormalizeData(courseId, appId, data) {
return newTopics;
}, {});
}
+ if (data.divideDiscussionIds) {
+ pluginConfiguration.divided_course_wide_discussions = data.divideDiscussionIds;
+ }
const ltiConfiguration = {};
diff --git a/src/pages-and-resources/discussions/data/redux.test.js b/src/pages-and-resources/discussions/data/redux.test.js
index 8d630c315..b0716b4cd 100644
--- a/src/pages-and-resources/discussions/data/redux.test.js
+++ b/src/pages-and-resources/discussions/data/redux.test.js
@@ -61,6 +61,8 @@ const piazzaApp = {
let axiosMock;
let store;
+let divideDiscussionIds;
+let discussionTopicIds;
describe('Data layer integration tests', () => {
beforeEach(() => {
@@ -76,6 +78,14 @@ describe('Data layer integration tests', () => {
axiosMock = new MockAdapter(getAuthenticatedHttpClient());
store = initializeStore();
+ divideDiscussionIds = [
+ '13f106c6-6735-4e84-b097-0456cff55960',
+ 'course',
+ ];
+ discussionTopicIds = [
+ '13f106c6-6735-4e84-b097-0456cff55960',
+ 'course',
+ ];
});
afterEach(() => {
@@ -158,10 +168,8 @@ describe('Data layer integration tests', () => {
status: LOADED,
saveStatus: SAVED,
hasValidationError: false,
- discussionTopicIds: [
- '13f106c6-6735-4e84-b097-0456cff55960',
- 'course-generated-id-123-client-made-this-up',
- ],
+ discussionTopicIds,
+ divideDiscussionIds,
});
expect(store.getState().models.apps.legacy).toEqual(legacyApp);
expect(store.getState().models.apps.piazza).toEqual(piazzaApp);
@@ -173,11 +181,9 @@ describe('Data layer integration tests', () => {
blackoutDates: '[]',
// TODO: Note! As of this writing, all the data below this line is NOT returned in the API
// but we add it in during normalization.
- divideByCohorts: false,
+ divideByCohorts: true,
allowDivisionByUnit: false,
- divideCourseWideTopics: false,
- divideGeneralTopic: false,
- divideQuestionsForTAsTopic: false,
+ divideCourseTopicsByCohorts: false,
});
});
});
@@ -324,6 +330,14 @@ describe('Data layer integration tests', () => {
allow_anonymous: true,
allow_anonymous_to_peers: true,
discussion_blackouts: [['2015-09-15', '2015-09-21'], ['2015-10-01', '2015-10-08']],
+ discussion_topics: {
+ Edx: { id: '13f106c6-6735-4e84-b097-0456cff55960' },
+ General: { id: 'course' },
+ },
+ divided_course_wide_discussions: [
+ '13f106c6-6735-4e84-b097-0456cff55960',
+ 'course',
+ ],
},
provider_type: 'legacy',
}).reply(200, {
@@ -333,6 +347,14 @@ describe('Data layer integration tests', () => {
allow_anonymous: true,
allow_anonymous_to_peers: true,
discussion_blackouts: [['2015-09-15', '2015-09-21'], ['2015-10-01', '2015-10-08']],
+ discussion_topics: {
+ Edx: { id: '13f106c6-6735-4e84-b097-0456cff55960' },
+ General: { id: 'course' },
+ },
+ divided_course_wide_discussions: [
+ '13f106c6-6735-4e84-b097-0456cff55960',
+ 'course',
+ ],
},
});
@@ -350,9 +372,12 @@ describe('Data layer integration tests', () => {
// but we technically send it to the thunk, so here it is.
divideByCohorts: true,
allowDivisionByUnit: true,
- divideCourseWideTopics: true,
- divideGeneralTopic: true,
- divideQuestionsForTAsTopic: true,
+ divideCourseTopicsByCohorts: false,
+ divideDiscussionIds,
+ discussionTopics: [
+ { name: 'Edx', id: '13f106c6-6735-4e84-b097-0456cff55960' },
+ { name: 'General', id: 'course' },
+ ],
},
pagesAndResourcesPath,
), store.dispatch);
@@ -367,6 +392,8 @@ describe('Data layer integration tests', () => {
status: LOADED,
saveStatus: SAVED,
hasValidationError: false,
+ divideDiscussionIds,
+ discussionTopicIds,
}),
);
expect(store.getState().models.appConfigs.legacy).toEqual({
@@ -377,11 +404,9 @@ describe('Data layer integration tests', () => {
blackoutDates: '[["2015-09-15","2015-09-21"],["2015-10-01","2015-10-08"]]',
// TODO: Note! The values we tried to save were ignored, this test reflects what currently
// happens, but NOT what we want to have happen!
- divideByCohorts: false,
+ divideByCohorts: true,
allowDivisionByUnit: false,
- divideCourseWideTopics: false,
- divideGeneralTopic: false,
- divideQuestionsForTAsTopic: false,
+ divideCourseTopicsByCohorts: false,
});
});
});
diff --git a/src/pages-and-resources/discussions/data/slice.js b/src/pages-and-resources/discussions/data/slice.js
index e6dcfa0a6..1d79b06ed 100644
--- a/src/pages-and-resources/discussions/data/slice.js
+++ b/src/pages-and-resources/discussions/data/slice.js
@@ -24,6 +24,7 @@ const slice = createSlice({
// ValidationError is the Flag that represents a form validation status.
hasValidationError: false,
discussionTopicIds: [],
+ divideDiscussionIds: [],
},
reducers: {
loadApps: (state, { payload }) => {
@@ -33,6 +34,7 @@ const slice = createSlice({
state.status = LOADED;
state.saveStatus = SAVED;
state.discussionTopicIds = payload.discussionTopicIds;
+ state.divideDiscussionIds = payload.divideDiscussionIds;
},
selectApp: (state, { payload }) => {
const { appId } = payload;
@@ -51,7 +53,12 @@ const slice = createSlice({
state.hasValidationError = hasError;
},
updateDiscussionTopicIds: (state, { payload }) => {
- state.discussionTopicIds = payload;
+ const { updatedDiscussionTopicIds } = payload;
+ state.discussionTopicIds = updatedDiscussionTopicIds;
+ },
+ updateDividedDiscussionsIds: (state, { payload }) => {
+ const { divideDiscussionIds } = payload;
+ state.divideDiscussionIds = divideDiscussionIds;
},
},
});
@@ -63,6 +70,7 @@ export const {
updateSaveStatus,
updateValidationStatus,
updateDiscussionTopicIds,
+ updateDividedDiscussionsIds,
} = slice.actions;
export const {
diff --git a/src/pages-and-resources/discussions/data/thunks.js b/src/pages-and-resources/discussions/data/thunks.js
index 246842ca2..6e58e44e7 100644
--- a/src/pages-and-resources/discussions/data/thunks.js
+++ b/src/pages-and-resources/discussions/data/thunks.js
@@ -1,8 +1,5 @@
import { history } from '@edx/frontend-platform';
-import {
- addModel, addModels, updateModels,
-} from '../../../generic/model-store';
-
+import { addModel, addModels } from '../../../generic/model-store';
import { getApps, postAppConfig } from './api';
import {
FAILED,
@@ -26,11 +23,12 @@ export function fetchApps(courseId) {
appConfig,
discussionTopicIds,
discussionTopics,
+ divideDiscussionIds,
} = await getApps(courseId);
+ dispatch(addModel({ modelType: 'appConfigs', model: appConfig }));
dispatch(addModels({ modelType: 'apps', models: apps }));
dispatch(addModels({ modelType: 'features', models: features }));
- dispatch(addModel({ modelType: 'appConfigs', model: appConfig }));
dispatch(addModels({ modelType: 'discussionTopics', models: discussionTopics }));
dispatch(loadApps({
@@ -38,6 +36,7 @@ export function fetchApps(courseId) {
appIds: apps.map(app => app.id),
featureIds: features.map(feature => feature.id),
discussionTopicIds,
+ divideDiscussionIds,
}));
} catch (error) {
if (error.response && error.response.status === 403) {
@@ -61,11 +60,12 @@ export function saveAppConfig(courseId, appId, drafts, successPath) {
appConfig,
discussionTopicIds,
discussionTopics,
+ divideDiscussionIds,
} = await postAppConfig(courseId, appId, drafts);
+ dispatch(addModel({ modelType: 'appConfigs', model: appConfig }));
dispatch(addModels({ modelType: 'apps', models: apps }));
dispatch(addModels({ modelType: 'features', models: features }));
- dispatch(addModel({ modelType: 'appConfigs', model: appConfig }));
dispatch(addModels({ modelType: 'discussionTopics', models: discussionTopics }));
dispatch(loadApps({
@@ -73,6 +73,7 @@ export function saveAppConfig(courseId, appId, drafts, successPath) {
appIds: apps.map(app => app.id),
featureIds: features.map(feature => feature.id),
discussionTopicIds,
+ divideDiscussionIds,
}));
dispatch(updateSaveStatus({ status: SAVED }));
// Note that we redirect here to avoid having to work with the promise over in AppConfigForm.
@@ -88,9 +89,3 @@ export function saveAppConfig(courseId, appId, drafts, successPath) {
}
};
}
-
-export function updatedDiscussionTopics(payload) {
- return async (dispatch) => {
- dispatch(updateModels({ modelType: 'discussionTopics', models: payload }));
- };
-}
diff --git a/src/pages-and-resources/discussions/factories/mockApiResponses.js b/src/pages-and-resources/discussions/factories/mockApiResponses.js
index 585c8e5ed..ac731e9fa 100644
--- a/src/pages-and-resources/discussions/factories/mockApiResponses.js
+++ b/src/pages-and-resources/discussions/factories/mockApiResponses.js
@@ -52,9 +52,12 @@ export const legacyApiResponse = {
available_division_schemes: ['enrollment_track'],
discussion_topics: {
Edx: { id: '13f106c6-6735-4e84-b097-0456cff55960' },
- General: { id: 'course-generated-id-123-client-made-this-up' },
+ General: { id: 'course' },
},
- divided_course_wide_discussions: [],
+ divided_course_wide_discussions: [
+ '13f106c6-6735-4e84-b097-0456cff55960',
+ 'course',
+ ],
divided_inline_discussions: [],
division_scheme: 'none',
// Note, this gets stringified when normalized into the app, but the API returns it as an