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