From 1b4b30677e594d1bc05e51978865e83fa85e744c Mon Sep 17 00:00:00 2001 From: Mehak Nasir Date: Thu, 10 Jun 2021 14:50:34 +0500 Subject: [PATCH] refactor: renamed external link keys and implemented review fixes --- package-lock.json | 3 +- .../apps/lti/LtiConfigForm.jsx | 17 ++-- .../app-config-form/apps/lti/messages.js | 10 +-- .../apps/shared/AppExternalLinks.jsx | 67 +++++++++++++++ .../apps/shared/ExternalDocumentations.jsx | 83 ------------------- .../discussions/app-list/FeatureList.test.jsx | 2 +- .../app-list/FeaturesTable.test.jsx | 4 +- .../discussions/data/api.js | 8 +- .../discussions/data/redux.test.js | 24 +++--- .../discussions/factories/mockApiResponses.js | 40 ++++----- 10 files changed, 125 insertions(+), 133 deletions(-) create mode 100644 src/pages-and-resources/discussions/app-config-form/apps/shared/AppExternalLinks.jsx delete mode 100644 src/pages-and-resources/discussions/app-config-form/apps/shared/ExternalDocumentations.jsx diff --git a/package-lock.json b/package-lock.json index 7ed3a3c06..017f177bd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24703,7 +24703,8 @@ "uuid": { "version": "3.4.0", "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz", - "integrity": "sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==" + "integrity": "sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==", + "dev": true }, "v8-compile-cache": { "version": "2.3.0", diff --git a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx index 01f7314f4..373e3ad4c 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx +++ b/src/pages-and-resources/discussions/app-config-form/apps/lti/LtiConfigForm.jsx @@ -9,7 +9,7 @@ import * as Yup from 'yup'; import { useDispatch } from 'react-redux'; import AppConfigFormDivider from '../shared/AppConfigFormDivider'; -import ExternalDocumentations from '../shared/ExternalDocumentations'; +import AppExternalLinks from '../shared/AppExternalLinks'; import { updateValidationStatus, @@ -20,6 +20,7 @@ function LtiConfigForm({ appConfig, app, onSubmit, intl, formRef, title, }) { const dispatch = useDispatch(); + const { externalLinks } = app; const { handleSubmit, handleChange, @@ -91,7 +92,7 @@ function LtiConfigForm({ - + ); } @@ -99,12 +100,12 @@ function LtiConfigForm({ LtiConfigForm.propTypes = { app: PropTypes.shape({ id: PropTypes.string.isRequired, - documentationUrls: PropTypes.shape({ - learn_more: PropTypes.string, - configuration_documentation: PropTypes.string, - documentation: PropTypes.string, - accessibility_documentation: PropTypes.string, - emailId: PropTypes.string, + externalLinks: PropTypes.shape({ + learnMore: PropTypes.string, + configuration: PropTypes.string, + general: PropTypes.string, + accessibility: PropTypes.string, + contactEmail: PropTypes.string, }).isRequired, }).isRequired, appConfig: PropTypes.shape({ diff --git a/src/pages-and-resources/discussions/app-config-form/apps/lti/messages.js b/src/pages-and-resources/discussions/app-config-form/apps/lti/messages.js index f5ba371be..e69d5828b 100644 --- a/src/pages-and-resources/discussions/app-config-form/apps/lti/messages.js +++ b/src/pages-and-resources/discussions/app-config-form/apps/lti/messages.js @@ -44,22 +44,22 @@ const messages = defineMessages({ defaultMessage: 'Contact: {link}', description: 'Contact', }, - documentation: { + general: { id: 'authoring.discussions.appDocInstructions.documentationLink', defaultMessage: 'General documentation', description: 'Application Document Instructions message for documentation link', }, - accessibility_documentation: { + accessibility: { id: 'authoring.discussions.appDocInstructions.accessibilityDocumentationLink', defaultMessage: 'Accessibility documentation', description: 'Application Document Instructions message for accessibility link', }, - configuration_documentation: { - id: 'authoring.discussions.appDocInstructions.configurationDocumentationLink', + configuration: { + id: 'authoring.discussions.appDocInstructions.configurationLink', defaultMessage: 'Configuration documentation', description: 'Application Document Instructions message for configurations link', }, - learn_more: { + learnMore: { id: 'authoring.discussions.appDocInstructions.learnMoreLink', defaultMessage: 'Learn more about {title}', description: 'Application Document Instructions message for learn more links', diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/AppExternalLinks.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/AppExternalLinks.jsx new file mode 100644 index 000000000..847b30906 --- /dev/null +++ b/src/pages-and-resources/discussions/app-config-form/apps/shared/AppExternalLinks.jsx @@ -0,0 +1,67 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/i18n'; +import { + Hyperlink, MailtoLink, +} from '@edx/paragon'; + +import messages from '../lti/messages'; + +function AppExternalLinks({ + externalLinks, + intl, + title, +}) { + const { contactEmail, ...links } = externalLinks; + const linkTypes = Object.keys(links); + return ( +
+

{intl.formatMessage(messages.linkTextHeading)}

+
+ {linkTypes.map((type) => ( + links[type] && ( +
+ + {intl.formatMessage(messages[type], { title })} + +
+ ) + ))} +
+ {contactEmail && ( + + {contactEmail} + + ), + }} + /> + )} +
+
+ ); +} + +AppExternalLinks.propTypes = { + externalLinks: PropTypes.shape({ + learnMore: PropTypes.string, + configuration: PropTypes.string, + general: PropTypes.string, + accessibility: PropTypes.string, + contactEmail: PropTypes.string, + }).isRequired, + title: PropTypes.string.isRequired, + intl: intlShape.isRequired, +}; + +export default injectIntl(AppExternalLinks); diff --git a/src/pages-and-resources/discussions/app-config-form/apps/shared/ExternalDocumentations.jsx b/src/pages-and-resources/discussions/app-config-form/apps/shared/ExternalDocumentations.jsx deleted file mode 100644 index 2e8412ee6..000000000 --- a/src/pages-and-resources/discussions/app-config-form/apps/shared/ExternalDocumentations.jsx +++ /dev/null @@ -1,83 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/i18n'; -import { - Hyperlink, MailtoLink, -} from '@edx/paragon'; - -import messages from '../lti/messages'; - -const messageFormatting = (title, instructionType, intl, documentationUrls) => ( - documentationUrls[instructionType] - && instructionType !== 'email_id' && ( -
- - - - ), - }} - /> -
- ) -); - -function ExternalDocumentations({ - app, intl, title, -}) { - const { documentationUrls } = app; - const appInstructions = Object.keys(documentationUrls); - return ( -
-

{intl.formatMessage(messages.linkTextHeading)}

-
- {appInstructions.map((instructionType) => ( - messageFormatting(title, instructionType, intl, documentationUrls) - ))} -
- {documentationUrls.email_id && ( - - {documentationUrls.email_id} - - ), - }} - /> - )} -
-
- ); -} - -ExternalDocumentations.propTypes = { - app: PropTypes.shape({ - id: PropTypes.string.isRequired, - documentationUrls: PropTypes.shape({ - learn_more: PropTypes.string, - configuration_documentation: PropTypes.string, - documentation: PropTypes.string, - accessibility_documentation: PropTypes.string, - email_id: PropTypes.string, - }).isRequired, - }).isRequired, - intl: intlShape.isRequired, - title: PropTypes.string.isRequired, -}; - -export default injectIntl(ExternalDocumentations); diff --git a/src/pages-and-resources/discussions/app-list/FeatureList.test.jsx b/src/pages-and-resources/discussions/app-list/FeatureList.test.jsx index 00546ec85..8e8f418b1 100644 --- a/src/pages-and-resources/discussions/app-list/FeatureList.test.jsx +++ b/src/pages-and-resources/discussions/app-list/FeatureList.test.jsx @@ -9,7 +9,7 @@ import messages from './messages'; describe('FeaturesList', () => { const app = { - documentationUrls: {}, + externalLinks: {}, featureIds: ['discussion-page', 'embedded-course-sections', 'wcag-2.1'], hasFullSupport: false, id: 'legacy', diff --git a/src/pages-and-resources/discussions/app-list/FeaturesTable.test.jsx b/src/pages-and-resources/discussions/app-list/FeaturesTable.test.jsx index d4b9d77d4..c62a0f951 100644 --- a/src/pages-and-resources/discussions/app-list/FeaturesTable.test.jsx +++ b/src/pages-and-resources/discussions/app-list/FeaturesTable.test.jsx @@ -12,13 +12,13 @@ describe('FeaturesTable', () => { beforeEach(() => { apps = [ { - documentationUrls: {}, + externalLinks: {}, featureIds: ['discussion-page', 'embedded-course-sections', 'wcag-2.1'], hasFullSupport: false, id: 'legacy', }, { - documentationUrls: {}, + externalLinks: {}, featureIds: ['discussion-page', 'lti'], hasFullSupport: false, id: 'piazza', diff --git a/src/pages-and-resources/discussions/data/api.js b/src/pages-and-resources/discussions/data/api.js index db393311c..e3d331a00 100644 --- a/src/pages-and-resources/discussions/data/api.js +++ b/src/pages-and-resources/discussions/data/api.js @@ -48,7 +48,13 @@ function normalizeApps(data) { id: key, featureIds: app.features, // TODO: Fix this and get it from the backend! - documentationUrls: app.documentation_urls, + externalLinks: { + learnMore: app.external_links.learn_more, + configuration: app.external_links.configuration, + general: app.external_links.general, + accessibility: app.external_links.accessibility, + contactEmail: app.external_links.contact_email, + }, hasFullSupport: app.features.length >= data.features.length, })); return { diff --git a/src/pages-and-resources/discussions/data/redux.test.js b/src/pages-and-resources/discussions/data/redux.test.js index 6872c414a..ce3549891 100644 --- a/src/pages-and-resources/discussions/data/redux.test.js +++ b/src/pages-and-resources/discussions/data/redux.test.js @@ -43,12 +43,12 @@ const legacyApp = { 'embedded-course-sections', 'wcag-2.1', ], - documentationUrls: { - learn_more: '', - configuration_documentation: '', - documentation: '', - accessibility_documentation: '', - email_id: '', + externalLinks: { + learnMore: '', + configuration: '', + general: '', + accessibility: '', + contactEmail: '', }, hasFullSupport: false, }; @@ -61,12 +61,12 @@ const piazzaApp = { 'wcag-2.1', 'lti', ], - documentationUrls: { - learn_more: '', - configuration_documentation: '', - documentation: '', - accessibility_documentation: '', - email_id: '', + externalLinks: { + learnMore: '', + configuration: '', + general: '', + accessibility: '', + contactEmail: '', }, hasFullSupport: true, }; diff --git a/src/pages-and-resources/discussions/factories/mockApiResponses.js b/src/pages-and-resources/discussions/factories/mockApiResponses.js index f930e43db..588e69f50 100644 --- a/src/pages-and-resources/discussions/factories/mockApiResponses.js +++ b/src/pages-and-resources/discussions/factories/mockApiResponses.js @@ -24,12 +24,12 @@ export const piazzaApiResponse = { 'embedded-course-sections', 'wcag-2.1', ], - documentation_urls: { + external_links: { learn_more: '', - configuration_documentation: '', - documentation: '', - accessibility_documentation: '', - email_id: '', + configuration: '', + general: '', + accessibility: '', + contact_email: '', }, }, piazza: { @@ -40,12 +40,12 @@ export const piazzaApiResponse = { 'wcag-2.1', 'lti', ], - documentation_urls: { + external_links: { learn_more: '', - configuration_documentation: '', - documentation: '', - accessibility_documentation: '', - email_id: '', + configuration: '', + general: '', + accessibility: '', + contact_email: '', }, }, }, @@ -91,12 +91,12 @@ export const legacyApiResponse = { 'embedded-course-sections', 'wcag-2.1', ], - documentation_urls: { + external_links: { learn_more: '', - configuration_documentation: '', - documentation: '', - accessibility_documentation: '', - email_id: '', + configuration: '', + general: '', + accessibility: '', + contact_email: '', }, }, piazza: { @@ -107,12 +107,12 @@ export const legacyApiResponse = { 'wcag-2.1', 'lti', ], - documentation_urls: { + external_links: { learn_more: '', - configuration_documentation: '', - documentation: '', - accessibility_documentation: '', - email_id: '', + configuration: '', + general: '', + accessibility: '', + contact_email: '', }, }, },