diff --git a/src/generic/Loading.jsx b/src/generic/Loading.jsx new file mode 100644 index 000000000..2e12d2b18 --- /dev/null +++ b/src/generic/Loading.jsx @@ -0,0 +1,25 @@ +import React from 'react'; +import { Spinner } from '@edx/paragon'; +import { FormattedMessage } from '@edx/frontend-platform/i18n'; + +export default function Loading() { + return ( +
+ + + + + +
+ ); +} diff --git a/src/pages-and-resources/discussions/DiscussionsSettings.jsx b/src/pages-and-resources/discussions/DiscussionsSettings.jsx index e2c11c9a6..aa51f5d09 100644 --- a/src/pages-and-resources/discussions/DiscussionsSettings.jsx +++ b/src/pages-and-resources/discussions/DiscussionsSettings.jsx @@ -1,5 +1,5 @@ import React, { - useCallback, useContext, + useCallback, useContext, useEffect, } from 'react'; import PropTypes from 'prop-types'; import { @@ -7,6 +7,7 @@ import { useLocation, useRouteMatch, } from 'react-router'; +import { useDispatch } from 'react-redux'; import { history } from '@edx/frontend-platform'; import { PageRoute } from '@edx/frontend-platform/react'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; @@ -16,18 +17,24 @@ import { Check } from '@edx/paragon/icons'; import FullScreenModal from '../../generic/full-screen-modal'; import Stepper from '../../generic/stepper'; import { PagesAndResourcesContext } from '../PagesAndResourcesProvider'; -import AppList from './app-list'; -import AppConfigForm from './app-config-form'; + import messages from './messages'; import DiscussionsProvider from './DiscussionsProvider'; +import { fetchApps } from './data/thunks'; +import AppList from './app-list'; +import AppConfigForm from './app-config-form'; function DiscussionsSettings({ courseId, intl }) { + const dispatch = useDispatch(); const { path } = useRouteMatch(); const { pathname } = useLocation(); - const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext); - const discussionsPath = `${pagesAndResourcesPath}/discussions`; + useEffect(() => { + dispatch(fetchApps(courseId)); + }, [courseId]); + + const discussionsPath = `${pagesAndResourcesPath}/discussions`; const isFirstStep = pathname === discussionsPath; const steps = [{ @@ -65,9 +72,7 @@ function DiscussionsSettings({ courseId, intl }) { - + { - dispatch(fetchAppConfig(courseId, routeAppId)); - }, [courseId]); + const { formRef } = useContext(AppConfigFormContext); + const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext); + const { params: { appId: routeAppId } } = useRouteMatch(); + const { selectedAppId, status } = useSelector(state => state.discussions); + const app = useModel('apps', selectedAppId); + // appConfigs have no ID of their own, so we use the active app ID to reference them. + // This appConfig may come back as null if the selectedAppId is not the activeAppId, i.e., + // if we're configuring a new app. + const appConfig = useModel('appConfigs', selectedAppId); - const { appId, appConfigId } = useSelector(state => state.discussions.appConfigForm); + useEffect(() => { + if (status === LOADED) { + if (routeAppId !== selectedAppId) { + dispatch(selectApp({ appId: routeAppId })); + } + } + }, [selectedAppId, routeAppId, status]); // This is a callback that gets called after the form has been submitted successfully. const handleSubmit = useCallback((values) => { - dispatch(saveAppConfig(courseId, appId, values)).then(() => { + dispatch(saveAppConfig(courseId, selectedAppId, values)).then(() => { history.push(pagesAndResourcesPath); }); - }, [courseId, appId, courseId]); + }, [courseId, selectedAppId, courseId]); - const app = useModel('apps', appId); - const appConfig = useModel('appConfigs', appConfigId); - - if (!appConfig || !app) { + if (!selectedAppId || status !== LOADED) { return null; } let form = null; - if (app.id === 'legacy') { form = ( state.discussions.appConfigForm.status); + const saveStatus = useSelector(state => state.discussions.saveStatus); const { formRef } = useContext(AppConfigFormContext); - const submitButtonState = status === SAVING ? 'pending' : 'default'; + const submitButtonState = saveStatus === SAVING ? 'pending' : 'default'; // This causes the form to be submitted from a button outside the form. const handleApply = useCallback(() => { diff --git a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx index c9911cf93..b75df9a44 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.jsx @@ -70,7 +70,7 @@ LegacyConfigForm.propTypes = { allowAnonymousPosts: PropTypes.bool.isRequired, allowAnonymousPostsPeers: PropTypes.bool.isRequired, blackoutDates: PropTypes.string.isRequired, - }).isRequired, + }), onSubmit: PropTypes.func.isRequired, // eslint-disable-next-line react/forbid-prop-types formRef: PropTypes.object.isRequired, @@ -78,4 +78,17 @@ LegacyConfigForm.propTypes = { title: PropTypes.string.isRequired, }; +LegacyConfigForm.defaultProps = { + appConfig: { + divideByCohorts: false, + allowDivisionByUnit: false, + divideCourseWideTopics: false, + divideGeneralTopic: false, + divideQuestionsForTAs: false, + allowAnonymousPosts: false, + allowAnonymousPostsPeers: false, + blackoutDates: '[]', + }, +}; + export default injectIntl(LegacyConfigForm); diff --git a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx index f0fea3a8b..79d61bcc8 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/legacy/LegacyConfigForm.test.jsx @@ -16,6 +16,20 @@ const defaultAppConfig = { }; describe('LegacyConfigForm', () => { + test('title rendering', () => { + const { container } = render( + + + , + ); + + expect(container.querySelector('h3')).toHaveTextContent('Test Legacy edX Discussions'); + }); test('calls onSubmit when the formRef is submitted', async () => { const formRef = createRef(); const handleSubmit = jest.fn(); @@ -23,6 +37,7 @@ describe('LegacyConfigForm', () => { render( { const { container } = render( { const { container } = render(
@@ -44,47 +49,46 @@ function LtiConfigForm({ target="_blank" rel="noopener noreferrer" > - {intl.formatMessage(messages.documentationPage, { name: app.name })} + {intl.formatMessage(messages.documentationPage, { name: title })} ), - name: app.name, }} />

- + {intl.formatMessage(messages.consumerKey)} - {errors.consumerKey && ( + {isInvalidConsumerKey && ( {errors.consumerKey} )} - + {intl.formatMessage(messages.consumerSecret)} - {errors.consumerSecret && ( + {isInvalidConsumerSecret && ( {errors.consumerSecret} )} - + {intl.formatMessage(messages.launchUrl)} - {errors.launchUrl && ( + {isInvalidLaunchUrl && ( {errors.launchUrl} @@ -98,14 +102,13 @@ function LtiConfigForm({ LtiConfigForm.propTypes = { app: PropTypes.shape({ id: PropTypes.string.isRequired, - name: PropTypes.string.isRequired, documentationUrl: PropTypes.string.isRequired, }).isRequired, appConfig: PropTypes.shape({ consumerKey: PropTypes.string.isRequired, consumerSecret: PropTypes.string.isRequired, launchUrl: PropTypes.string.isRequired, - }).isRequired, + }), intl: intlShape.isRequired, onSubmit: PropTypes.func.isRequired, // eslint-disable-next-line react/forbid-prop-types @@ -113,4 +116,12 @@ LtiConfigForm.propTypes = { title: PropTypes.string.isRequired, }; +LtiConfigForm.defaultProps = { + appConfig: { + consumerKey: '', + consumerSecret: '', + launchUrl: '', + }, +}; + export default injectIntl(LtiConfigForm); diff --git a/src/pages-and-resources/discussions/app-config-form/data/api.js b/src/pages-and-resources/discussions/app-config-form/data/api.js deleted file mode 100644 index 37c42c297..000000000 --- a/src/pages-and-resources/discussions/app-config-form/data/api.js +++ /dev/null @@ -1,143 +0,0 @@ -const legacyEdXDiscussions = { - id: 'legacy', - hasFullSupport: false, - featureIds: [ - 'discussion-page', - 'embedded-course-sections', - 'wcag-2.1', - ], -}; - -const piazzaApp = { - id: 'piazza', - hasFullSupport: false, - featureIds: [ - 'lti', - 'discussion-page', - 'embedded-course-sections', - 'wcag-2.1', - ], -}; - -const yellowdigApp = { - id: 'yellowdig', - hasFullSupport: false, - featureIds: [ - 'lti', - 'discussion-page', - 'embedded-course-sections', - 'wcag-2.1', - ], -}; - -export function getAppConfig(courseId, appConfigId, appId) { - let app = null; - switch (appId) { - case 'piazza': - app = piazzaApp; - break; - case 'yellowdig': - app = yellowdigApp; - break; - default: - app = legacyEdXDiscussions; - } - - let appConfig = { - id: 'appConfig1', - consumerSecret: 'its-a-secret-to-everybody', - consumerKey: 'abc123', - launchUrl: 'https://localhost/launch', - }; - - if (appId === 'legacy') { - appConfig = { - id: 'legacy', - divideByCohorts: false, - allowDivisionByUnit: false, - divideCourseWideTopics: false, - divideGeneralTopic: false, - divideQuestionsForTAs: false, - inContextDiscussion: false, - gradedUnitPages: false, - groupInContextSubsection: false, - allowUnitLevelVisibility: false, - allowAnonymousPosts: false, - allowAnonymousPostsPeers: false, - blackoutDates: '[]', - }; - } - - return Promise.resolve({ - app, - appConfig, - features: [ - { - id: 'lti', - }, - { - id: 'discussion-page', - }, - { - id: 'embedded-course-sections', - }, - { - id: 'embedded-course-units', - }, - { - id: 'wcag-2.1', - }, - ], - }); -} - -export function postAppConfig(courseId, appId, drafts) { - let app = null; - switch (appId) { - case 'piazza': - app = piazzaApp; - break; - case 'yellowdig': - app = yellowdigApp; - break; - default: - app = legacyEdXDiscussions; - } - return new Promise((resolve) => { - setTimeout(() => { - resolve({ - app, - appConfig: { - id: 'appConfig1', - consumerSecret: 'its-a-secret-to-everybody', - consumerKey: 'abc123', - launchUrl: 'https://localhost/launch', - documentationUrl: 'https://localhost/docs', - ...drafts, - }, - features: [ - { - id: 'lti', - name: 'LTI Integration', - }, - { - id: 'discussion-page', - name: 'Discussion Page', - }, - { - id: 'embedded-course-sections', - name: 'Embedded Course Sections', - }, - { - id: 'embedded-course-units', - name: 'Embedded Course Units', - }, - { - id: 'wcag-2.1', - name: 'WCAG 2.1 Support', - }, - ], - }); - }, 1000); - }); -} diff --git a/src/pages-and-resources/discussions/app-config-form/data/slice.js b/src/pages-and-resources/discussions/app-config-form/data/slice.js deleted file mode 100644 index b8871afd7..000000000 --- a/src/pages-and-resources/discussions/app-config-form/data/slice.js +++ /dev/null @@ -1,39 +0,0 @@ -/* eslint-disable no-param-reassign */ -import { createSlice } from '@reduxjs/toolkit'; - -export const LOADING = 'LOADING'; -export const LOADED = 'LOADED'; -export const FAILED = 'FAILED'; -export const SAVING = 'SAVING'; -export const SAVED = 'SAVED'; -export const DIRTY = 'DIRTY'; - -const slice = createSlice({ - name: 'appConfigForm', - initialState: { - // app config state - appId: null, - appConfigId: null, - appConfigStatus: LOADING, - }, - reducers: { - loadAppConfig: (state, { payload }) => { - state.appId = payload.appId; - state.appConfigId = payload.appConfigId; - state.featureIds = payload.featureIds; - state.status = LOADED; - }, - updateStatus: (state, { status }) => { - state.status = status; - }, - }, -}); - -export const { - loadAppConfig, - updateStatus, -} = slice.actions; - -export const { - reducer, -} = slice; diff --git a/src/pages-and-resources/discussions/app-config-form/data/thunks.js b/src/pages-and-resources/discussions/app-config-form/data/thunks.js deleted file mode 100644 index ac317de8a..000000000 --- a/src/pages-and-resources/discussions/app-config-form/data/thunks.js +++ /dev/null @@ -1,60 +0,0 @@ -import { addModel, addModels } from '../../../../generic/model-store'; -import { - getAppConfig, - postAppConfig, -} from './api'; -import { - FAILED, - LOADING, - updateStatus, - SAVING, - SAVED, - loadAppConfig, -} from './slice'; - -export function fetchAppConfig(courseId, appId, appConfigId) { - return async (dispatch) => { - dispatch(updateStatus({ status: LOADING })); - try { - const { app, appConfig, features } = await getAppConfig(courseId, appConfigId, appId); - - dispatch(addModel({ modelType: 'apps', model: app })); - dispatch(addModels({ modelType: 'features', models: features })); - dispatch(addModel({ modelType: 'appConfigs', model: appConfig })); - - dispatch(loadAppConfig({ - appId: app.id, - appConfigId: appConfig.id, - featureIds: features.map(feature => feature.id), - })); - } catch (error) { - // TODO: We need generic error handling in the app for when a request just fails... in other - // parts of the app (proctored exam settings) we show a nice message and ask the user to - // reload/try again later. - dispatch(updateStatus({ status: FAILED })); - } - }; -} - -export function saveAppConfig(courseId, appId, drafts) { - return async (dispatch) => { - dispatch(updateStatus({ status: SAVING })); - - try { - const { app, appConfig, features } = await postAppConfig(courseId, appId, drafts); - - dispatch(addModel({ modelType: 'apps', model: app })); - dispatch(addModels({ modelType: 'features', models: features })); - dispatch(addModel({ modelType: 'appConfigs', model: appConfig })); - - dispatch(loadAppConfig({ - activeAppId: app.id, - activeAppConfigId: appConfig.id, - featureIds: features.map(feature => feature.id), - })); - dispatch(updateStatus({ status: SAVED })); - } catch (error) { - dispatch(updateStatus({ status: FAILED })); - } - }; -} diff --git a/src/pages-and-resources/discussions/app-config-form/index.js b/src/pages-and-resources/discussions/app-config-form/index.js index de8b7964e..971895f30 100644 --- a/src/pages-and-resources/discussions/app-config-form/index.js +++ b/src/pages-and-resources/discussions/app-config-form/index.js @@ -1,2 +1 @@ export { default } from './AppConfigForm'; -export { reducer } from './data/slice'; diff --git a/src/pages-and-resources/discussions/app-list/AppList.jsx b/src/pages-and-resources/discussions/app-list/AppList.jsx index a55e6a1f3..18471b5c1 100644 --- a/src/pages-and-resources/discussions/app-list/AppList.jsx +++ b/src/pages-and-resources/discussions/app-list/AppList.jsx @@ -1,28 +1,23 @@ -import React, { useCallback, useEffect } from 'react'; -import PropTypes from 'prop-types'; +import React, { useCallback } from 'react'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { CardGrid, Container } from '@edx/paragon'; import { useDispatch, useSelector } from 'react-redux'; import { useModels } from '../../../generic/model-store'; +import { selectApp, LOADED, LOADING } from '../data/slice'; import AppCard from './AppCard'; import messages from './messages'; import FeaturesTable from './FeaturesTable'; import AppListNextButton from './AppListNextButton'; +import Loading from '../../../generic/Loading'; -import { fetchApps } from './data/thunks'; -import { selectApp, LOADED } from './data/slice'; - -function AppList({ courseId, intl }) { +function AppList({ intl }) { const dispatch = useDispatch(); - useEffect(() => { - dispatch(fetchApps(courseId)); - }, [courseId]); const { - appIds, featureIds, status, selectedAppId, - } = useSelector(state => state.discussions.appList); + appIds, featureIds, status, activeAppId, selectedAppId, + } = useSelector(state => state.discussions); const apps = useModels('apps', appIds); const features = useModels('features', featureIds); @@ -30,6 +25,15 @@ function AppList({ courseId, intl }) { dispatch(selectApp({ appId })); }, [selectedAppId]); + // If selectedAppId is not set, use activeAppId + const finalSelectedAppId = selectedAppId || activeAppId; + + if (status === LOADING) { + return ( + + ); + } + if (status === LOADED && apps.length === 0) { return ( @@ -54,7 +58,7 @@ function AppList({ courseId, intl }) { ))} @@ -73,7 +77,6 @@ function AppList({ courseId, intl }) { } AppList.propTypes = { - courseId: PropTypes.string.isRequired, intl: intlShape.isRequired, }; diff --git a/src/pages-and-resources/discussions/app-list/AppListNextButton.jsx b/src/pages-and-resources/discussions/app-list/AppListNextButton.jsx index 0a98e1b69..c10434839 100644 --- a/src/pages-and-resources/discussions/app-list/AppListNextButton.jsx +++ b/src/pages-and-resources/discussions/app-list/AppListNextButton.jsx @@ -9,7 +9,7 @@ import { DiscussionsContext } from '../DiscussionsProvider'; import messages from './messages'; function AppListNextButton({ intl }) { - const { selectedAppId } = useSelector(state => state.discussions.appList); + const { selectedAppId } = useSelector(state => state.discussions); const { path: discussionsPath } = useContext(DiscussionsContext); const handleStartConfig = useCallback(() => { diff --git a/src/pages-and-resources/discussions/app-list/data/api.js b/src/pages-and-resources/discussions/app-list/data/api.js deleted file mode 100644 index 435fcbd69..000000000 --- a/src/pages-and-resources/discussions/app-list/data/api.js +++ /dev/null @@ -1,34 +0,0 @@ -import { getConfig } from '@edx/frontend-platform'; -import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; - -function normalizeApps(data) { - const apps = Object.entries(data.providers.available).map(([key, app]) => ({ - id: key, - featureIds: app.features, - hasFullSupport: app.features.length >= data.features.length, - })); - return { - courseId: data.context_key, - enabled: data.enabled, - features: data.features.map(id => ({ - id, - })), - appConfig: data.plugin_configuration, - // TODO: Defaulting to "legacy" should come from the backend. For now this makes sense as we - // don't create a DiscussionsConfiguration object in the backend for legacy discussions, which - // is configured by default on all courses. The endpoint doesn't know this and returns a null - // for `active`, which we can then interpret as "the user hasn't configured something else" and - // choose to fall back to legacy. In the long run, we should be able to trust `active` to give - // us 'legacy' if the legacy experience is being used. - activeAppId: data.providers.active || 'legacy', - apps, - }; -} - -// eslint-disable-next-line import/prefer-default-export -export async function getApps(courseId) { - const { data } = await getAuthenticatedHttpClient() - .get(`${getConfig().LMS_BASE_URL}/discussions/api/v0/${courseId}`); - - return normalizeApps(data); -} diff --git a/src/pages-and-resources/discussions/app-list/data/thunks.js b/src/pages-and-resources/discussions/app-list/data/thunks.js deleted file mode 100644 index a55958a1c..000000000 --- a/src/pages-and-resources/discussions/app-list/data/thunks.js +++ /dev/null @@ -1,29 +0,0 @@ -import { addModels } from '../../../../generic/model-store'; - -import { getApps } from './api'; -import { - updateStatus, LOADING, FAILED, loadApps, -} from './slice'; - -/* eslint-disable import/prefer-default-export */ -export function fetchApps(courseId) { - return async (dispatch) => { - dispatch(updateStatus({ status: LOADING })); - try { - const { apps, features, activeAppId } = await getApps(courseId); - - dispatch(addModels({ modelType: 'apps', models: apps })); - dispatch(addModels({ modelType: 'features', models: features })); - dispatch(loadApps({ - activeAppId, - appIds: apps.map(app => app.id), - featureIds: features.map(feature => feature.id), - })); - } catch (error) { - // TODO: We need generic error handling in the app for when a request just fails... in other - // parts of the app (proctored exam settings) we show a nice message and ask the user to - // reload/try again later. - dispatch(updateStatus({ status: FAILED })); - } - }; -} diff --git a/src/pages-and-resources/discussions/app-list/index.js b/src/pages-and-resources/discussions/app-list/index.js index 1038107ed..a303ce01c 100644 --- a/src/pages-and-resources/discussions/app-list/index.js +++ b/src/pages-and-resources/discussions/app-list/index.js @@ -1,2 +1 @@ export { default } from './AppList'; -export { reducer } from './data/slice'; diff --git a/src/pages-and-resources/discussions/data/api.js b/src/pages-and-resources/discussions/data/api.js new file mode 100644 index 000000000..27fa255e8 --- /dev/null +++ b/src/pages-and-resources/discussions/data/api.js @@ -0,0 +1,130 @@ +import { getConfig } from '@edx/frontend-platform'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; + +function normalizeLtiConfig(data) { + if (!data || Object.keys(data).length < 1) { + return {}; + } + + return { + consumerKey: data.lti_1p1_client_key, + consumerSecret: data.lti_1p1_client_secret, + launchUrl: data.lti_1p1_launch_url, + }; +} + +function normalizePluginConfig(data) { + if (!data || Object.keys(data).length < 1) { + return {}; + } + + return { + allowAnonymousPosts: data.allow_anonymous, + allowAnonymousPostsPeers: data.allow_anonymous_to_peers, + blackoutDates: JSON.stringify(data.discussion_blackouts), + // TODO: We need all these added to the API. ... default them for now + divideByCohorts: false, + allowDivisionByUnit: false, + divideCourseWideTopics: false, + // TODO: Note that these last two are in the `discussion_topics` data, but we haven't been able + // to add them properly here yet. I'm not sure the data is in a usable state as is, since it + // only seems to include the topics for which these would be "true". Assuming its only these + // two, I think we could check for the existence of "General" for the first, but I'm not sure + // what the topic title is for the second. "Questions for TAs" maybe? + divideGeneralTopic: false, + divideQuestionsForTAs: false, + }; +} + +function normalizeApps(data) { + const apps = Object.entries(data.providers.available).map(([key, app]) => ({ + id: key, + featureIds: app.features, + // TODO: Fix this and get it from the backend! + documentationUrl: 'http://example.com', + hasFullSupport: app.features.length >= data.features.length, + })); + return { + courseId: data.context_key, + enabled: data.enabled, + features: data.features.map(id => ({ + id, + })), + appConfig: { + id: data.providers.active, + ...normalizePluginConfig(data.plugin_configuration), + ...normalizeLtiConfig(data.lti_configuration), + }, + activeAppId: data.providers.active, + apps, + }; +} + +function denormalizeData(courseId, appId, data) { + /* + TODO: What about these? Some are from the instructor dashboard, presumably... but I don't think they all are. + + divideByCohorts + allowDivisionByUnit + divideCourseWideTopics + divideGeneralTopic + divideQuestionsForTAs + */ + const pluginConfiguration = {}; + + if (data.allowAnonymousPosts) { + pluginConfiguration.allow_anonymous = data.allowAnonymousPosts; + } + if (data.allowAnonymousPostsPeers) { + pluginConfiguration.allow_anonymous_to_peers = data.allowAnonymousPostsPeers; + } + if (data.blackoutDates) { + pluginConfiguration.discussion_blackouts = JSON.parse(data.blackoutDates); + } + + const ltiConfiguration = {}; + + if (data.consumerKey) { + ltiConfiguration.lti_1p1_client_key = data.consumerKey; + } + if (data.consumerSecret) { + ltiConfiguration.lti_1p1_client_secret = data.consumerSecret; + } + if (data.launchUrl) { + ltiConfiguration.lti_1p1_launch_url = data.launchUrl; + } + + if (Object.keys(ltiConfiguration).length > 0) { + // Only add this in if we're sending LTI fields. + // TODO: Eventually support LTI v1.3 here. + ltiConfiguration.version = 'lti_1p1'; + } + + return { + context_key: courseId, + enabled: true, + lti_configuration: ltiConfiguration, + plugin_configuration: pluginConfiguration, + provider_type: appId, + }; +} + +export function getAppsUrl(courseId) { + return `${getConfig().LMS_BASE_URL}/discussions/api/v0/${courseId}`; +} + +export async function getApps(courseId) { + const { data } = await getAuthenticatedHttpClient() + .get(getAppsUrl(courseId)); + + return normalizeApps(data); +} + +export async function postAppConfig(courseId, appId, values) { + const { data } = await getAuthenticatedHttpClient().post( + getAppsUrl(courseId), + denormalizeData(courseId, appId, values), + ); + + return normalizeApps(data); +} diff --git a/src/pages-and-resources/discussions/data/reducer.js b/src/pages-and-resources/discussions/data/reducer.js deleted file mode 100644 index 64ae7b9c3..000000000 --- a/src/pages-and-resources/discussions/data/reducer.js +++ /dev/null @@ -1,11 +0,0 @@ -import { combineReducers } from 'redux'; - -import { reducer as appListReducer } from '../app-list'; -import { reducer as appConfigFormReducer } from '../app-config-form'; - -const reducer = combineReducers({ - appList: appListReducer, - appConfigForm: appConfigFormReducer, -}); - -export default reducer; diff --git a/src/pages-and-resources/discussions/data/redux.test.js b/src/pages-and-resources/discussions/data/redux.test.js new file mode 100644 index 000000000..83b95034c --- /dev/null +++ b/src/pages-and-resources/discussions/data/redux.test.js @@ -0,0 +1,380 @@ +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import MockAdapter from 'axios-mock-adapter'; +import { initializeMockApp } from '@edx/frontend-platform/testing'; +import initializeStore from '../../../store'; +import { getAppsUrl } from './api'; +import { FAILED, SAVED, selectApp } from './slice'; +import { fetchApps, saveAppConfig } from './thunks'; +import { LOADED } from '../../../data/slice'; + +let axiosMock; +let store; + +// Helper, that is used to forcibly finalize all promises +// in thunk before running matcher against state. +const executeThunk = async (thunk, dispatch, getState) => { + await thunk(dispatch, getState); + await new Promise(setImmediate); +}; + +const courseId = 'course-v1:edX+TestX+Test_Course'; + +const piazzaApiResponse = { + context_key: 'course-v1:edX+DemoX+Demo_Course', + enabled: true, + provider_type: 'piazza', + features: [ + 'discussion-page', + 'embedded-course-sections', + 'wcag-2.1', + 'lti', + ], + lti_configuration: { + lti_1p1_client_key: 'client_key_123', + lti_1p1_client_secret: 'client_secret_123', + lti_1p1_launch_url: 'https://localhost/example', + version: 'lti_1p1', + }, + plugin_configuration: {}, + providers: { + active: 'piazza', + available: { + legacy: { + features: [ + 'discussion-page', + 'embedded-course-sections', + 'wcag-2.1', + ], + }, + piazza: { + features: [ + 'discussion-page', + 'lti', + ], + }, + }, + }, +}; + +const legacyApiResponse = { + context_key: 'course-v1:edX+DemoX+Demo_Course', + enabled: true, + provider_type: 'legacy', + features: [ + 'discussion-page', + 'embedded-course-sections', + 'wcag-2.1', + 'lti', + ], + lti_configuration: {}, + plugin_configuration: { + allow_anonymous: false, + allow_anonymous_to_peers: false, + // Note, this gets stringified when normalized into the app, but the API returns it as an + // actual array. Argh. + discussion_blackouts: [], + }, + providers: { + active: 'legacy', + available: { + legacy: { + features: [ + 'discussion-page', + 'embedded-course-sections', + 'wcag-2.1', + ], + }, + piazza: { + features: [ + 'discussion-page', + 'lti', + ], + }, + }, + }, +}; + +const featuresState = { + 'discussion-page': { + id: 'discussion-page', + }, + 'embedded-course-sections': { + id: 'embedded-course-sections', + }, + 'wcag-2.1': { + id: 'wcag-2.1', + }, + lti: { + id: 'lti', + }, +}; + +const featureIds = [ + 'discussion-page', + 'embedded-course-sections', + 'wcag-2.1', + 'lti', +]; + +const legacyApp = { + id: 'legacy', + featureIds: [ + 'discussion-page', + 'embedded-course-sections', + 'wcag-2.1', + ], + documentationUrl: 'http://example.com', + hasFullSupport: false, +}; + +const piazzaApp = { + id: 'piazza', + featureIds: [ + 'discussion-page', + 'lti', + ], + documentationUrl: 'http://example.com', + hasFullSupport: false, +}; + +describe('Data layer integration tests', () => { + beforeEach(() => { + initializeMockApp({ + authenticatedUser: { + userId: 3, + username: 'abc123', + administrator: true, + roles: [], + }, + }); + + axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + + store = initializeStore(); + }); + + afterEach(() => { + axiosMock.reset(); + }); + + describe('fetchApps', () => { + test('network error', async () => { + axiosMock.onGet(getAppsUrl(courseId)).networkError(); + + await executeThunk(fetchApps(courseId), store.dispatch); + + expect(store.getState().discussions).toEqual( + expect.objectContaining({ + appIds: [], + featureIds: [], + activeAppId: null, + selectedAppId: null, + status: FAILED, + saveStatus: SAVED, + }), + ); + }); + + test('successfully loads an LTI configuration', async () => { + axiosMock.onGet(getAppsUrl(courseId)).reply(200, piazzaApiResponse); + + await executeThunk(fetchApps(courseId), store.dispatch); + + expect(store.getState().discussions).toEqual({ + appIds: ['legacy', 'piazza'], + featureIds, + activeAppId: 'piazza', + selectedAppId: null, + status: LOADED, + saveStatus: SAVED, + }); + expect(store.getState().models.apps.legacy).toEqual(legacyApp); + expect(store.getState().models.apps.piazza).toEqual(piazzaApp); + expect(store.getState().models.features).toEqual(featuresState); + expect(store.getState().models.appConfigs.piazza).toEqual({ + id: 'piazza', + consumerKey: 'client_key_123', + consumerSecret: 'client_secret_123', + launchUrl: 'https://localhost/example', + }); + }); + + test('successfully loads a Legacy configuration', async () => { + axiosMock.onGet(getAppsUrl(courseId)).reply(200, legacyApiResponse); + + await executeThunk(fetchApps(courseId), store.dispatch); + + expect(store.getState().discussions).toEqual({ + appIds: ['legacy', 'piazza'], + featureIds, + activeAppId: 'legacy', + selectedAppId: null, + status: LOADED, + saveStatus: SAVED, + }); + expect(store.getState().models.apps.legacy).toEqual(legacyApp); + expect(store.getState().models.apps.piazza).toEqual(piazzaApp); + expect(store.getState().models.features).toEqual(featuresState); + expect(store.getState().models.appConfigs.legacy).toEqual({ + id: 'legacy', + allowAnonymousPosts: false, + allowAnonymousPostsPeers: false, + blackoutDates: '[]', + // TODO: Note! As of this writing, all the data below this line is NOT returned in the API + // but we add it in during normalization. + divideByCohorts: false, + allowDivisionByUnit: false, + divideCourseWideTopics: false, + divideGeneralTopic: false, + divideQuestionsForTAs: false, + }); + }); + }); + + describe('selectApp', () => { + test('sets selectedAppId', () => { + const appId = 'piazza'; + store.dispatch(selectApp({ appId })); + + expect(store.getState().discussions.selectedAppId).toEqual(appId); + }); + }); + + describe('saveAppConfig', () => { + test('network error', async () => { + axiosMock.onGet(getAppsUrl(courseId)).reply(200, piazzaApiResponse); + axiosMock.onPost(getAppsUrl(courseId)).networkError(); + + // We call fetchApps and selectApp here too just to get us into a real state. + await executeThunk(fetchApps(courseId), store.dispatch); + store.dispatch(selectApp({ appId: 'piazza' })); + await executeThunk(saveAppConfig(courseId, 'piazza', {}), store.dispatch); + + expect(store.getState().discussions).toEqual( + expect.objectContaining({ + appIds: ['legacy', 'piazza'], + featureIds, + activeAppId: 'piazza', + selectedAppId: 'piazza', + status: LOADED, + saveStatus: FAILED, + }), + ); + }); + + test('successfully saves an LTI configuration', async () => { + axiosMock.onGet(getAppsUrl(courseId)).reply(200, piazzaApiResponse); + axiosMock.onPost(getAppsUrl(courseId), { + context_key: courseId, + enabled: true, + lti_configuration: { + lti_1p1_client_key: 'new_consumer_key', + lti_1p1_client_secret: 'new_consumer_secret', + lti_1p1_launch_url: 'http://localhost/new_launch_url', + version: 'lti_1p1', + }, + plugin_configuration: {}, + provider_type: 'piazza', + }).reply(200, { + ...piazzaApiResponse, // This uses the existing configuration but with a replacement + // lti_configuration that matches what we tried to save. + lti_configuration: { + lti_1p1_client_key: 'new_consumer_key', + lti_1p1_client_secret: 'new_consumer_secret', + lti_1p1_launch_url: 'https://localhost/new_launch_url', + version: 'lti_1p1', + }, + }); + + // We call fetchApps and selectApp here too just to get us into a real state. + await executeThunk(fetchApps(courseId), store.dispatch); + store.dispatch(selectApp({ appId: 'piazza' })); + await executeThunk(saveAppConfig(courseId, 'piazza', { + consumerKey: 'new_consumer_key', + consumerSecret: 'new_consumer_secret', + launchUrl: 'http://localhost/new_launch_url', + }), store.dispatch); + + expect(store.getState().discussions).toEqual( + expect.objectContaining({ + appIds: ['legacy', 'piazza'], + featureIds, + activeAppId: 'piazza', + selectedAppId: 'piazza', + status: LOADED, + saveStatus: SAVED, + }), + ); + expect(store.getState().models.appConfigs.piazza).toEqual({ + id: 'piazza', + consumerKey: 'new_consumer_key', + consumerSecret: 'new_consumer_secret', + launchUrl: 'https://localhost/new_launch_url', + }); + }); + + test('successfully saves a Legacy configuration', async () => { + axiosMock.onGet(getAppsUrl(courseId)).reply(200, legacyApiResponse); + axiosMock.onPost(getAppsUrl(courseId), { + context_key: courseId, + enabled: true, + lti_configuration: {}, + plugin_configuration: { + allow_anonymous: true, + allow_anonymous_to_peers: true, + discussion_blackouts: [['2015-09-15', '2015-09-21'], ['2015-10-01', '2015-10-08']], + }, + provider_type: 'legacy', + }).reply(200, { + ...legacyApiResponse, // This uses the existing configuration but with a replacement + // plugin_configuration that matches what we tried to save. + plugin_configuration: { + allow_anonymous: true, + allow_anonymous_to_peers: true, + discussion_blackouts: [['2015-09-15', '2015-09-21'], ['2015-10-01', '2015-10-08']], + }, + }); + + // We call fetchApps and selectApp here too just to get us into a real state. + await executeThunk(fetchApps(courseId), store.dispatch); + store.dispatch(selectApp({ appId: 'legacy' })); + await executeThunk(saveAppConfig(courseId, 'legacy', { + allowAnonymousPosts: true, + allowAnonymousPostsPeers: true, + blackoutDates: '[["2015-09-15","2015-09-21"],["2015-10-01","2015-10-08"]]', + // TODO: Note! As of this writing, all the data below this line is NOT returned in the API + // but we technically send it to the thunk, so here it is. + divideByCohorts: true, + allowDivisionByUnit: true, + divideCourseWideTopics: true, + divideGeneralTopic: true, + divideQuestionsForTAs: true, + }), store.dispatch); + + expect(store.getState().discussions).toEqual( + expect.objectContaining({ + appIds: ['legacy', 'piazza'], + featureIds, + activeAppId: 'legacy', + selectedAppId: 'legacy', + status: LOADED, + saveStatus: SAVED, + }), + ); + expect(store.getState().models.appConfigs.legacy).toEqual({ + id: 'legacy', + // These three fields should be updated. + allowAnonymousPosts: true, + allowAnonymousPostsPeers: true, + blackoutDates: '[["2015-09-15","2015-09-21"],["2015-10-01","2015-10-08"]]', + // TODO: Note! The values we tried to save were ignored, this test reflects what currently + // happens, but NOT what we want to have happen! + divideByCohorts: false, + allowDivisionByUnit: false, + divideCourseWideTopics: false, + divideGeneralTopic: false, + divideQuestionsForTAs: false, + }); + }); + }); +}); diff --git a/src/pages-and-resources/discussions/app-list/data/slice.js b/src/pages-and-resources/discussions/data/slice.js similarity index 75% rename from src/pages-and-resources/discussions/app-list/data/slice.js rename to src/pages-and-resources/discussions/data/slice.js index f0c588911..112644921 100644 --- a/src/pages-and-resources/discussions/app-list/data/slice.js +++ b/src/pages-and-resources/discussions/data/slice.js @@ -4,9 +4,11 @@ import { createSlice } from '@reduxjs/toolkit'; export const LOADING = 'LOADING'; export const LOADED = 'LOADED'; export const FAILED = 'FAILED'; +export const SAVING = 'SAVING'; +export const SAVED = 'SAVED'; const slice = createSlice({ - name: 'appList', + name: 'discussions', initialState: { appIds: [], featureIds: [], @@ -17,24 +19,28 @@ const slice = createSlice({ // instead. selectedAppId: null, status: LOADING, + saveStatus: SAVED, }, reducers: { loadApps: (state, { payload }) => { state.activeAppId = payload.activeAppId; - // When the UI loads, we want to set the selectedAppId to the activeAppId. This ensures the - // active one will be checked. - state.selectedAppId = payload.activeAppId; state.appIds = payload.appIds; state.featureIds = payload.featureIds; state.status = LOADED; + state.saveStatus = SAVED; }, selectApp: (state, { payload }) => { const { appId } = payload; state.selectedAppId = appId; }, - updateStatus: (state, { status }) => { + updateStatus: (state, { payload }) => { + const { status } = payload; state.status = status; }, + updateSaveStatus: (state, { payload }) => { + const { status } = payload; + state.saveStatus = status; + }, }, }); @@ -42,6 +48,7 @@ export const { loadApps, selectApp, updateStatus, + updateSaveStatus, } = slice.actions; export const { diff --git a/src/pages-and-resources/discussions/data/thunks.js b/src/pages-and-resources/discussions/data/thunks.js new file mode 100644 index 000000000..0c26dc766 --- /dev/null +++ b/src/pages-and-resources/discussions/data/thunks.js @@ -0,0 +1,70 @@ +import { addModel, addModels } from '../../../generic/model-store'; + +import { getApps, postAppConfig } from './api'; +import { + FAILED, + loadApps, + LOADING, + SAVED, + SAVING, + updateStatus, + updateSaveStatus, +} from './slice'; + +export function fetchApps(courseId) { + return async (dispatch) => { + dispatch(updateStatus({ status: LOADING })); + try { + const { + apps, + features, + activeAppId, + appConfig, + } = await getApps(courseId); + + dispatch(addModels({ modelType: 'apps', models: apps })); + dispatch(addModels({ modelType: 'features', models: features })); + dispatch(addModel({ modelType: 'appConfigs', model: appConfig })); + dispatch(loadApps({ + activeAppId, + appIds: apps.map(app => app.id), + featureIds: features.map(feature => feature.id), + })); + } catch (error) { + // TODO: We need generic error handling in the app for when a request just fails... in other + // parts of the app (proctored exam settings) we show a nice message and ask the user to + // reload/try again later. + dispatch(updateStatus({ status: FAILED })); + } + }; +} + +export function saveAppConfig(courseId, appId, drafts) { + return async (dispatch) => { + dispatch(updateSaveStatus({ status: SAVING })); + + try { + const { + apps, + features, + activeAppId, + appConfig, + } = await postAppConfig(courseId, appId, drafts); + + dispatch(addModels({ modelType: 'apps', models: apps })); + dispatch(addModels({ modelType: 'features', models: features })); + dispatch(addModel({ modelType: 'appConfigs', model: appConfig })); + dispatch(loadApps({ + activeAppId, + appIds: apps.map(app => app.id), + featureIds: features.map(feature => feature.id), + })); + dispatch(updateSaveStatus({ status: SAVED })); + } catch (error) { + // TODO: We need generic error handling in the app for when a request just fails... in other + // parts of the app (proctored exam settings) we show a nice message and ask the user to + // reload/try again later. + dispatch(updateSaveStatus({ status: FAILED })); + } + }; +} diff --git a/src/pages-and-resources/discussions/index.js b/src/pages-and-resources/discussions/index.js index 58b407589..aac60e6db 100644 --- a/src/pages-and-resources/discussions/index.js +++ b/src/pages-and-resources/discussions/index.js @@ -1,2 +1,2 @@ export { default } from './DiscussionsSettings'; -export { default as reducer } from './data/reducer'; +export { reducer } from './data/slice'; diff --git a/src/pages-and-resources/pages/PageCard.jsx b/src/pages-and-resources/pages/PageCard.jsx index 68f8525ea..4c5120d35 100644 --- a/src/pages-and-resources/pages/PageCard.jsx +++ b/src/pages-and-resources/pages/PageCard.jsx @@ -1,14 +1,14 @@ import PropTypes from 'prop-types'; -import React from 'react'; +import React, { useContext } from 'react'; import classNames from 'classnames'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { Button } from '@edx/paragon'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faCog } from '@fortawesome/free-solid-svg-icons'; -import { useLocation } from 'react-router'; import { history } from '@edx/frontend-platform'; import messages from '../messages'; +import { PagesAndResourcesContext } from '../PagesAndResourcesProvider'; const CoursePageShape = PropTypes.shape({ id: PropTypes.string.isRequired, @@ -23,7 +23,7 @@ const CoursePageShape = PropTypes.shape({ export { CoursePageShape }; function PageCard({ intl, page }) { - const { pathname } = useLocation(); + const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext); const pageStatusMsgId = page.isEnabled ? 'pageStatus.enabled' : 'pageStatus.disabled'; const componentClasses = classNames( @@ -33,7 +33,7 @@ function PageCard({ intl, page }) { ); const handleClick = () => { - history.push(`${pathname}/${page.id}`); + history.push(`${pagesAndResourcesPath}/${page.id}`); }; return ( diff --git a/src/proctored-exam-settings/ProctoredExamSettings.jsx b/src/proctored-exam-settings/ProctoredExamSettings.jsx index b60750587..c4ed093ee 100644 --- a/src/proctored-exam-settings/ProctoredExamSettings.jsx +++ b/src/proctored-exam-settings/ProctoredExamSettings.jsx @@ -14,6 +14,7 @@ import { import messages from './ProctoredExamSettings.messages'; import StudioApiService from '../data/services/StudioApiService'; +import Loading from '../generic/Loading'; function ProctoredExamSettings({ courseId, intl }) { const [loading, setLoading] = useState(true); @@ -387,23 +388,7 @@ function ProctoredExamSettings({ courseId, intl }) { function renderLoading() { return ( -
- - - - - -
+ ); }