From 9b2ad5e95de3287bd475b7beb6a4f8998349d207 Mon Sep 17 00:00:00 2001 From: David Joy Date: Mon, 11 Jan 2021 15:52:24 -0500 Subject: [PATCH] Backing discussions with data API/thunks/reducer. (#47) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Backing discussions with data API/thunks/reducer. This pulls all the data loading logic out of the React components and makes it significantly more flexible. - Both apps and features have IDs and can be looked up in the store. - The API layer is currently just returning hard coded data. - LOADED and LOADING statuses are available to implement loading spinners and feedback. - The taxonomy has been changed a bit - “forums and “tools” are now consistently referred to as “apps” - this code is almost completely agnostic to discussions, meaning that it could easily be repurposed for other kinds of apps, such as proctoring providers. * Using ‘app’ and ‘name language in DiscussionAppCard messages. * Using the selectedApp’s name for the Configure button. * Misc review fixes - better comment on error handling - Fixing some CSS class names. --- .../discussions/DiscussionAppCard.jsx | 32 +++---- .../discussions/DiscussionAppList.jsx | 88 ++++++++----------- .../discussions/DiscussionAppList.scss | 2 +- .../discussions/FeaturesTable.jsx | 28 +++--- .../discussions/data/api.js | 87 ++++++++++++++++++ .../discussions/data/slice.js | 34 +++++++ .../discussions/data/thunks.js | 28 ++++++ .../discussions/messages.js | 16 ++-- src/store.js | 2 + 9 files changed, 228 insertions(+), 89 deletions(-) create mode 100644 src/pages-and-resources/discussions/data/api.js create mode 100644 src/pages-and-resources/discussions/data/slice.js create mode 100644 src/pages-and-resources/discussions/data/thunks.js diff --git a/src/pages-and-resources/discussions/DiscussionAppCard.jsx b/src/pages-and-resources/discussions/DiscussionAppCard.jsx index 0e25d4c4c..37bfa06a2 100644 --- a/src/pages-and-resources/discussions/DiscussionAppCard.jsx +++ b/src/pages-and-resources/discussions/DiscussionAppCard.jsx @@ -9,19 +9,19 @@ import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import messages from './messages'; -function DiscussionToolOption({ - intl, forum, selected, onSelect, +function DiscussionAppCard({ + intl, app, selected, clickHandler, }) { return (
{ if (forum.isAvailable) { onSelect(forum.forumId); } }} - onKeyPress={() => { if (forum.isAvailable) { onSelect(forum.forumId); } }} + tabIndex={app.isAvailable ? '-1' : ''} + onClick={() => { if (app.isAvailable) { clickHandler(app.id); } }} + onKeyPress={() => { if (app.isAvailable) { clickHandler(app.id); } }} role="radio" aria-checked={selected} > @@ -33,7 +33,7 @@ function DiscussionToolOption({ right: '0.75rem', }} > - {forum.isAvailable ? ( + {app.isAvailable ? ( ) : ( @@ -45,28 +45,28 @@ function DiscussionToolOption({
{intl.formatMessage(messages.toolLogo,

-
{forum.description}
+
{app.description}

-
{forum.supportLevel}
+
{app.supportLevel}
); } -DiscussionToolOption.propTypes = { +DiscussionAppCard.propTypes = { intl: intlShape.isRequired, - forum: PropTypes.objectOf(PropTypes.any).isRequired, + app: PropTypes.objectOf(PropTypes.any).isRequired, selected: PropTypes.bool.isRequired, - onSelect: PropTypes.func.isRequired, + clickHandler: PropTypes.func.isRequired, }; -export default injectIntl(DiscussionToolOption); +export default injectIntl(DiscussionAppCard); diff --git a/src/pages-and-resources/discussions/DiscussionAppList.jsx b/src/pages-and-resources/discussions/DiscussionAppList.jsx index 37b104083..e3a81da85 100644 --- a/src/pages-and-resources/discussions/DiscussionAppList.jsx +++ b/src/pages-and-resources/discussions/DiscussionAppList.jsx @@ -1,71 +1,49 @@ -import React, { useState } from 'react'; - +import React, { useCallback, useEffect, useState } from 'react'; +import PropTypes from 'prop-types'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { Button, Container, Row } from '@edx/paragon'; +import { useDispatch, useSelector } from 'react-redux'; + import messages from './messages'; import DiscussionAppCard from './DiscussionAppCard'; import FeaturesTable from './FeaturesTable'; +import { useModel, useModels } from '../../generic/model-store'; +import { fetchApps } from './data/thunks'; -// XXX this is just for testing and should be removed ASAP -const forums = [ - { - forumId: 'edX Forum', - logo: 'https://cdn-blog.lawrencemcdaniel.com/wp-content/uploads/2018/01/22125436/edx-logo.png', - description: 'Start conversations with other learners, ask questions, and interact with other learners in the course.', - supportLevel: 'Full support', - isAvailable: true, - features: ['LTI Integration', 'Discussion Page', 'Embedded Course Sections', 'Embedded Course Units', 'WCAG 2.1 Support'], - }, - { - forumId: 'Piazza', - logo: 'https://cdn-blog.lawrencemcdaniel.com/wp-content/uploads/2018/01/22125436/edx-logo.png', - description: 'Piazza is designed to connect students, TAs, and professors so every student can get the help they need when they need it', - supportLevel: 'Partial support', - isAvailable: true, - features: ['LTI Integration', 'Discussion Page', 'Embedded Course Sections', 'WCAG 2.1 Support'], - }, - { - forumId: 'Yellowdig', - logo: 'https://cdn-blog.lawrencemcdaniel.com/wp-content/uploads/2018/01/22125436/edx-logo.png', - description: 'Yellowdig is the digital solution that impacts the entire student lifecycle and enables lifelong learning.', - supportLevel: 'Coming soon', - isAvailable: false, - features: ['LTI Integration', 'Discussion Page', 'Embedded Course Sections', 'WCAG 2.1 Support'], - }, - { - forumId: 'Untitled Forum', - logo: 'https://cdn-blog.lawrencemcdaniel.com/wp-content/uploads/2018/01/22125436/edx-logo.png', - description: 'Start conversations with other learners, ask questions, and interact with other learners in the course.', - supportLevel: 'Full support', - isAvailable: true, - features: ['LTI Integration', 'Discussion Page', 'Embedded Course Sections', 'Embedded Course Units', 'WCAG 2.1 Support'], - }, -]; +function DiscussionAppList({ courseId, intl }) { + const [selectedAppId, setSelectedAppId] = useState(null); -const featuresList = ['LTI Integration', 'Discussion Page', 'Embedded Course Sections', 'Embedded Course Units', 'WCAG 2.1 Support']; + const dispatch = useDispatch(); + useEffect(() => { + dispatch(fetchApps(courseId)); + }, [courseId]); -function DiscussionAppList({ intl }) { - const [selectedForumId, setSelectedForumId] = useState(null); + const appIds = useSelector(state => state.discussions.appIds); + const featureIds = useSelector(state => state.discussions.featureIds); + const apps = useModels('apps', appIds); + const features = useModels('features', featureIds); - const onSelectForum = (forumId) => { - if (selectedForumId === forumId) { - setSelectedForumId(null); + const selectedApp = useModel('apps', selectedAppId); + + const handleSelectApp = useCallback((appId) => { + if (selectedAppId === appId) { + setSelectedAppId(null); } else { - setSelectedForumId(forumId); + setSelectedAppId(appId); } - }; + }, [selectedAppId]); return (
{intl.formatMessage(messages.heading)}
- {forums.map(forum => ( + {apps.map(app => ( ))} @@ -74,19 +52,23 @@ function DiscussionAppList({ intl }) {

{intl.formatMessage(messages.supportedFeatures)}

- {selectedForumId && ( + {selectedAppId && ( )} - + ); } DiscussionAppList.propTypes = { + courseId: PropTypes.string.isRequired, intl: intlShape.isRequired, }; diff --git a/src/pages-and-resources/discussions/DiscussionAppList.scss b/src/pages-and-resources/discussions/DiscussionAppList.scss index afb17e34c..cf29b3a59 100644 --- a/src/pages-and-resources/discussions/DiscussionAppList.scss +++ b/src/pages-and-resources/discussions/DiscussionAppList.scss @@ -1,4 +1,4 @@ -.discussion-tool { +.discussion-app-card { &:hover, &:focus { border-color: theme-color("primary", "hover") !important; diff --git a/src/pages-and-resources/discussions/FeaturesTable.jsx b/src/pages-and-resources/discussions/FeaturesTable.jsx index c101cf8db..18f351542 100644 --- a/src/pages-and-resources/discussions/FeaturesTable.jsx +++ b/src/pages-and-resources/discussions/FeaturesTable.jsx @@ -3,25 +3,31 @@ import PropTypes from 'prop-types'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faCheck } from '@fortawesome/free-solid-svg-icons'; -export default function FeaturesTable({ forums, featuresList }) { +export default function FeaturesTable({ apps, features }) { return (
- {forums.map(forum => ( - + {apps.map(app => ( + ))} - {featuresList.map(feature => ( - - - {forums.map(forum => ( - + + {apps.map(app => ( + ))} @@ -33,6 +39,6 @@ export default function FeaturesTable({ forums, featuresList }) { } FeaturesTable.propTypes = { - forums: PropTypes.arrayOf(PropTypes.object).isRequired, - featuresList: PropTypes.arrayOf(PropTypes.string).isRequired, + apps: PropTypes.arrayOf(PropTypes.object).isRequired, + features: PropTypes.arrayOf(PropTypes.object).isRequired, }; 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..a19f674eb --- /dev/null +++ b/src/pages-and-resources/discussions/data/api.js @@ -0,0 +1,87 @@ +/* eslint-disable import/prefer-default-export */ +export function getDiscussionApps() { + 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: [ + { + id: 'edx-forums', + name: 'edX Forum', + logo: 'https://cdn-blog.lawrencemcdaniel.com/wp-content/uploads/2018/01/22125436/edx-logo.png', + description: 'Start conversations with other learners, ask questions, and interact with other learners in the course.', + supportLevel: 'Full support', + isAvailable: true, + featureIds: [ + 'lti', + 'discussion-page', + 'embedded-course-sections', + 'embedded-course-units', + 'wcag-2.1', + ], + }, + { + id: 'piazza', + name: 'Piazza', + logo: 'https://cdn-blog.lawrencemcdaniel.com/wp-content/uploads/2018/01/22125436/edx-logo.png', + description: 'Piazza is designed to connect students, TAs, and professors so every student can get the help they need when they need it', + supportLevel: 'Partial support', + isAvailable: true, + featureIds: [ + 'lti', + 'discussion-page', + 'embedded-course-sections', + 'wcag-2.1', + ], + }, + { + id: 'yellowdig', + name: 'Yellowdig', + logo: 'https://cdn-blog.lawrencemcdaniel.com/wp-content/uploads/2018/01/22125436/edx-logo.png', + description: 'Yellowdig is the digital solution that impacts the entire student lifecycle and enables lifelong learning.', + supportLevel: 'Coming soon', + isAvailable: false, + featureIds: [ + 'lti', + 'discussion-page', + 'embedded-course-sections', + 'wcag-2.1', + ], + }, + { + id: 'untitled-forum', + name: 'Untitled Forum', + logo: 'https://cdn-blog.lawrencemcdaniel.com/wp-content/uploads/2018/01/22125436/edx-logo.png', + description: 'Start conversations with other learners, ask questions, and interact with other learners in the course.', + supportLevel: 'Full support', + isAvailable: true, + featureIds: [ + 'lti', + 'discussion-page', + 'embedded-course-sections', + 'embedded-course-units', + 'wcag-2.1', + ], + }, + ], + }); +} diff --git a/src/pages-and-resources/discussions/data/slice.js b/src/pages-and-resources/discussions/data/slice.js new file mode 100644 index 000000000..82ebb1dc7 --- /dev/null +++ b/src/pages-and-resources/discussions/data/slice.js @@ -0,0 +1,34 @@ +/* eslint-disable no-param-reassign */ +import { createSlice } from '@reduxjs/toolkit'; + +export const LOADING = 'LOADING'; +export const LOADED = 'LOADED'; +export const FAILED = 'FAILED'; + +const slice = createSlice({ + name: 'discussions', + initialState: { + appIds: [], + featureIds: [], + status: LOADING, + }, + reducers: { + fetchAppsSuccess: (state, { payload }) => { + state.appIds = payload.appIds; + state.featureIds = payload.featureIds; + state.status = LOADED; + }, + updateStatus: (state, { payload }) => { + state.status = payload.status; + }, + }, +}); + +export const { + fetchAppsSuccess, + updateStatus, +} = slice.actions; + +export const { + reducer, +} = slice; 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..2f2d4a85b --- /dev/null +++ b/src/pages-and-resources/discussions/data/thunks.js @@ -0,0 +1,28 @@ +import { getDiscussionApps } from './api'; +import { addModels } from '../../../generic/model-store'; +import { + FAILED, fetchAppsSuccess, LOADING, updateStatus, +} from './slice'; + +/* eslint-disable import/prefer-default-export */ +export function fetchApps(courseId) { + return async (dispatch) => { + dispatch(updateStatus({ courseId, status: LOADING })); + + try { + const { apps, features } = await getDiscussionApps(courseId); + + dispatch(addModels({ modelType: 'apps', models: apps })); + dispatch(addModels({ modelType: 'features', models: features })); + dispatch(fetchAppsSuccess({ + 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({ courseId, status: FAILED })); + } + }; +} diff --git a/src/pages-and-resources/discussions/messages.js b/src/pages-and-resources/discussions/messages.js index b92db3869..9450d4e40 100644 --- a/src/pages-and-resources/discussions/messages.js +++ b/src/pages-and-resources/discussions/messages.js @@ -2,20 +2,20 @@ import { defineMessages } from '@edx/frontend-platform/i18n'; const messages = defineMessages({ heading: { - id: 'authoring.forumSelectorTool.heading', + id: 'authoring.discussions.heading', defaultMessage: 'Which discussion tool would you like to use for this course?', }, supportedFeatures: { - id: 'authoring.forumSelectorTool.supportedFeatures', + id: 'authoring.discussions.supportedFeatures', defaultMessage: 'Supported Features', }, - configureTool: { - id: 'authoring.forumSelectorTool.configureTool', - defaultMessage: 'Configure {toolName}', + configureApp: { + id: 'authoring.discussions.configureApp', + defaultMessage: 'Configure {name}', }, - toolLogo: { - id: 'authoring.forumSelectorTool.toolLogo', - defaultMessage: '{toolName} Logo', + appLogo: { + id: 'authoring.discussions.appLogo', + defaultMessage: '{name} Logo', }, }); diff --git a/src/store.js b/src/store.js index f0c79956b..c489d5643 100644 --- a/src/store.js +++ b/src/store.js @@ -2,11 +2,13 @@ import { configureStore } from '@reduxjs/toolkit'; import { reducer as modelsReducer } from './generic/model-store'; import { reducer as courseDetailReducer } from './data/slice'; +import { reducer as discussionsReducer } from './pages-and-resources/discussions/data/slice'; export default function initializeStore() { return configureStore({ reducer: { courseDetail: courseDetailReducer, + discussions: discussionsReducer, models: modelsReducer, }, });
 
{forum.forumId}
 +
{app.name}
+
{feature} - {forum.features.includes(feature) && } + {features.map(feature => ( +
+ {feature.name} + + {app.featureIds.includes(feature.id) && ( + + )}