feat: hookup discussions config to POST APIs (#83)

* fix: PageCard should use the pages and resources path from context

This prevents some odd behaviors around changing the URL based on the current path.

* fix: use the correct formRef for AppConfigForm children

Straight up refactoring bug.  This should be using the formRef from the context, but was still defining its own and passing it down.  This ultimately meant that the submit button wasn’t properly hooked up to the form, since the form was given a different ref than the submit button.

* fix: Only validate fields that have been touched

* fix: use the title instead of app.name

app.name doesn’t exist - but the title is the string we’re looking for.  Use it.

* refactor: default appConfig appropriately for both config forms

In subsequent PRs we’re going to start passing null appConfigs to forms sometimes - this uses prop types to set default values for the forms if the component isn’t passed a meaningful appConfig.

* refactor: replace and flesh out discussions data layer

Since we only have one GET endpoint for all the data around discussions, we don’t need two separate slices/thunks/api files.  The API needs to be hit once when the discussions settings load.  This means the app-list and app-config-form share far more state than originally thought.

The AppList and AppConfigForm are no longer responsible for data loading - we’ve moved that back up into DiscussionsSettings.  Now they just read from the redux state and load what they need.

The main change is moving app-list/data/slice.js up to discussions/data/slice.js, renaming the reducer, and adding a saveStatus to it - turns out this is all we need.  The original two slices go away.

The discussions/data/api file gets a proper implementation of postAppConfig which mostly works with the server as of this writing - we have some data shape issues to figure out.

AppConfigForm needed a little help.  It now checks whether our data is loaded and selects an app based on its route.  This allows us to deep link in and keep the selectedAppId correct.

* Adding a loading spinner to AppList

This is only tangentally related to the current PR, admittedly.

* test: adding some title tests to LegacyConfigForm.test.jsx

Needed to add the title prop, then decided I’d test it.

* test: adding a test suite for discussions/data files

This test exercises the thunks, slice, and api parts of the data layer all at once in ‘real’ scenarios with mocked API requests.
This commit is contained in:
David Joy
2021-04-22 07:49:08 -04:00
committed by GitHub
parent dff749d0ab
commit dbad0f6aac
24 changed files with 738 additions and 403 deletions

25
src/generic/Loading.jsx Normal file
View File

@@ -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 (
<div
className="d-flex justify-content-center align-items-center flex-column"
style={{
height: '50vh',
}}
data-test-id="spinnerContainer"
>
<Spinner className animation="border" role="status" variant="primary">
<span className="sr-only">
<FormattedMessage
id="authoring.loading"
defaultMessage="Loading..."
description="Screen-reader message for when a page is loading."
/>
</span>
</Spinner>
</div>
);
}

View File

