From 0ac5e2ed15702cd892d70b7bfe293107775a7517 Mon Sep 17 00:00:00 2001 From: Kshitij Sobti Date: Thu, 11 Nov 2021 18:14:38 +0530 Subject: [PATCH] feat: Add support for the new Open edX discussion provider This adds support for configuring the new Open edX discussion provider. It expands on the features of the legacy configuration by adding additional ways to configure discussions. --- .../discussions/DiscussionsSettings.jsx | 4 +- .../discussions/DiscussionsSettings.test.jsx | 73 ++++++---- .../app-config-form/AppConfigForm.jsx | 31 +++- .../app-config-form/apps/legacy/index.js | 1 - .../apps/lti/LtiConfigForm.jsx | 4 +- .../OpenedXConfigForm.jsx} | 87 ++++++++---- .../OpenedXConfigForm.test.jsx} | 37 +++-- .../OpenedXConfigFormProvider.jsx} | 10 +- .../app-config-form/apps/openedx/index.js | 1 + .../apps/shared/DivisionByGroupFields.jsx | 4 +- .../apps/shared/InContextDiscussionFields.jsx | 26 ++-- .../discussion-topics/DiscussionTopics.jsx | 4 +- .../DiscussionTopics.test.jsx | 14 +- .../discussion-topics/TopicItem.test.jsx | 8 +- .../discussions/app-config-form/messages.js | 7 +- .../discussions/app-list/AppCard.test.jsx | 8 +- .../discussions/app-list/AppList.test.jsx | 25 ++-- .../discussions/app-list/messages.js | 23 +++ .../discussions/data/api.js | 96 ++++++++----- .../discussions/data/redux.test.js | 110 ++++++++------- .../discussions/data/slice.js | 9 +- .../discussions/data/thunks.js | 65 +++++---- .../discussions/factories/mockApiResponses.js | 133 +++++++++--------- 23 files changed, 462 insertions(+), 318 deletions(-) delete mode 100644 src/pages-and-resources/discussions/app-config-form/apps/legacy/index.js rename src/pages-and-resources/discussions/app-config-form/apps/{legacy/LegacyConfigForm.jsx => openedx/OpenedXConfigForm.jsx} (69%) rename src/pages-and-resources/discussions/app-config-form/apps/{legacy/LegacyConfigForm.test.jsx => openedx/OpenedXConfigForm.test.jsx} (87%) rename src/pages-and-resources/discussions/app-config-form/apps/{legacy/LegacyConfigFormProvider.jsx => openedx/OpenedXConfigFormProvider.jsx} (75%) create mode 100644 src/pages-and-resources/discussions/app-config-form/apps/openedx/index.js diff --git a/src/pages-and-resources/discussions/DiscussionsSettings.jsx b/src/pages-and-resources/discussions/DiscussionsSettings.jsx index 691f82169a..8240de93c5 100644 --- a/src/pages-and-resources/discussions/DiscussionsSettings.jsx +++ b/src/pages-and-resources/discussions/DiscussionsSettings.jsx @@ -16,7 +16,7 @@ import { PagesAndResourcesContext } from '../PagesAndResourcesProvider'; import messages from './messages'; import DiscussionsProvider from './DiscussionsProvider'; -import { fetchApps } from './data/thunks'; +import { fetchProviders } from './data/thunks'; import AppList from './app-list'; import AppConfigForm from './app-config-form'; import { DENIED, FAILED } from './data/slice'; @@ -36,7 +36,7 @@ function DiscussionsSettings({ courseId, intl }) { const courseDetail = useModel('courseDetails', courseId); useEffect(() => { - dispatch(fetchApps(courseId)); + dispatch(fetchProviders(courseId)); }, [courseId]); const discussionsPath = `${pagesAndResourcesPath}/discussion`; diff --git a/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx b/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx index 261063b209..ce7b185470 100644 --- a/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx +++ b/src/pages-and-resources/discussions/DiscussionsSettings.test.jsx @@ -5,6 +5,7 @@ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { AppProvider, PageRoute } from '@edx/frontend-platform/react'; import { act, + findByRole, getByRole, queryByLabelText, queryByRole, @@ -19,21 +20,22 @@ import userEvent from '@testing-library/user-event'; import MockAdapter from 'axios-mock-adapter'; import React from 'react'; import { Switch } from 'react-router'; +import { fetchCourseDetail } from '../../data/thunks'; import initializeStore from '../../store'; +import { executeThunk } from '../../utils'; import PagesAndResourcesProvider from '../PagesAndResourcesProvider'; +import ltiMessages from './app-config-form/apps/lti/messages'; import appMessages from './app-config-form/messages'; import messages from './app-list/messages'; -import ltiMessages from './app-config-form/apps/lti/messages'; -import { getAppsUrl } from './data/api'; +import { getDiscussionsProvidersUrl, getDiscussionsSettingsUrl } from './data/api'; import DiscussionsSettings from './DiscussionsSettings'; import { + courseDetailResponse, generatePiazzaApiResponse, + generateProvidersApiResponse, legacyApiResponse, piazzaApiResponse, - courseDetailResponse, } from './factories/mockApiResponses'; -import { executeThunk } from '../../utils'; -import { fetchCourseDetail } from '../../data/thunks'; const courseId = 'course-v1:edX+TestX+Test_Course'; let axiosMock; @@ -88,7 +90,10 @@ describe('DiscussionsSettings', () => { describe('with successful network connections', () => { beforeEach(() => { - axiosMock.onGet(getAppsUrl(courseId)).reply(200, piazzaApiResponse); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)) + .reply(200, generateProvidersApiResponse(false)); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)) + .reply(200, piazzaApiResponse); renderComponent(); }); @@ -124,6 +129,8 @@ describe('DiscussionsSettings', () => { userEvent.click(queryByLabelText(container, 'Select Piazza')); userEvent.click(queryByText(container, messages.nextButton.defaultMessage)); + await waitForElementToBeRemoved(screen.getByRole('status')); + expect(queryByTestId(container, 'appList')).not.toBeInTheDocument(); expect(queryByTestId(container, 'appConfigForm')).toBeInTheDocument(); expect(queryByTestId(container, 'ltiConfigForm')).toBeInTheDocument(); @@ -131,7 +138,8 @@ describe('DiscussionsSettings', () => { }); test('successfully advances to settings step for legacy', async () => { - axiosMock.onGet(getAppsUrl(courseId)).reply(200, legacyApiResponse); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, generateProvidersApiResponse(false, 'legacy')); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, legacyApiResponse); renderComponent(); history.push(`/course/${courseId}/pages-and-resources/discussion`); @@ -139,9 +147,11 @@ describe('DiscussionsSettings', () => { // content has been loaded - prior to proceeding with our expectations. await waitForElementToBeRemoved(screen.getByRole('status')); - userEvent.click(queryByLabelText(container, 'Select edX')); + userEvent.click(queryByLabelText(container, 'Select edX (Legacy)')); userEvent.click(queryByText(container, messages.nextButton.defaultMessage)); + await waitForElementToBeRemoved(screen.getByRole('status')); + expect(queryByTestId(container, 'appList')).not.toBeInTheDocument(); expect(queryByTestId(container, 'appConfigForm')).toBeInTheDocument(); expect(queryByTestId(container, 'ltiConfigForm')).not.toBeInTheDocument(); @@ -183,7 +193,7 @@ describe('DiscussionsSettings', () => { test('successfully submit the modal', async () => { history.push(`/course/${courseId}/pages-and-resources/discussion`); - axiosMock.onPost(getAppsUrl(courseId)).reply(200, piazzaApiResponse); + axiosMock.onPost(getDiscussionsSettingsUrl(courseId)).reply(200, piazzaApiResponse); // This is an important line that ensures the spinner has been removed - and thus our main // content has been loaded - prior to proceeding with our expectations. @@ -193,7 +203,7 @@ describe('DiscussionsSettings', () => { userEvent.click(getByRole(container, 'button', { name: 'Next' })); - userEvent.click(getByRole(container, 'button', { name: 'Save' })); + userEvent.click(await findByRole(container, 'button', { name: 'Save' })); // This is an important line that ensures the Close button has been removed, which implies that // the full screen modal has been closed following our click of Apply. Once this has happened, @@ -215,6 +225,7 @@ describe('DiscussionsSettings', () => { userEvent.click(getByRole(container, 'checkbox', { name: 'Select Discourse' })); userEvent.click(getByRole(container, 'button', { name: 'Next' })); + await findByRole(container, 'button', { name: 'Save' }); userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Key' }), 'key'); userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Secret' }), 'secret'); userEvent.type(getByRole(container, 'textbox', { name: 'Launch URL' }), 'http://example.test'); @@ -237,6 +248,7 @@ describe('DiscussionsSettings', () => { userEvent.click(discourseBox); userEvent.click(getByRole(container, 'button', { name: 'Next' })); + await waitForElementToBeRemoved(screen.getByRole('status')); expect(getByRole(container, 'heading', { name: 'Discourse' })).toBeInTheDocument(); userEvent.type(getByRole(container, 'textbox', { name: 'Consumer Key' }), 'a'); @@ -252,7 +264,7 @@ describe('DiscussionsSettings', () => { }); }); - describe('with network error fetchApps API requests', () => { + describe('with network error fetchProviders API requests', () => { beforeEach(() => { // Expedient way of getting SUPPORT_URL into config. setConfig({ @@ -260,8 +272,8 @@ describe('DiscussionsSettings', () => { SUPPORT_URL: 'http://support.edx.org', }); - axiosMock.onGet(getAppsUrl(courseId)).networkError(); - + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).networkError(); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).networkError(); renderComponent(); }); @@ -287,8 +299,11 @@ describe('DiscussionsSettings', () => { SUPPORT_URL: 'http://support.edx.org', }); - axiosMock.onGet(getAppsUrl(courseId)).reply(200, piazzaApiResponse); - axiosMock.onPost(getAppsUrl(courseId)).networkError(); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)) + .reply(200, generateProvidersApiResponse()); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)) + .reply(200, piazzaApiResponse); + axiosMock.onPost(getDiscussionsSettingsUrl(courseId)).networkError(); renderComponent(); }); @@ -307,17 +322,17 @@ describe('DiscussionsSettings', () => { await waitFor(() => expect(axiosMock.history.post.length).toBe(1)); expect(queryByTestId(container, 'appConfigForm')).toBeInTheDocument(); - - const alert = queryByRole(container, 'alert'); + const alert = await findByRole(container, 'alert'); expect(alert).toBeInTheDocument(); expect(alert.textContent).toEqual(expect.stringContaining('We encountered a technical error when applying changes.')); expect(alert.innerHTML).toEqual(expect.stringContaining(getConfig().SUPPORT_URL)); }); }); - describe('with permission denied error for fetchApps API requests', () => { + describe('with permission denied error for fetchProviders API requests', () => { beforeEach(() => { - axiosMock.onGet(getAppsUrl(courseId)).reply(403); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(403); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(403); renderComponent(); }); @@ -337,8 +352,10 @@ describe('DiscussionsSettings', () => { describe('with permission denied error for postAppConfig API requests', () => { beforeEach(() => { - axiosMock.onGet(getAppsUrl(courseId)).reply(200, piazzaApiResponse); - axiosMock.onPost(getAppsUrl(courseId)).reply(403); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)) + .reply(200, generateProvidersApiResponse()); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, piazzaApiResponse); + axiosMock.onPost(getDiscussionsSettingsUrl(courseId)).reply(403); renderComponent(); }); @@ -360,7 +377,7 @@ describe('DiscussionsSettings', () => { // We don't technically leave the route in this case, though the modal is hidden. expect(window.location.pathname).toEqual(`/course/${courseId}/pages-and-resources/discussion/configure/piazza`); - const alert = queryByRole(container, 'alert'); + const alert = await findByRole(container, 'alert'); expect(alert).toBeInTheDocument(); expect(alert.textContent).toEqual(expect.stringContaining('You are not authorized to view this page.')); }); @@ -394,7 +411,10 @@ describe.each([ // Leave the DiscussionsSettings route after the test. history.push(`/course/${courseId}/pages-and-resources`); - axiosMock.onGet(getAppsUrl(courseId)).reply(200, generatePiazzaApiResponse(isAdminOnlyConfig)); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)) + .reply(200, generateProvidersApiResponse(isAdminOnlyConfig)); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)) + .reply(200, generatePiazzaApiResponse()); renderComponent(); }); @@ -408,6 +428,7 @@ describe.each([ userEvent.click(queryByLabelText(container, 'Select Piazza')); userEvent.click(queryByText(container, messages.nextButton.defaultMessage)); + await waitForElementToBeRemoved(screen.getByRole('status')); if (showLTIConfig) { expect(queryByText(container, ltiMessages.formInstructions.defaultMessage)).toBeInTheDocument(); @@ -446,7 +467,10 @@ describe.each([ // Leave the DiscussionsSettings route after the test. history.push(`/course/${courseId}/pages-and-resources`); - axiosMock.onGet(getAppsUrl(courseId)).reply(200, generatePiazzaApiResponse(false, piiSharingAllowed)); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)) + .reply(200, generateProvidersApiResponse(false)); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)) + .reply(200, generatePiazzaApiResponse(piiSharingAllowed)); renderComponent(); }); @@ -460,6 +484,7 @@ describe.each([ userEvent.click(queryByLabelText(container, 'Select Piazza')); userEvent.click(queryByText(container, messages.nextButton.defaultMessage)); + await waitForElementToBeRemoved(screen.getByRole('status')); if (enablePIISharing) { expect(queryByTestId(container, 'piiSharingFields')).toBeInTheDocument(); } else { 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 d898d5e2d4..73776e007a 100644 --- a/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/AppConfigForm.jsx @@ -21,8 +21,8 @@ import { DENIED, FAILED, LOADED, LOADING, selectApp, } from '../data/slice'; -import { saveAppConfig } from '../data/thunks'; -import LegacyConfigForm from './apps/legacy'; +import { fetchDiscussionSettings, saveProviderConfig } from '../data/thunks'; +import OpenedXConfigForm from './apps/openedx'; import LtiConfigForm from './apps/lti'; import AppConfigFormProvider, { AppConfigFormContext } from './AppConfigFormProvider'; import AppConfigFormSaveButton from './AppConfigFormSaveButton'; @@ -36,12 +36,20 @@ function AppConfigForm({ const { formRef } = useContext(AppConfigFormContext); const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext); const { params: { appId: routeAppId } } = useRouteMatch(); + const [isLoading, setLoading] = useState(true); const { activeAppId, selectedAppId, status, saveStatus, } = useSelector(state => state.discussions); const [confirmationDialogVisible, setConfirmationDialogVisible] = useState(false); + useEffect(() => { + (async () => { + await dispatch(fetchDiscussionSettings(courseId, selectedAppId)); + setLoading(false); + })(); + }, [courseId, selectedAppId]); + useEffect(() => { if (status === LOADED) { if (routeAppId !== selectedAppId) { @@ -57,12 +65,12 @@ function AppConfigForm({ setConfirmationDialogVisible(true); } else { setConfirmationDialogVisible(false); - // Note that when this action succeeds, we redirect to pagesAndResurcesPath in the thunk. - dispatch(saveAppConfig(courseId, selectedAppId, values, pagesAndResourcesPath)); + // Note that when this action succeeds, we redirect to pagesAndResourcesPath in the thunk. + dispatch(saveProviderConfig(courseId, selectedAppId, values, pagesAndResourcesPath)); } }, [activeAppId, confirmationDialogVisible, courseId, selectedAppId]); - if (!selectedAppId || status === LOADING) { + if (!selectedAppId || status === LOADING || isLoading) { return ( ); @@ -78,12 +86,21 @@ function AppConfigForm({ alert = ; } - let form = null; + let form; if (selectedAppId === 'legacy') { form = ( - + ); + } else if (selectedAppId === 'openedx') { + form = ( + ); } else { diff --git a/src/pages-and-resources/discussions/app-config-form/apps/legacy/index.js b/src/pages-and-resources/discussions/app-config-form/apps/legacy/index.js deleted file mode 100644 index f0b9341294..0000000000 --- a/src/pages-and-resources/discussions/app-config-form/apps/legacy/index.js +++ /dev/null @@ -1 +0,0 @@ -export { default } from './LegacyConfigForm'; diff --git a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx index 9d74736502..a5f7005055 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx @@ -21,9 +21,7 @@ ensureConfig(['SITE_NAME', 'SUPPORT_EMAIL'], 'LTI Config Form'); function LtiConfigForm({ onSubmit, intl, formRef }) { const dispatch = useDispatch(); - const { selectedAppId } = useSelector((state) => state.discussions); - - const piiConfig = useModel('appConfigs', 'pii'); + const { selectedAppId, piiConfig } = useSelector((state) => state.discussions); const appConfig = useModel('appConfigs', selectedAppId); const app = useModel('apps', selectedAppId); const providerName = intl.formatMessage(appMessages[`appName-${app?.id}`]); 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/openedx/OpenedXConfigForm.jsx similarity index 69% rename from src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx rename to src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx index 71335f774a..5bb417908c 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.jsx @@ -1,41 +1,57 @@ -import React, { useState } from 'react'; -import PropTypes from 'prop-types'; +import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { Card, Form } from '@edx/paragon'; import { Formik } from 'formik'; -import * as Yup from 'yup'; -import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; +import PropTypes from 'prop-types'; +import React, { useState } from 'react'; import { useSelector } from 'react-redux'; -import DivisionByGroupFields from '../shared/DivisionByGroupFields'; -import AnonymousPostingFields from '../shared/AnonymousPostingFields'; -import DiscussionTopics from '../shared/discussion-topics/DiscussionTopics'; -import BlackoutDatesField from '../shared/BlackoutDatesField'; -import LegacyConfigFormProvider from './LegacyConfigFormProvider'; -import AppConfigFormDivider from '../shared/AppConfigFormDivider'; -import { checkFieldErrors } from '../../utils'; -import { setupYupExtensions } from '../../../../../utils'; +import * as Yup from 'yup'; import { useModel, useModels } from '../../../../../generic/model-store'; +import { setupYupExtensions } from '../../../../../utils'; import messages from '../../messages'; +import { checkFieldErrors } from '../../utils'; +import AnonymousPostingFields from '../shared/AnonymousPostingFields'; +import AppConfigFormDivider from '../shared/AppConfigFormDivider'; +import BlackoutDatesField from '../shared/BlackoutDatesField'; +import DiscussionTopics from '../shared/discussion-topics/DiscussionTopics'; +import DivisionByGroupFields from '../shared/DivisionByGroupFields'; +import InContextDiscussionFields from '../shared/InContextDiscussionFields'; +import OpenedXConfigFormProvider from './OpenedXConfigFormProvider'; setupYupExtensions(); -function LegacyConfigForm({ onSubmit, formRef, intl }) { - const { discussionTopicIds, divideDiscussionIds, selectedAppId } = useSelector((state) => state.discussions); +function OpenedXConfigForm({ + onSubmit, formRef, intl, legacy, +}) { + const { + selectedAppId, enableInContext, enableGradedUnits, unitLevelVisibility, discussionTopicIds, divideDiscussionIds, + } = useSelector(state => state.discussions); const appConfigObj = useModel('appConfigs', selectedAppId); const discussionTopicsModel = useModels('discussionTopics', discussionTopicIds); - const appConfig = { ...appConfigObj, discussionTopics: discussionTopicsModel, divideDiscussionIds }; - const LegacyAppConfig = { - ...appConfig, - allowAnonymousPosts: appConfig.allowAnonymousPosts || false, - allowAnonymousPostsPeers: appConfig.allowAnonymousPostsPeers || false, - blackoutDates: appConfig.blackoutDates || [], - discussionTopics: appConfig.discussionTopics || [], - divideByCohorts: appConfig.divideByCohorts || false, - divideCourseTopicsByCohorts: appConfig.divideCourseTopicsByCohorts || false, + const legacyAppConfig = { + ...(appConfigObj || {}), + divideDiscussionIds, + enableInContext, + enableGradedUnits, + unitLevelVisibility, + allowAnonymousPosts: appConfigObj?.allowAnonymousPosts || false, + allowAnonymousPostsPeers: appConfigObj?.allowAnonymousPostsPeers || false, + blackoutDates: appConfigObj?.blackoutDates || [], + discussionTopics: discussionTopicsModel || [], + divideByCohorts: appConfigObj?.divideByCohorts || false, + divideCourseTopicsByCohorts: appConfigObj?.divideCourseTopicsByCohorts || false, + groupAtSubsection: appConfigObj?.groupAtSubsection || false, }; - const [validDiscussionTopics, setValidDiscussionTopics] = useState(appConfig.discussionTopics); - const legacyFormValidationSchema = Yup.object().shape({ + const [validDiscussionTopics, setValidDiscussionTopics] = useState(discussionTopicsModel); + // These fields are only used for the new provider and aren't supported for legacy. + const additionalFields = legacy ? {} : { + enableInContext: Yup.bool().default(true), + enabledGradedUnits: Yup.bool().default(false), + groupAtSubsection: Yup.bool().default(false), + unitLevelVisibility: Yup.bool().default(false), + }; + const validationSchema = Yup.object().shape({ blackoutDates: Yup.array( Yup.object().shape({ startDate: Yup.string() @@ -65,13 +81,14 @@ function LegacyConfigForm({ onSubmit, formRef, intl }) { name: Yup.string().required(intl.formatMessage(messages.discussionTopicRequired)), }).uniqueObjectProperty('name', intl.formatMessage(messages.discussionTopicNameAlreadyExist)), ), + ...additionalFields, }); return ( onSubmit(values)} > {({ @@ -97,11 +114,18 @@ function LegacyConfigForm({ onSubmit, formRef, intl }) { }; return ( - +

{intl.formatMessage(messages[`appName-${selectedAppId}`])}

+ {!legacy + && ( + <> + + + + )}
- + ); }} ); } -LegacyConfigForm.propTypes = { +OpenedXConfigForm.propTypes = { + legacy: PropTypes.bool.isRequired, onSubmit: PropTypes.func.isRequired, // eslint-disable-next-line react/forbid-prop-types formRef: PropTypes.object.isRequired, intl: intlShape.isRequired, }; -export default injectIntl(LegacyConfigForm); +export default injectIntl(OpenedXConfigForm); 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/openedx/OpenedXConfigForm.test.jsx similarity index 87% rename from src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx rename to src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigForm.test.jsx index 8816729ae4..fd102e6191 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/openedx/OpenedXConfigForm.test.jsx @@ -2,7 +2,8 @@ import React, { createRef } from 'react'; import { act, - fireEvent, queryAllByText, + fireEvent, + queryAllByText, queryByLabelText, queryByRole, queryByTestId, @@ -20,11 +21,11 @@ import { AppProvider } from '@edx/frontend-platform/react'; import initializeStore from '../../../../../store'; import { executeThunk } from '../../../../../utils'; -import { getAppsUrl } from '../../../data/api'; -import { fetchApps } from '../../../data/thunks'; -import { legacyApiResponse } from '../../../factories/mockApiResponses'; +import { getDiscussionsProvidersUrl, getDiscussionsSettingsUrl } from '../../../data/api'; +import { fetchDiscussionSettings, fetchProviders } from '../../../data/thunks'; +import { generateProvidersApiResponse, legacyApiResponse } from '../../../factories/mockApiResponses'; import messages from '../../messages'; -import LegacyConfigForm from './LegacyConfigForm'; +import OpenedXConfigForm from './OpenedXConfigForm'; import { selectApp } from '../../../data/slice'; import { DivisionSchemes } from '../../../../../data/constants'; @@ -39,12 +40,16 @@ const defaultAppConfig = (divideDiscussionIds = []) => ({ { name: 'General', id: 'course' }, ], divideDiscussionIds, + enableGradedUnits: undefined, + enableInContext: undefined, + groupAtSubsection: false, + unitLevelVisibility: undefined, allowAnonymousPosts: false, allowAnonymousPostsPeers: false, allowDivisionByUnit: false, blackoutDates: [], }); -describe('LegacyConfigForm', () => { +describe('OpenedXConfigForm', () => { let axiosMock; let store; let container; @@ -66,13 +71,14 @@ describe('LegacyConfigForm', () => { axiosMock.reset(); }); - const createComponent = (onSubmit = jest.fn(), formRef = createRef()) => { + const createComponent = (onSubmit = jest.fn(), formRef = createRef(), legacy = true) => { const wrapper = render( - , @@ -82,8 +88,10 @@ describe('LegacyConfigForm', () => { }; const mockStore = async (mockResponse) => { - axiosMock.onGet(getAppsUrl(courseId)).reply(200, mockResponse); - await executeThunk(fetchApps(courseId), store.dispatch); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, generateProvidersApiResponse()); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, mockResponse); + await executeThunk(fetchProviders(courseId), store.dispatch); + await executeThunk(fetchDiscussionSettings(courseId), store.dispatch); store.dispatch(selectApp({ appId: 'legacy' })); }; @@ -93,6 +101,15 @@ describe('LegacyConfigForm', () => { expect(container.querySelector('h3')).toHaveTextContent('edX'); }); + test('new Open edX provider config', async () => { + await mockStore({ ...legacyApiResponse, enable_in_context: true }); + createComponent(jest.fn(), createRef(), false); + expect(queryByText(container, messages.visibilityInContext.defaultMessage)).toBeInTheDocument(); + expect(queryByText(container, messages.gradedUnitPagesLabel.defaultMessage)).toBeInTheDocument(); + expect(queryByText(container, messages.groupInContextSubsectionLabel.defaultMessage)).toBeInTheDocument(); + expect(queryByText(container, messages.allowUnitLevelVisibilityLabel.defaultMessage)).toBeInTheDocument(); + }); + test('calls onSubmit when the formRef is submitted', async () => { const formRef = createRef(); const handleSubmit = jest.fn(); diff --git a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigFormProvider.jsx b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigFormProvider.jsx similarity index 75% rename from src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigFormProvider.jsx rename to src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigFormProvider.jsx index 2d0fcd66e7..d5a1c8792f 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigFormProvider.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/openedx/OpenedXConfigFormProvider.jsx @@ -3,9 +3,9 @@ import PropTypes from 'prop-types'; import { useDispatch } from 'react-redux'; import { updateValidationStatus } from '../../../data/slice'; -export const LegacyConfigFormContext = createContext({}); +export const OpenedXConfigFormContext = createContext({}); -export default function LegacyConfigFormProvider({ children, value }) { +export default function OpenedXConfigFormProvider({ children, value }) { const dispatch = useDispatch(); useEffect(() => { @@ -13,13 +13,13 @@ export default function LegacyConfigFormProvider({ children, value }) { }, [value.isFormInvalid]); return ( - + {children} - + ); } -LegacyConfigFormProvider.propTypes = { +OpenedXConfigFormProvider.propTypes = { children: PropTypes.node.isRequired, value: PropTypes.shape({ discussionTopicErrors: PropTypes.arrayOf(PropTypes.bool), diff --git a/src/pages-and-resources/discussions/app-config-form/apps/openedx/index.js b/src/pages-and-resources/discussions/app-config-form/apps/openedx/index.js new file mode 100644 index 0000000000..d0140aae7a --- /dev/null +++ b/src/pages-and-resources/discussions/app-config-form/apps/openedx/index.js @@ -0,0 +1 @@ +export { default } from './OpenedXConfigForm'; 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 7809e70694..fdbf57bdd0 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 @@ -6,10 +6,10 @@ import _ from 'lodash'; import FormSwitchGroup from '../../../../../generic/FormSwitchGroup'; import messages from '../../messages'; import AppConfigFormDivider from './AppConfigFormDivider'; -import { LegacyConfigFormContext } from '../legacy/LegacyConfigFormProvider'; +import { OpenedXConfigFormContext } from '../openedx/OpenedXConfigFormProvider'; const DivisionByGroupFields = ({ intl }) => { - const { validDiscussionTopics } = useContext(LegacyConfigFormContext); + const { validDiscussionTopics } = useContext(OpenedXConfigFormContext); const { handleChange, handleBlur, diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx index 0190c09f83..c2a9228a8f 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/InContextDiscussionFields.jsx @@ -18,21 +18,21 @@ function InContextDiscussionFields({ - {values.inContextDiscussion ? ( + {values.enableInContext ? ( @@ -41,8 +41,8 @@ function InContextDiscussionFields({ onChange={onChange} onBlur={onBlur} className="ml-4" - id="groupInContextSubsection" - checked={values.groupInContextSubsection} + id="groupAtSubsection" + checked={values.groupAtSubsection} label={intl.formatMessage(messages.groupInContextSubsectionLabel)} helpText={intl.formatMessage(messages.groupInContextSubsectionHelp)} /> @@ -51,8 +51,8 @@ function InContextDiscussionFields({ onChange={onChange} onBlur={onBlur} className="ml-4" - id="allowUnitLevelVisibility" - checked={values.allowUnitLevelVisibility} + id="unitLevelVisibility" + checked={values.unitLevelVisibility} label={intl.formatMessage(messages.allowUnitLevelVisibilityLabel)} helpText={intl.formatMessage(messages.allowUnitLevelVisibilityHelp)} /> @@ -69,10 +69,10 @@ InContextDiscussionFields.propTypes = { onChange: PropTypes.func.isRequired, intl: intlShape.isRequired, values: PropTypes.shape({ - inContextDiscussion: PropTypes.bool, - gradedUnitPages: PropTypes.bool, - groupInContextSubsection: PropTypes.bool, - allowUnitLevelVisibility: PropTypes.bool, + enableInContext: PropTypes.bool, + enableGradedUnits: PropTypes.bool, + groupAtSubsection: PropTypes.bool, + unitLevelVisibility: PropTypes.bool, }).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 943ae00567..661c83998b 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 @@ -7,7 +7,7 @@ import { v4 as uuid } from 'uuid'; import _ from 'lodash'; import messages from '../../../messages'; import TopicItem from './TopicItem'; -import { LegacyConfigFormContext } from '../../legacy/LegacyConfigFormProvider'; +import { OpenedXConfigFormContext } from '../../openedx/OpenedXConfigFormProvider'; import { filterItemFromObject } from '../../../utils'; const DiscussionTopics = ({ intl }) => { @@ -21,7 +21,7 @@ const DiscussionTopics = ({ intl }) => { discussionTopicErrors, validDiscussionTopics, setValidDiscussionTopics, - } = useContext(LegacyConfigFormContext); + } = useContext(OpenedXConfigFormContext); const handleTopicDelete = async (topicIndex, topicId, remove) => { await remove(topicIndex); diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/DiscussionTopics.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/DiscussionTopics.test.jsx index 548e052dde..d25638e6fa 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/DiscussionTopics.test.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/DiscussionTopics.test.jsx @@ -14,10 +14,10 @@ import { AppProvider } from '@edx/frontend-platform/react'; import initializeStore from '../../../../../../store'; import { executeThunk } from '../../../../../../utils'; -import { getAppsUrl } from '../../../../data/api'; -import { fetchApps } from '../../../../data/thunks'; +import { getDiscussionsProvidersUrl } from '../../../../data/api'; +import { fetchProviders } from '../../../../data/thunks'; import { legacyApiResponse } from '../../../../factories/mockApiResponses'; -import LegacyConfigFormProvider from '../../legacy/LegacyConfigFormProvider'; +import OpenedXConfigFormProvider from '../../openedx/OpenedXConfigFormProvider'; import messages from '../../../messages'; import DiscussionTopics from './DiscussionTopics'; @@ -73,11 +73,11 @@ describe('DiscussionTopics', () => { const wrapper = render( - + - + , ); @@ -85,8 +85,8 @@ describe('DiscussionTopics', () => { }; const mockStore = async (mockResponse) => { - axiosMock.onGet(getAppsUrl(courseId)).reply(200, mockResponse); - await executeThunk(fetchApps(courseId), store.dispatch); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, mockResponse); + await executeThunk(fetchProviders(courseId), store.dispatch); }; test('renders each discussion topic correctly', async () => { diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/TopicItem.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/TopicItem.test.jsx index 55355f07fa..46e122205e 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/TopicItem.test.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/discussion-topics/TopicItem.test.jsx @@ -20,8 +20,8 @@ import { AppProvider } from '@edx/frontend-platform/react'; import initializeStore from '../../../../../../store'; import { executeThunk } from '../../../../../../utils'; -import { getAppsUrl } from '../../../../data/api'; -import { fetchApps } from '../../../../data/thunks'; +import { getDiscussionsProvidersUrl } from '../../../../data/api'; +import { fetchProviders } from '../../../../data/thunks'; import { legacyApiResponse } from '../../../../factories/mockApiResponses'; import messages from '../../../messages'; import TopicItem from './TopicItem'; @@ -91,8 +91,8 @@ describe('TopicItem', () => { }; const mockStore = async (mockResponse) => { - axiosMock.onGet(getAppsUrl(courseId)).reply(200, mockResponse); - await executeThunk(fetchApps(courseId), store.dispatch); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, mockResponse); + await executeThunk(fetchProviders(courseId), store.dispatch); }; test('displays a collapsible card for discussion topic', async () => { diff --git a/src/pages-and-resources/discussions/app-config-form/messages.js b/src/pages-and-resources/discussions/app-config-form/messages.js index 1d303a0b0e..a065a51e79 100644 --- a/src/pages-and-resources/discussions/app-config-form/messages.js +++ b/src/pages-and-resources/discussions/app-config-form/messages.js @@ -82,9 +82,14 @@ const messages = defineMessages({ }, 'appName-legacy': { id: 'authoring.discussions.appConfigForm.appName-legacy', - defaultMessage: 'edX', + defaultMessage: 'edX (Legacy)', description: 'The name of the Legacy edX Discussions app.', }, + 'appName-openedx': { + id: 'authoring.discussions.appConfigForm.appName-openedx', + defaultMessage: 'edX (NEW)', + description: 'The name of the new edX Discussions app.', + }, divisionByGroup: { id: 'authoring.discussions.builtIn.divisionByGroup', defaultMessage: 'Cohorts', diff --git a/src/pages-and-resources/discussions/app-list/AppCard.test.jsx b/src/pages-and-resources/discussions/app-list/AppCard.test.jsx index 5a4391d454..5280f8420d 100644 --- a/src/pages-and-resources/discussions/app-list/AppCard.test.jsx +++ b/src/pages-and-resources/discussions/app-list/AppCard.test.jsx @@ -11,8 +11,8 @@ import messages from './messages'; import appMessages from '../app-config-form/messages'; import initializeStore from '../../../store'; import { executeThunk } from '../../../utils'; -import { getAppsUrl } from '../data/api'; -import { fetchApps } from '../data/thunks'; +import { getDiscussionsProvidersUrl } from '../data/api'; +import { fetchProviders } from '../data/thunks'; import { legacyApiResponse } from '../factories/mockApiResponses'; const courseId = 'course-v1:edX+TestX+Test_Course'; @@ -43,8 +43,8 @@ describe('AppCard', () => { }); const mockStore = async (mockResponse) => { - axiosMock.onGet(getAppsUrl(courseId)).reply(200, mockResponse); - await executeThunk(fetchApps(courseId), store.dispatch); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, mockResponse); + await executeThunk(fetchProviders(courseId), store.dispatch); }; const createComponent = (data) => { diff --git a/src/pages-and-resources/discussions/app-list/AppList.test.jsx b/src/pages-and-resources/discussions/app-list/AppList.test.jsx index 7ff124d606..abd15fcc90 100644 --- a/src/pages-and-resources/discussions/app-list/AppList.test.jsx +++ b/src/pages-and-resources/discussions/app-list/AppList.test.jsx @@ -14,9 +14,12 @@ import { Context as ResponsiveContext } from 'react-responsive'; import initializeStore from '../../../store'; import { executeThunk } from '../../../utils'; -import { getAppsUrl } from '../data/api'; -import { fetchApps } from '../data/thunks'; -import { emptyAppApiResponse, piazzaApiResponse } from '../factories/mockApiResponses'; +import { getDiscussionsProvidersUrl, getDiscussionsSettingsUrl } from '../data/api'; +import { fetchDiscussionSettings, fetchProviders } from '../data/thunks'; +import { + generateProvidersApiResponse, + piazzaApiResponse, +} from '../factories/mockApiResponses'; import AppList from './AppList'; import messages from './messages'; @@ -54,23 +57,15 @@ describe('AppList', () => { }); const mockStore = async (mockResponse, screenWidth = breakpoints.extraLarge.minWidth) => { - axiosMock.onGet(getAppsUrl(courseId)).reply(200, mockResponse); - await executeThunk(fetchApps(courseId), store.dispatch); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, generateProvidersApiResponse()); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, mockResponse); + await executeThunk(fetchProviders(courseId), store.dispatch); + await executeThunk(fetchDiscussionSettings(courseId), store.dispatch); const component = createComponent(screenWidth); const wrapper = render(component); container = wrapper.container; }; - test('displays a message when there are no apps available', async () => { - await mockStore(emptyAppApiResponse); - expect(queryByText(container, `${messages.noApps.defaultMessage}`)).toBeInTheDocument(); - }); - - test('displays loading state when there is no active App', async () => { - await mockStore({}); - expect(queryByRole(container, 'status')).toBeInTheDocument(); - }); - test('display a card for each available app', async () => { await mockStore(piazzaApiResponse); const appCount = store.getState().discussions.appIds.length; diff --git a/src/pages-and-resources/discussions/app-list/messages.js b/src/pages-and-resources/discussions/app-list/messages.js index 92747ce3c3..e45da318ac 100644 --- a/src/pages-and-resources/discussions/app-list/messages.js +++ b/src/pages-and-resources/discussions/app-list/messages.js @@ -45,11 +45,34 @@ const messages = defineMessages({ description: 'A label for the checkbox that allows a user to select the discussions app they want to configure.', }, + // Legacy + 'appName-legacy': { + id: 'authoring.discussions.appList.appName-legacy', + defaultMessage: 'edX (Legacy)', + description: 'The name of the Legacy edX Discussions app.', + }, 'appDescription-legacy': { id: 'authoring.discussions.appList.appDescription-legacy', defaultMessage: 'Start conversations with other learners, ask questions, and interact with other learners in the course.', description: 'A description of the Legacy edX Discussions app.', }, + // New provider + 'appName-openedx': { + id: 'authoring.discussions.appList.appName-openedx', + defaultMessage: 'edX', + description: 'The name of the new edX Discussions app.', + }, + 'appDescription-openedx': { + id: 'authoring.discussions.appList.appDescription-openedx', + defaultMessage: 'Start conversations with other learners, ask questions, and interact with other learners in the course.', + description: 'A description of the new edX Discussions app.', + }, + // Piazza + 'appName-piazza': { + id: 'authoring.discussions.appList.appName-piazza', + defaultMessage: 'Piazza', + description: 'The name of the Piazza app.', + }, 'appDescription-piazza': { id: 'authoring.discussions.appList.appDescription-piazza', defaultMessage: 'Piazza is designed to connect students, TAs, and professors so every student can get the help they need when they need it.', diff --git a/src/pages-and-resources/discussions/data/api.js b/src/pages-and-resources/discussions/data/api.js index 9fa7360881..f99f3fc4e9 100644 --- a/src/pages-and-resources/discussions/data/api.js +++ b/src/pages-and-resources/discussions/data/api.js @@ -1,20 +1,19 @@ -import { ensureConfig, getConfig, camelCaseObject } from '@edx/frontend-platform'; +import { camelCaseObject, ensureConfig, getConfig } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import _ from 'lodash'; import { v4 as uuid } from 'uuid'; +import { DivisionSchemes } from '../../../data/constants'; import { checkStatus, - sortBlackoutDatesByStatus, + endOfDayTime, + getTime, mergeDateTime, normalizeDate, normalizeTime, - getTime, + sortBlackoutDatesByStatus, startOfDayTime, - endOfDayTime, } from '../app-config-form/utils'; import { blackoutDatesStatus as constants } from './constants'; -import { DivisionSchemes } from '../../../data/constants'; ensureConfig([ 'STUDIO_BASE_URL', @@ -80,23 +79,11 @@ function normalizePiiSharing(data) { } function normalizeAppConfig(data) { - let ltiConfig = {}; - const legacyConfig = { - id: 'legacy', + return { + id: data.provider_type, ...normalizePluginConfig(data.plugin_configuration), + ...normalizeLtiConfig(data.lti_configuration), }; - const piiConfig = { - id: 'pii', - ...normalizePiiSharing(data.lti_configuration), - }; - if (data.providers.active !== 'legacy') { - ltiConfig = { - id: data.providers.active, - ...normalizeLtiConfig(data.lti_configuration), - }; - } - if (!_.isEmpty(ltiConfig)) { return [legacyConfig, ltiConfig, piiConfig]; } - return [legacyConfig, piiConfig]; } function normalizeDiscussionTopic(data) { @@ -123,8 +110,8 @@ function normalizeFeatures(data, apps) { ); } -function normalizeApps(data) { - const apps = Object.entries(data.providers.available).map(([key, app]) => ({ +function normalizeProviders(data) { + const apps = Object.entries(data.available).map(([key, app]) => ({ id: key, messages: app.messages, featureIds: app.features, @@ -139,12 +126,20 @@ function normalizeApps(data) { adminOnlyConfig: !!app.admin_only_config, })); return { - courseId: data.context_key, - enabled: data.enabled, features: normalizeFeatures(data.features, apps), - appConfigs: normalizeAppConfig(data), - activeAppId: data.providers.active, + activeAppId: data.active, apps, + }; +} + +function normalizeSettings(data) { + return { + enabled: data.enabled, + enableInContext: data.enable_in_context, + enableGradedUnits: data.enable_graded_units, + unitLevelVisibility: data.unit_level_visibility, + appConfig: normalizeAppConfig(data), + piiConfig: normalizePiiSharing(data.lti_configuration), discussionTopicIds: data.plugin_configuration.discussion_topics ? extractDiscussionTopicIds(data.plugin_configuration.discussion_topics) : [], @@ -181,6 +176,9 @@ function denormalizeData(courseId, appId, data) { pluginConfiguration.division_scheme = data.divideByCohorts ? DivisionSchemes.COHORT : DivisionSchemes.NONE; pluginConfiguration.always_divide_inline_discussions = data.divideByCohorts; } + if ('groupAtSubsection' in data) { + pluginConfiguration.group_at_subsection = data.groupAtSubsection; + } if (data.blackoutDates?.length) { pluginConfiguration.discussion_blackouts = data.blackoutDates.map((blackoutDates) => ( denormalizeBlackoutDate(blackoutDates) @@ -224,31 +222,57 @@ function denormalizeData(courseId, appId, data) { ltiConfiguration.version = 'lti_1p1'; } - return { + const apiData = { context_key: courseId, enabled: true, lti_configuration: ltiConfiguration, plugin_configuration: pluginConfiguration, provider_type: appId, }; + if ('enableInContext' in data) { + apiData.enable_in_context = data.enableInContext; + } + if ('enableGradedUnits' in data) { + apiData.enable_graded_units = data.enableGradedUnits; + } + if ('unitLevelVisibility' in data) { + apiData.unit_level_visibility = data.unitLevelVisibility; + } + return apiData; } -export function getAppsUrl(courseId) { - return `${getConfig().STUDIO_BASE_URL}/api/discussions/v0/${courseId}`; +export function getDiscussionsProvidersUrl(courseId) { + return `${getConfig().STUDIO_BASE_URL}/api/discussions/v0/course/${courseId}/providers`; } -export async function getApps(courseId) { +export function getDiscussionsSettingsUrl(courseId) { + return `${getConfig().STUDIO_BASE_URL}/api/discussions/v0/course/${courseId}/settings`; +} + +export async function getDiscussionsProviders(courseId) { + const { data } = await getAuthenticatedHttpClient() + .get(getDiscussionsProvidersUrl(courseId)); + + return normalizeProviders(data); +} + +export async function getDiscussionsSettings(courseId, providerId = null) { + const params = {}; + if (providerId) { + params.params = { provider_id: providerId }; + } + const url = getDiscussionsSettingsUrl(courseId); const { data } = await getAuthenticatedHttpClient() - .get(getAppsUrl(courseId)); + .get(url, params); - return normalizeApps(data); + return normalizeSettings(data); } -export async function postAppConfig(courseId, appId, values) { +export async function postDiscussionsSettings(courseId, appId, values) { const { data } = await getAuthenticatedHttpClient().post( - getAppsUrl(courseId), + getDiscussionsSettingsUrl(courseId), denormalizeData(courseId, appId, values), ); - return normalizeApps(data); + return normalizeSettings(data); } diff --git a/src/pages-and-resources/discussions/data/redux.test.js b/src/pages-and-resources/discussions/data/redux.test.js index f02b801e11..5d70e609bc 100644 --- a/src/pages-and-resources/discussions/data/redux.test.js +++ b/src/pages-and-resources/discussions/data/redux.test.js @@ -1,17 +1,17 @@ +import { history } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import MockAdapter from 'axios-mock-adapter'; import { initializeMockApp } from '@edx/frontend-platform/testing'; -import { history } from '@edx/frontend-platform'; +import MockAdapter from 'axios-mock-adapter'; +import { DivisionSchemes } from '../../../data/constants'; +import { LOADED } from '../../../data/slice'; import initializeStore from '../../../store'; -import { getAppsUrl } from './api'; +import { executeThunk } from '../../../utils'; +import { generateProvidersApiResponse, legacyApiResponse, piazzaApiResponse } from '../factories/mockApiResponses'; +import { getDiscussionsProvidersUrl, getDiscussionsSettingsUrl } from './api'; import { - FAILED, SAVED, DENIED, selectApp, updateValidationStatus, + DENIED, FAILED, SAVED, selectApp, updateValidationStatus, } from './slice'; -import { fetchApps, saveAppConfig } from './thunks'; -import { LOADED } from '../../../data/slice'; -import { legacyApiResponse, piazzaApiResponse } from '../factories/mockApiResponses'; -import { executeThunk } from '../../../utils'; -import { DivisionSchemes } from '../../../data/constants'; +import { fetchDiscussionSettings, fetchProviders, saveProviderConfig } from './thunks'; const courseId = 'course-v1:edX+TestX+Test_Course'; const pagesAndResourcesPath = `/course/${courseId}/pages-and-resources`; @@ -116,11 +116,11 @@ describe('Data layer integration tests', () => { axiosMock.reset(); }); - describe('fetchApps', () => { + describe('fetchProviders', () => { test('network error', async () => { - axiosMock.onGet(getAppsUrl(courseId)).networkError(); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).networkError(); - await executeThunk(fetchApps(courseId), store.dispatch); + await executeThunk(fetchProviders(courseId), store.dispatch); expect(store.getState().discussions).toEqual( expect.objectContaining({ @@ -136,9 +136,9 @@ describe('Data layer integration tests', () => { }); test('permission denied error', async () => { - axiosMock.onGet(getAppsUrl(courseId)).reply(403); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(403); - await executeThunk(fetchApps(courseId), store.dispatch); + await executeThunk(fetchProviders(courseId), store.dispatch); expect(store.getState().discussions).toEqual( expect.objectContaining({ @@ -154,11 +154,13 @@ describe('Data layer integration tests', () => { }); test('successfully loads an LTI configuration', async () => { - axiosMock.onGet(getAppsUrl(courseId)).reply(200, piazzaApiResponse); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, generateProvidersApiResponse()); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, piazzaApiResponse); - await executeThunk(fetchApps(courseId), store.dispatch); + await executeThunk(fetchProviders(courseId), store.dispatch); + await executeThunk(fetchDiscussionSettings(courseId), store.dispatch); - expect(store.getState().discussions).toEqual({ + expect(store.getState().discussions).toEqual(expect.objectContaining({ appIds: ['legacy', 'piazza', 'discourse'], featureIds, activeAppId: 'piazza', @@ -167,7 +169,7 @@ describe('Data layer integration tests', () => { saveStatus: SAVED, hasValidationError: false, discussionTopicIds: [], - }); + })); expect(store.getState().models.apps.legacy).toEqual(legacyApp); expect(store.getState().models.apps.piazza).toEqual(piazzaApp); expect(store.getState().models.features).toEqual(featuresState); @@ -180,7 +182,8 @@ describe('Data layer integration tests', () => { }); test('successfully loads an LTI configuration with PII Sharing', async () => { - axiosMock.onGet(getAppsUrl(courseId)).reply(200, { + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, generateProvidersApiResponse()); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, { ...piazzaApiResponse, lti_configuration: { ...piazzaApiResponse.lti_configuration, @@ -190,9 +193,10 @@ describe('Data layer integration tests', () => { }, }); - await executeThunk(fetchApps(courseId), store.dispatch); + await executeThunk(fetchProviders(courseId), store.dispatch); + await executeThunk(fetchDiscussionSettings(courseId), store.dispatch); - expect(store.getState().discussions).toEqual({ + expect(store.getState().discussions).toEqual(expect.objectContaining({ appIds: ['legacy', 'piazza', 'discourse'], featureIds, activeAppId: 'piazza', @@ -201,7 +205,7 @@ describe('Data layer integration tests', () => { saveStatus: SAVED, hasValidationError: false, discussionTopicIds: [], - }); + })); expect(store.getState().models.apps.legacy).toEqual(legacyApp); expect(store.getState().models.apps.piazza).toEqual(piazzaApp); expect(store.getState().models.features).toEqual(featuresState); @@ -214,12 +218,14 @@ describe('Data layer integration tests', () => { }); test('successfully loads a Legacy configuration', async () => { - axiosMock.onGet(getAppsUrl(courseId)).reply(200, legacyApiResponse); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, generateProvidersApiResponse(false, 'legacy')); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, legacyApiResponse); - await executeThunk(fetchApps(courseId), store.dispatch); + await executeThunk(fetchProviders(courseId), store.dispatch); + await executeThunk(fetchDiscussionSettings(courseId), store.dispatch); - expect(store.getState().discussions).toEqual({ - appIds: ['legacy', 'piazza'], + expect(store.getState().discussions).toEqual(expect.objectContaining({ + appIds: ['legacy', 'piazza', 'discourse'], featureIds, activeAppId: 'legacy', selectedAppId: null, @@ -228,7 +234,7 @@ describe('Data layer integration tests', () => { hasValidationError: false, discussionTopicIds, divideDiscussionIds: [], - }); + })); expect(store.getState().models.apps.legacy).toEqual(legacyApp); expect(store.getState().models.apps.piazza).toEqual(piazzaApp); expect(store.getState().models.features).toEqual(featuresState); @@ -269,13 +275,15 @@ describe('Data layer integration tests', () => { test('network error', async () => { history.push(`/course/${courseId}/pages-and-resources/discussions/configure/piazza`); - axiosMock.onGet(getAppsUrl(courseId)).reply(200, piazzaApiResponse); - axiosMock.onPost(getAppsUrl(courseId)).networkError(); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, generateProvidersApiResponse()); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, piazzaApiResponse); + axiosMock.onPost(getDiscussionsSettingsUrl(courseId)).networkError(); - // We call fetchApps and selectApp here too just to get us into a real state. - await executeThunk(fetchApps(courseId), store.dispatch); + // We call fetchProviders and selectApp here too just to get us into a real state. + await executeThunk(fetchProviders(courseId), store.dispatch); + await executeThunk(fetchDiscussionSettings(courseId), store.dispatch); store.dispatch(selectApp({ appId: 'piazza' })); - await executeThunk(saveAppConfig(courseId, 'piazza', {}, pagesAndResourcesPath), store.dispatch); + await executeThunk(saveProviderConfig(courseId, 'piazza', {}, pagesAndResourcesPath), store.dispatch); // Assert we're still on the form. expect(window.location.pathname).toEqual(`/course/${courseId}/pages-and-resources/discussions/configure/piazza`); @@ -295,13 +303,14 @@ describe('Data layer integration tests', () => { test('permission denied error', async () => { history.push(`/course/${courseId}/pages-and-resources/discussions/configure/piazza`); - axiosMock.onGet(getAppsUrl(courseId)).reply(200, piazzaApiResponse); - axiosMock.onPost(getAppsUrl(courseId)).reply(403); + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, generateProvidersApiResponse()); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, piazzaApiResponse); + axiosMock.onPost(getDiscussionsSettingsUrl(courseId)).reply(403); - // We call fetchApps and selectApp here too just to get us into a real state. - await executeThunk(fetchApps(courseId), store.dispatch); + // We call fetchProviders and selectApp here too just to get us into a real state. + await executeThunk(fetchProviders(courseId), store.dispatch); store.dispatch(selectApp({ appId: 'piazza' })); - await executeThunk(saveAppConfig(courseId, 'piazza', {}, pagesAndResourcesPath), store.dispatch); + await executeThunk(saveProviderConfig(courseId, 'piazza', {}, pagesAndResourcesPath), store.dispatch); // Assert we're still on the form. expect(window.location.pathname).toEqual(`/course/${courseId}/pages-and-resources/discussions/configure/piazza`); @@ -311,7 +320,7 @@ describe('Data layer integration tests', () => { featureIds, activeAppId: 'piazza', selectedAppId: 'piazza', - status: DENIED, // We set BOTH statuses to DENIED for saveAppConfig - this removes the UI. + status: DENIED, // We set BOTH statuses to DENIED for saveProviderConfig - this removes the UI. saveStatus: DENIED, hasValidationError: false, }), @@ -321,8 +330,9 @@ describe('Data layer integration tests', () => { test('successfully saves an LTI configuration', async () => { history.push(`/course/${courseId}/pages-and-resources/discussions/configure/piazza`); - axiosMock.onGet(getAppsUrl(courseId)).reply(200, piazzaApiResponse); - axiosMock.onPost(getAppsUrl(courseId), { + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, generateProvidersApiResponse()); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, piazzaApiResponse); + axiosMock.onPost(getDiscussionsSettingsUrl(courseId), { context_key: courseId, enabled: true, lti_configuration: { @@ -344,10 +354,10 @@ describe('Data layer integration tests', () => { }, }); - // We call fetchApps and selectApp here too just to get us into a real state. - await executeThunk(fetchApps(courseId), store.dispatch); + // We call fetchProviders and selectApp here too just to get us into a real state. + await executeThunk(fetchProviders(courseId), store.dispatch); store.dispatch(selectApp({ appId: 'piazza' })); - await executeThunk(saveAppConfig( + await executeThunk(saveProviderConfig( courseId, 'piazza', { @@ -381,8 +391,9 @@ describe('Data layer integration tests', () => { test('successfully saves a Legacy configuration', async () => { history.push(`/course/${courseId}/pages-and-resources/discussions/configure/legacy`); - axiosMock.onGet(getAppsUrl(courseId)).reply(200, legacyApiResponse); - axiosMock.onPost(getAppsUrl(courseId), { + axiosMock.onGet(getDiscussionsProvidersUrl(courseId)).reply(200, generateProvidersApiResponse(false, 'legacy')); + axiosMock.onGet(getDiscussionsSettingsUrl(courseId)).reply(200, legacyApiResponse); + axiosMock.onPost(getDiscussionsSettingsUrl(courseId), { context_key: courseId, enabled: true, lti_configuration: {}, @@ -422,10 +433,11 @@ describe('Data layer integration tests', () => { }, }); - // We call fetchApps and selectApp here too just to get us into a real state. - await executeThunk(fetchApps(courseId), store.dispatch); + // We call fetchProviders and selectApp here too just to get us into a real state. + await executeThunk(fetchProviders(courseId), store.dispatch); + await executeThunk(fetchDiscussionSettings(courseId), store.dispatch); store.dispatch(selectApp({ appId: 'legacy' })); - await executeThunk(saveAppConfig( + await executeThunk(saveProviderConfig( courseId, 'legacy', { @@ -450,7 +462,7 @@ describe('Data layer integration tests', () => { expect(window.location.pathname).toEqual(pagesAndResourcesPath); expect(store.getState().discussions).toEqual( expect.objectContaining({ - appIds: ['legacy', 'piazza'], + appIds: ['legacy', 'piazza', 'discourse'], featureIds, activeAppId: 'legacy', selectedAppId: 'legacy', diff --git a/src/pages-and-resources/discussions/data/slice.js b/src/pages-and-resources/discussions/data/slice.js index 1d79b06ed8..174b31e12e 100644 --- a/src/pages-and-resources/discussions/data/slice.js +++ b/src/pages-and-resources/discussions/data/slice.js @@ -25,16 +25,15 @@ const slice = createSlice({ hasValidationError: false, discussionTopicIds: [], divideDiscussionIds: [], + enableInContext: false, + enableGradedUnits: false, + unitLevelVisibility: false, }, reducers: { loadApps: (state, { payload }) => { - state.activeAppId = payload.activeAppId; - state.appIds = payload.appIds; - state.featureIds = payload.featureIds; state.status = LOADED; state.saveStatus = SAVED; - state.discussionTopicIds = payload.discussionTopicIds; - state.divideDiscussionIds = payload.divideDiscussionIds; + Object.assign(state, payload); }, selectApp: (state, { payload }) => { const { appId } = payload; diff --git a/src/pages-and-resources/discussions/data/thunks.js b/src/pages-and-resources/discussions/data/thunks.js index a155a17c17..660403a280 100644 --- a/src/pages-and-resources/discussions/data/thunks.js +++ b/src/pages-and-resources/discussions/data/thunks.js @@ -1,52 +1,63 @@ import { history } from '@edx/frontend-platform'; -import { addModels } from '../../../generic/model-store'; -import { getApps, postAppConfig } from './api'; +import { addModel, addModels } from '../../../generic/model-store'; +import { getDiscussionsProviders, getDiscussionsSettings, postDiscussionsSettings } from './api'; import { - FAILED, - loadApps, - LOADING, - SAVED, - SAVING, - updateStatus, - updateSaveStatus, - DENIED, + DENIED, FAILED, loadApps, LOADING, SAVED, SAVING, updateSaveStatus, updateStatus, } from './slice'; -function updateAppState({ +function updateDiscussionSettingsState({ + appConfig, + discussionTopics, + ...discussionSettings +}) { + return async (dispatch) => { + dispatch(addModel({ modelType: 'appConfigs', model: appConfig })); + dispatch(addModels({ modelType: 'discussionTopics', models: discussionTopics })); + dispatch(loadApps(discussionSettings)); + }; +} + +function updateProviderState({ apps, features, activeAppId, - appConfigs, - discussionTopicIds, - discussionTopics, - divideDiscussionIds, - userPermissions, }) { return async (dispatch) => { dispatch(addModels({ modelType: 'apps', models: apps })); dispatch(addModels({ modelType: 'features', models: features })); - dispatch(addModels({ modelType: 'appConfigs', models: appConfigs })); - dispatch(addModels({ modelType: 'discussionTopics', models: discussionTopics })); dispatch( loadApps({ activeAppId, appIds: apps.map((app) => app.id), featureIds: features.map((feature) => feature.id), - discussionTopicIds, - divideDiscussionIds, - userPermissions, }), ); }; } -export function fetchApps(courseId) { +export function fetchProviders(courseId) { + return async (dispatch) => { + dispatch(updateStatus({ status: LOADING })); + try { + const apps = await getDiscussionsProviders(courseId); + dispatch(updateProviderState(apps)); + } catch (error) { + if (error.response && error.response.status === 403) { + dispatch(updateStatus({ status: DENIED })); + } else { + dispatch(updateStatus({ status: FAILED })); + } + } + }; +} + +export function fetchDiscussionSettings(courseId, providerId = null) { return async (dispatch) => { dispatch(updateStatus({ status: LOADING })); try { - const apps = await getApps(courseId); - dispatch(updateAppState(apps)); + const apps = await getDiscussionsSettings(courseId, providerId); + dispatch(updateDiscussionSettingsState(apps)); } catch (error) { if (error.response && error.response.status === 403) { dispatch(updateStatus({ status: DENIED })); @@ -57,13 +68,13 @@ export function fetchApps(courseId) { }; } -export function saveAppConfig(courseId, appId, drafts, successPath) { +export function saveProviderConfig(courseId, appId, drafts, successPath) { return async (dispatch) => { dispatch(updateSaveStatus({ status: SAVING })); try { - const apps = await postAppConfig(courseId, appId, drafts); - dispatch(updateAppState(apps)); + const apps = await postDiscussionsSettings(courseId, appId, drafts); + dispatch(updateDiscussionSettingsState(apps)); dispatch(updateSaveStatus({ status: SAVED })); // Note that we redirect here to avoid having to work with the promise over in AppConfigForm. diff --git a/src/pages-and-resources/discussions/factories/mockApiResponses.js b/src/pages-and-resources/discussions/factories/mockApiResponses.js index 71ad23c2b8..0d165b6674 100644 --- a/src/pages-and-resources/discussions/factories/mockApiResponses.js +++ b/src/pages-and-resources/discussions/factories/mockApiResponses.js @@ -1,15 +1,9 @@ import { DivisionSchemes } from '../../../data/constants'; -export const generatePiazzaApiResponse = (piazzaAdminOnlyConfig = false, piiSharingAllowed = false) => ({ +export const generatePiazzaApiResponse = (piiSharingAllowed = false) => ({ context_key: 'course-v1:edX+DemoX+Demo_Course', enabled: true, provider_type: 'piazza', - features: [ - { id: 'discussion-page', feature_support_type: 'basic' }, - { id: 'embedded-course-sections', feature_support_type: 'full' }, - { id: 'wcag-2.1', feature_support_type: 'partial' }, - { id: 'basic-configuration', feature_support_type: 'common' }, - ], lti_configuration: { lti_1p1_client_key: 'client_key_123', lti_1p1_client_secret: 'client_secret_123', @@ -20,63 +14,70 @@ export const generatePiazzaApiResponse = (piazzaAdminOnlyConfig = false, piiShar version: 'lti_1p1', }, plugin_configuration: {}, - providers: { - active: 'piazza', - available: { - legacy: { - features: [ - 'discussion-page', - 'embedded-course-sections', - 'wcag-2.1', - ], - external_links: { - learn_more: '', - configuration: '', - general: '', - accessibility: '', - contact_email: '', - }, - messages: [], - has_full_support: true, - admin_only_config: false, +}); + +export const generateProvidersApiResponse = (piazzaAdminOnlyConfig = false, activeProvider = 'piazza') => ({ + active: activeProvider, + features: [ + { id: 'discussion-page', feature_support_type: 'basic' }, + { id: 'embedded-course-sections', feature_support_type: 'full' }, + { id: 'wcag-2.1', feature_support_type: 'partial' }, + { id: 'basic-configuration', feature_support_type: 'common' }, + ], + available: { + legacy: { + features: [ + 'discussion-page', + 'embedded-course-sections', + 'wcag-2.1', + ], + external_links: { + learn_more: '', + configuration: '', + general: '', + accessibility: '', + contact_email: '', }, - piazza: { - features: [ - // We give piazza all features just so we can test our "full support" text. - 'discussion-page', - 'embedded-course-sections', - 'wcag-2.1', - 'basic-configuration', - ], - external_links: { - learn_more: '', - configuration: '', - general: '', - accessibility: '', - contact_email: '', - }, - messages: [], - has_full_support: false, - admin_only_config: piazzaAdminOnlyConfig, + messages: [], + has_full_support: true, + admin_only_config: false, + }, + piazza: { + features: [ + // We give piazza all features just so we can test our "full support" text. + 'discussion-page', + 'embedded-course-sections', + 'wcag-2.1', + 'basic-configuration', + ], + external_links: { + learn_more: '', + configuration: '', + general: '', + accessibility: '', + contact_email: '', }, - discourse: { - features: [ - 'discussion-page', - 'embedded-course-sections', - 'wcag-2.1', - 'lti', - ], - external_links: { - learn_more: '', - configuration: '', - general: '', - accessibility: '', - contact_email: '', - }, - messages: [], - has_full_support: false, - admin_only_config: false, + messages: [], + has_full_support: false, + admin_only_config: piazzaAdminOnlyConfig, + }, + discourse: { + features: [ + 'discussion-page', + 'embedded-course-sections', + 'wcag-2.1', + 'lti', + ], + external_links: { + learn_more: '', + configuration: '', + general: '', + accessibility: '', + contact_email: '', }, + messages: [], + has_full_support: false, + admin_only_config: false, }, }, }); @@ -85,12 +86,6 @@ export const generateLegacyApiResponse = () => ({ context_key: 'course-v1:edX+DemoX+Demo_Course', enabled: true, provider_type: 'legacy', - features: [ - { id: 'discussion-page', feature_support_type: 'basic' }, - { id: 'embedded-course-sections', feature_support_type: 'full' }, - { id: 'wcag-2.1', feature_support_type: 'partial' }, - { id: 'basic-configuration', feature_support_type: 'common' }, - ], lti_configuration: {}, plugin_configuration: { allow_anonymous: false, @@ -125,7 +120,6 @@ export const generateLegacyApiResponse = () => ({ }, messages: [], has_full_support: true, - admin_only_config: false, }, piazza: { features: [ @@ -144,7 +138,6 @@ export const generateLegacyApiResponse = () => ({ }, messages: [], has_full_support: false, - admin_only_config: false, }, }, }, @@ -165,7 +158,7 @@ export const emptyAppApiResponse = { }, }; -export const piazzaApiResponse = generatePiazzaApiResponse(false); +export const piazzaApiResponse = generatePiazzaApiResponse(); export const courseDetailResponse = { blocks_url: 'http://localhost:18000/api/courses/v2/blocks/?course_id=course-v1%3AedX%2BDemoX%2BDemo_Course',