From 0f8cc4b3ae83aa985cde6684f8975c8490c171cd Mon Sep 17 00:00:00 2001 From: David Joy Date: Tue, 23 Mar 2021 15:14:39 -0400 Subject: [PATCH] feat: hook discussions provider list up to API (#57) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this, the list of providers in the UI was hardcoded in the api.js layer. This commit hooks that up to our actual API endpoint, allowing us to load the provider list from the server. It also - out of necessity - changes the way the AppCards are displayed, and what content is in them. This was somewhat opportunistic as our design for them simplified anyway, no longer requiring a logo or a few of the other fields. Because the actual API sends us less display-oriented data (i.e., no names or descriptions for things), we had to modify FeaturesTable and AppCard to fetch these strings from the messages file based on the app and feature IDs. I’m not super thrilled with this approach, since it’s somewhat brittle. Unexpected providers won’t display properly. In the long run, I expect all this may come from some other system. Using translations to load dynamic strings isn’t quite what it was intended for - i.e., we’re not putting “Piazza” in there because we want to translate it, but because the backend didn’t tell us what to use for the key “piazza”. --- .../discussions/AppCard.jsx | 48 +++++--------- .../discussions/ConfigFormContainer.jsx | 2 +- .../discussions/FeaturesTable.jsx | 11 +++- .../discussions/data/api.js | 66 ++++++++----------- .../discussions/data/slice.js | 1 + .../discussions/data/thunks.js | 3 +- .../discussions/messages.js | 56 +++++++++++++++- 7 files changed, 110 insertions(+), 77 deletions(-) diff --git a/src/pages-and-resources/discussions/AppCard.jsx b/src/pages-and-resources/discussions/AppCard.jsx index c272aa833..1a55915b6 100644 --- a/src/pages-and-resources/discussions/AppCard.jsx +++ b/src/pages-and-resources/discussions/AppCard.jsx @@ -3,20 +3,22 @@ import PropTypes from 'prop-types'; import classNames from 'classnames'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { Card, Input } from '@edx/paragon'; -import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; -import { faLock } from '@fortawesome/free-solid-svg-icons'; import messages from './messages'; function AppCard({ app, onClick, intl, selected, }) { + const supportText = app.hasFullSupport + ? intl.formatMessage(messages.appFullSupport) + : intl.formatMessage(messages.appPartialSupport); + return ( { if (app.isAvailable) { onClick(app.id); } }} - onKeyPress={() => { if (app.isAvailable) { onClick(app.id); } }} + tabIndex="-1" + onClick={() => onClick(app.id)} + onKeyPress={() => onClick(app.id)} role="radio" aria-checked={selected} style={{ @@ -29,48 +31,28 @@ function AppCard({
- {app.isAvailable ? ( - - ) : ( - - )} +
- - {app.name} - {app.description} + + {intl.formatMessage(messages[`appName-${app.id}`])} + + {supportText} + {intl.formatMessage(messages[`appDescription-${app.id}`])} - - {app.supportLevel} - ); } AppCard.propTypes = { app: PropTypes.shape({ - description: PropTypes.string.isRequired, id: PropTypes.string.isRequired, - isAvailable: PropTypes.bool.isRequired, - logo: PropTypes.string.isRequired, - name: PropTypes.string.isRequired, - supportLevel: PropTypes.string.isRequired, + featureIds: PropTypes.arrayOf(PropTypes.string).isRequired, + hasFullSupport: PropTypes.bool.isRequired, }).isRequired, onClick: PropTypes.func.isRequired, selected: PropTypes.bool.isRequired, diff --git a/src/pages-and-resources/discussions/ConfigFormContainer.jsx b/src/pages-and-resources/discussions/ConfigFormContainer.jsx index 47fce44c3..cd117c9db 100644 --- a/src/pages-and-resources/discussions/ConfigFormContainer.jsx +++ b/src/pages-and-resources/discussions/ConfigFormContainer.jsx @@ -32,7 +32,7 @@ function ConfigFormContainer({ let form = null; - if (app.id === 'edx-discussions') { + if (app.id === 'legacy') { form = ( ({ - Header: app.name, + Header: intl.formatMessage(messages[`appName-${app.id}`]), accessor: app.id, })), ]} @@ -50,7 +52,10 @@ export default function FeaturesTable({ apps, features }) { ); } +export default injectIntl(FeaturesTable); + FeaturesTable.propTypes = { apps: PropTypes.arrayOf(PropTypes.object).isRequired, features: PropTypes.arrayOf(PropTypes.object).isRequired, + intl: intlShape.isRequired, }; diff --git a/src/pages-and-resources/discussions/data/api.js b/src/pages-and-resources/discussions/data/api.js index 644b4d40f..51c974033 100644 --- a/src/pages-and-resources/discussions/data/api.js +++ b/src/pages-and-resources/discussions/data/api.js @@ -1,3 +1,31 @@ +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, + activeAppId: data.providers.active, + apps, + }; +} + +export async function getApps(courseId) { + const { data } = await getAuthenticatedHttpClient() + .get(`${getConfig().LMS_BASE_URL}/discussions/api/v0/${courseId}`); + + return normalizeApps(data); +} + const legacyEdXDiscussions = { id: 'edx-discussions', name: 'edX Discussions', @@ -47,39 +75,6 @@ const yellowdigApp = { ], }; -export function getApps() { - return Promise.resolve({ - 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', - }, - ], - apps: [ - legacyEdXDiscussions, - piazzaApp, - yellowdigApp, - ], - activeAppId: 'piazza', - }); -} - export function getAppConfig(courseId, appId) { let app = null; switch (appId) { @@ -124,23 +119,18 @@ export function getAppConfig(courseId, appId) { 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', }, ], }); diff --git a/src/pages-and-resources/discussions/data/slice.js b/src/pages-and-resources/discussions/data/slice.js index 9bf00362c..88e8b5318 100644 --- a/src/pages-and-resources/discussions/data/slice.js +++ b/src/pages-and-resources/discussions/data/slice.js @@ -21,6 +21,7 @@ const slice = createSlice({ fetchAppsSuccess: (state, { payload }) => { state.appIds = payload.appIds; state.featureIds = payload.featureIds; + state.activeAppId = payload.activeAppId; }, fetchAppConfigSuccess: (state, { payload }) => { state.activeAppId = payload.activeAppId; diff --git a/src/pages-and-resources/discussions/data/thunks.js b/src/pages-and-resources/discussions/data/thunks.js index 1a5531b77..e578e55ec 100644 --- a/src/pages-and-resources/discussions/data/thunks.js +++ b/src/pages-and-resources/discussions/data/thunks.js @@ -21,11 +21,12 @@ export function fetchApps(courseId) { dispatch(updateStatus({ courseId, status: LOADING })); try { - const { apps, features } = await getApps(courseId); + const { apps, features, activeAppId } = await getApps(courseId); dispatch(addModels({ modelType: 'apps', models: apps })); dispatch(addModels({ modelType: 'features', models: features })); dispatch(fetchAppsSuccess({ + activeAppId, appIds: apps.map(app => app.id), featureIds: features.map(feature => feature.id), })); diff --git a/src/pages-and-resources/discussions/messages.js b/src/pages-and-resources/discussions/messages.js index 754d9071a..f3d027cbc 100644 --- a/src/pages-and-resources/discussions/messages.js +++ b/src/pages-and-resources/discussions/messages.js @@ -3,7 +3,7 @@ import { defineMessages } from '@edx/frontend-platform/i18n'; const messages = defineMessages({ heading: { id: 'authoring.discussions.heading', - defaultMessage: 'Which discussion tool would you like to use for this course?', + defaultMessage: 'Select a discussion tool for this course', }, supportedFeatures: { id: 'authoring.discussions.supportedFeatures', @@ -60,6 +60,60 @@ const messages = defineMessages({ defaultMessage: 'Select discussion tool', description: 'A label for the first step of a wizard where the user chooses a discussion tool to configure.', }, + appFullSupport: { + id: 'authoring.discussions.appFullSupport', + defaultMessage: 'Full support', + description: 'A label indicating that an app supports the full set of possible features for a discussions app.', + }, + appPartialSupport: { + id: 'authoring.discussions.appPartialSupport', + defaultMessage: 'Partial support', + description: 'A label indicating that an app only supports a subset of the possible features of a discussions app.', + }, + // Legacy + 'appName-legacy': { + id: 'authoring.discussions.appName-legacy', + defaultMessage: 'Legacy edX Discussions', + description: 'The name of the Legacy edX Discussions app.', + }, + 'appDescription-legacy': { + id: 'authoring.discussions.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.', + }, + // Piazza + 'appName-piazza': { + id: 'authoring.discussions.appName-piazza', + defaultMessage: 'Piazza', + description: 'The name of the Piazza app.', + }, + 'appDescription-piazza': { + id: 'authoring.discussions.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.', + description: 'A description of the Piazza app.', + }, + + // Features + 'featureName-discussion-page': { + id: 'authoring.discussions.featureName-discussion-page', + defaultMessage: 'Discussion Page', + description: 'The name of a discussions feature.', + }, + 'featureName-embedded-course-sections': { + id: 'authoring.discussions.featureName-embedded-course-sections', + defaultMessage: 'Embedded Course Sections', + description: 'The name of a discussions feature.', + }, + 'featureName-lti': { + id: 'authoring.discussions.featureName-lti', + defaultMessage: 'LTI Integration', + description: 'The name of a discussions feature.', + }, + 'featureName-wcag-2.1': { + id: 'authoring.discussions.featureName-wcag-2.1', + defaultMessage: 'WCAG 2.1 Support', + description: 'The name of a discussions feature.', + }, }); export default messages;