@@ -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 }) {
<Stepper.Body className="bg-light-200">
<Switch>
<PageRoute exact path={`${path}`}>
<AppList
courseId={courseId}
/>
<AppList />
</PageRoute>
<PageRoute path={`${path}/configure/:appId`}>
<AppConfigForm

View File

@@ -1,51 +1,58 @@
import React, {
useCallback, useContext, useEffect, useRef,
useCallback, useContext, useEffect,
} from 'react';
import PropTypes from 'prop-types';
import { useDispatch, useSelector } from 'react-redux';
import { useRouteMatch } from 'react-router';
import { Container } from '@edx/paragon';
import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { history } from '@edx/frontend-platform';
import { Container } from '@edx/paragon';
import { useModel } from '../../../generic/model-store';
import { fetchAppConfig, saveAppConfig } from './data/thunks';
import { PagesAndResourcesContext } from '../../PagesAndResourcesProvider';
import { LOADED, selectApp } from '../data/slice';
import { saveAppConfig } from '../data/thunks';
import messages from './messages';
import AppConfigFormProvider, { AppConfigFormContext } from './AppConfigFormProvider';
import AppConfigFormApplyButton from './AppConfigFormApplyButton';
import LegacyConfigForm from './apps/legacy';
import LtiConfigForm from './apps/lti';
import messages from './messages';
import { PagesAndResourcesContext } from '../../PagesAndResourcesProvider';
import AppConfigFormProvider from './AppConfigFormProvider';
import AppConfigFormApplyButton from './AppConfigFormApplyButton';
function AppConfigForm({
courseId, intl,
}) {
const { params: { appId: routeAppId } } = useRouteMatch();
const formRef = useRef();
const { path: pagesAndResourcesPath } = useContext(PagesAndResourcesContext);
const dispatch = useDispatch();
useEffect(() => {
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 = (
<LegacyConfigForm

View File

@@ -4,14 +4,14 @@ import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { StatefulButton } from '@edx/paragon';
import messages from './messages';
import { SAVING } from './data/slice';
import { SAVING } from '../data/slice';
import { AppConfigFormContext } from './AppConfigFormProvider';
function AppConfigFormApplyButton({ intl }) {
const status = useSelector(state => 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(() => {

View File

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

View File

@@ -16,6 +16,20 @@ const defaultAppConfig = {
};
describe('LegacyConfigForm', () => {
test('title rendering', () => {
const { container } = render(
<IntlProvider locale="en">
<LegacyConfigForm
title="Test Legacy edX Discussions"
appConfig={defaultAppConfig}
onSubmit={jest.fn()}
formRef={createRef()}
/>
</IntlProvider>,
);
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(
<IntlProvider locale="en">
<LegacyConfigForm
title="Test Legacy edX Discussions"
appConfig={defaultAppConfig}
onSubmit={handleSubmit}
formRef={formRef}
@@ -48,6 +63,7 @@ describe('LegacyConfigForm', () => {
const { container } = render(
<IntlProvider locale="en">
<LegacyConfigForm
title="Test Legacy edX Discussions"
appConfig={defaultAppConfig}
onSubmit={jest.fn()}
formRef={createRef()}
@@ -77,6 +93,7 @@ describe('LegacyConfigForm', () => {
const { container } = render(
<IntlProvider locale="en">
<LegacyConfigForm
title="Test Legacy edX Discussions"
appConfig={{
...defaultAppConfig,
divideByCohorts: true,

View File

@@ -16,6 +16,7 @@ function LtiConfigForm({
handleChange,
handleBlur,
values,
touched,
errors,
} = useFormik({
initialValues: appConfig,
@@ -27,6 +28,10 @@ function LtiConfigForm({
onSubmit,
});
const isInvalidConsumerKey = touched.consumerKey && errors.consumerKey;
const isInvalidConsumerSecret = touched.consumerSecret && errors.consumerSecret;
const isInvalidLaunchUrl = touched.launchUrl && errors.launchUrl;
return (
<Card className="mb-5 pt-3 px-5 pb-5">
<Form ref={formRef} onSubmit={handleSubmit}>
@@ -44,47 +49,46 @@ function LtiConfigForm({
target="_blank"
rel="noopener noreferrer"
>
{intl.formatMessage(messages.documentationPage, { name: app.name })}
{intl.formatMessage(messages.documentationPage, { name: title })}
</Hyperlink>
),
name: app.name,
}}
/>
</p>
<Form.Group controlId="consumerKey" isInvalid={errors.consumerKey}>
<Form.Group controlId="consumerKey" isInvalid={isInvalidConsumerKey}>
<Form.Label>{intl.formatMessage(messages.consumerKey)}</Form.Label>
<Form.Control
onChange={handleChange}
onBlur={handleBlur}
value={values.consumerKey}
/>
{errors.consumerKey && (
{isInvalidConsumerKey && (
<Form.Control.Feedback type="invalid">
{errors.consumerKey}
</Form.Control.Feedback>
)}
</Form.Group>
<Form.Group controlId="consumerSecret" isInvalid={errors.consumerSecret}>
<Form.Group controlId="consumerSecret" isInvalid={isInvalidConsumerSecret}>
<Form.Label>{intl.formatMessage(messages.consumerSecret)}</Form.Label>
<Form.Control
onChange={handleChange}
onBlur={handleBlur}
value={values.consumerSecret}
/>
{errors.consumerSecret && (
{isInvalidConsumerSecret && (
<Form.Control.Feedback type="invalid">
{errors.consumerSecret}
</Form.Control.Feedback>
)}
</Form.Group>
<Form.Group controlId="launchUrl" isInvalid={errors.launchUrl}>
<Form.Group controlId="launchUrl" isInvalid={isInvalidLaunchUrl}>
<Form.Label>{intl.formatMessage(messages.launchUrl)}</Form.Label>
<Form.Control
onChange={handleChange}
onBlur={handleBlur}
value={values.launchUrl}
/>
{errors.launchUrl && (
{isInvalidLaunchUrl && (
<Form.Control.Feedback type="invalid">
{errors.launchUrl}
</Form.Control.Feedback>
@@ -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);

View File

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

View File

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

View File

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

View File

@@ -1,2 +1 @@
export { default } from './AppConfigForm';
export { reducer } from './data/slice';

View File

@@ -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 (
<Loading />
);
}
if (status === LOADED && apps.length === 0) {
return (
<Container className="mt-5">
@@ -54,7 +58,7 @@ function AppList({ courseId, intl }) {
<AppCard
key={app.id}
app={app}
selected={app.id === selectedAppId}
selected={app.id === finalSelectedAppId}
onClick={handleSelectApp}
/>
))}
@@ -73,7 +77,6 @@ function AppList({ courseId, intl }) {
}
AppList.propTypes = {
courseId: PropTypes.string.isRequired,
intl: intlShape.isRequired,
};

View File

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

View File

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

View File

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

View File

@@ -1,2 +1 @@
export { default } from './AppList';
export { reducer } from './data/slice';

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@@ -1,2 +1,2 @@
export { default } from './DiscussionsSettings';
export { default as reducer } from './data/reducer';
export { reducer } from './data/slice';

View File

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

View File

@@ -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 (
<div
className="d-flex justify-content-center align-items-center flex-column"
style={{
height: '50vh',
}}
data-test-id="spinnerContainer"
>
<Spinner className animation="border" role="status" variant="primary">
<span className="sr-only">
<FormattedMessage
id="authoring.examsettings.loading"
defaultMessage="Loading..."
description=""
/>
</span>
</Spinner>
</div>
<Loading />
);
}