feat: update feature table UI (#198)

* feat: update feature table * test: fix failed feature table test cases * refactor: features name is feature table * refactor: update provider name
2021-10-09 00:23:17 +05:00
parent 3757c71446
commit 1c5fb4c646
7 changed files with 115 additions and 50 deletions
--- a/src/pages-and-resources/discussions/app-list/FeaturesTable.jsx
+++ b/src/pages-and-resources/discussions/app-list/FeaturesTable.jsx
@@ -3,43 +3,73 @@ import PropTypes from 'prop-types';
 import { Remove, Check } from '@edx/paragon/icons';
 import { DataTable } from '@edx/paragon';
 import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
+import _ from 'lodash';
+
 import messages from './messages';
+import { FEATURE_TYPES } from '../data/constants';
 import './FeaturesTable.scss';

 function FeaturesTable({ apps, features, intl }) {
-  return (
-    <DataTable
-      itemCount={features.length}
-      data={features.map(feature => {
-        const appCheckmarkCells = {};
-        // DataTable wants 'data' to be an array of objects where each property of an object
-        // represents a cell in that row, identified by its key.
-        apps.forEach(app => {
-          // If our app's set of feature Ids includes this feature, return a checkmark.
-          // i.e, if this app has the current feature, check it!
-          appCheckmarkCells[app.id] = (
-            <div
-              className="text-center"
-              key={`${app.id}&${feature.id}`}
-              data-testid={`${app.id}-${feature.id.replaceAll('.', '-')}`}
-            >
-              {
+  const {
+    basic, partial, full, common,
+  } = _.groupBy(features, (feature) => feature.featureSupportType);
+
+  const createRow = (feature) => {
+    const appCheckmarkCells = {};
+    // DataTable wants 'data' to be    an array of objects where each property of an object
+    // represents a cell in that row, identified by its key.
+    apps.forEach(app => {
+      // If our app's set of feature Ids includes this feature, return a checkmark.
+      // i.e, if this app has the current feature, check it!
+      if (FEATURE_TYPES.includes(feature)) {
+        appCheckmarkCells[app.id] = <div key={feature} data-testid={feature} />;
+      } else {
+        appCheckmarkCells[app.id] = (
+          <div
+            className="text-center"
+            key={`${app.id}&${feature.id}`}
+            data-testid={`${app.id}-${feature.id.replaceAll('.', '-')}`}
+          >
+            {
                app.featureIds.includes(feature.id)
                  ? <Check id="check-icon" className="text-success-500" />
                  : <Remove id="remove-icon" className="text-light-700" />
              }
-            </div>
-          );
-        });
+          </div>
+        );
+      }
+    });

-        return {
-          feature: intl.formatMessage(messages[`featureName-${feature.id}`]), // 'feature' is the identifier for cells in the first column.
-          // This is spreading the app IDs from appCheckmarkCells into the return array, creating
-          // one object with 'feature' and the app.id keys from above.  The values are the JSX
-          // above with the font awesome checkmarks in 'em
-          ...appCheckmarkCells,
-        };
-      })}
+    return {
+      feature: FEATURE_TYPES.includes(feature) ? (
+        <span className="font-weight-bold">
+          {intl.formatMessage(messages[`featureType-${feature}`])}
+        </span>
+      )
+        : intl.formatMessage(messages[`featureName-${feature.id}`]),
+      // 'feature' is the identifier for cells in the first column.
+      // This is spreading the app IDs from appCheckmarkCells into the return array, creating
+      // one object with 'feature' and the app.id keys from above.  The values are the JSX
+      // above with the font awesome checkmarks in 'em
+      ...appCheckmarkCells,
+    };
+  };
+
+  return (
+    <DataTable
+      itemCount={features.length}
+      data={
+        [
+          { ...createRow('basic') },
+          ...basic.map((feature) => createRow(feature)),
+          { ...createRow('partial') },
+          ...partial.map((feature) => createRow(feature)),
+          { ...createRow('full') },
+          ...full.map((feature) => createRow(feature)),
+          { ...createRow('common') },
+          ...common.map((feature) => createRow(feature)),
+        ]
+      }
      columns={[
        {
          Header: '',
--- a/src/pages-and-resources/discussions/app-list/FeaturesTable.test.jsx
+++ b/src/pages-and-resources/discussions/app-list/FeaturesTable.test.jsx
@@ -25,10 +25,10 @@ describe('FeaturesTable', () => {
      }];

    features = [
-      { id: 'lti-basic-configuration' },
-      { id: 'wcag-2.1' },
-      { id: 'discussion-page' },
-      { id: 'embedded-course-sections' },
+      { id: 'discussion-page', featureSupportType: 'basic' },
+      { id: 'embedded-course-sections', featureSupportType: 'full' },
+      { id: 'wcag-2.1', featureSupportType: 'partial' },
+      { id: 'lti-basic-configuration', featureSupportType: 'common' },
    ];

    const wrapper = render(
@@ -49,7 +49,7 @@ describe('FeaturesTable', () => {
  });

  test('displays a row for each available feature', () => {
-    expect(container.querySelectorAll('tbody > tr')).toHaveLength(features.length);
+    expect(container.querySelectorAll('tbody > tr')).toHaveLength(8);
  });

  test('apps columns receive a check for each feature they support', () => {
--- a/src/pages-and-resources/discussions/app-list/messages.js
+++ b/src/pages-and-resources/discussions/app-list/messages.js
@@ -105,8 +105,8 @@ const messages = defineMessages({
  // Ed Discuss
  'appName-ed-discuss': {
    id: 'authoring.discussions.appList.appName-ed-discuss',
-    defaultMessage: 'Ed Discuss',
-    description: 'The name of the Ed Discuss app.',
+    defaultMessage: 'Ed Discussion',
+    description: 'The name of the Ed Discussion app.',
  },
  'appDescription-ed-discuss': {
    id: 'authoring.discussions.appList.appDescription-ed-discus',
@@ -186,12 +186,12 @@ const messages = defineMessages({
  },
  'featureName-lti-advanced-sharing-mode': {
    id: 'authoring.discussions.featureName-lti-advanced-sharing-mode',
-    defaultMessage: 'LTI advanced sharing mode',
+    defaultMessage: 'LTI advanced sharing',
    description: 'The name of a discussions feature.',
  },
  'featureName-lti-basic-configuration': {
    id: 'authoring.discussions.featureName-lti-basic-configuration',
-    defaultMessage: 'LTI basic configuration',
+    defaultMessage: 'Basic configuration',
    description: 'The name of a discussions feature.',
  },
  'featureName-primary-discussion-app-experience': {
@@ -206,7 +206,7 @@ const messages = defineMessages({
  },
  'featureName-report/flag-content-to-moderators': {
    id: 'authoring.discussions.featureName-report/flag-content-to-moderators',
-    defaultMessage: 'Report/flag content to moderators',
+    defaultMessage: 'Report content to moderators',
    description: 'The name of a discussions feature.',
  },
  'featureName-research-data-events': {
@@ -234,6 +234,26 @@ const messages = defineMessages({
    defaultMessage: 'WCAG 2.0 support',
    description: 'The name of a discussions feature.',
  },
+  'featureType-basic': {
+    id: 'authoring.discussions.basic-support',
+    defaultMessage: 'Basic support',
+    description: 'The type of a discussions feature.',
+  },
+  'featureType-partial': {
+    id: 'authoring.discussions.partial-support',
+    defaultMessage: 'Partial support',
+    description: 'The type of a discussions feature.',
+  },
+  'featureType-full': {
+    id: 'authoring.discussions.full-support',
+    defaultMessage: 'Full support',
+    description: 'The type of a discussions feature.',
+  },
+  'featureType-common': {
+    id: 'authoring.discussions.common-support',
+    defaultMessage: 'Commonly requested',
+    description: 'The type of a discussions feature.',
+  },
 });

 export default messages;
--- a/src/pages-and-resources/discussions/data/api.js
+++ b/src/pages-and-resources/discussions/data/api.js
@@ -1,4 +1,4 @@
-import { getConfig } from '@edx/frontend-platform';
+import { getConfig, camelCaseObject } from '@edx/frontend-platform';
 import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
 import _ from 'lodash';
 import moment from 'moment';
@@ -76,6 +76,15 @@ function normalizePluginConfig(data) {
  };
 }

+function normalizeFeatures(data, apps) {
+  if (!data || data.length < 1) {
+    return [];
+  }
+
+  return camelCaseObject(data.filter((feature) => (
+    apps.map(app => app.featureIds.includes(feature.id)).some((supported) => supported))));
+}
+
 function normalizeApps(data) {
  const apps = Object.entries(data.providers.available).map(([key, app]) => ({
    id: key,
@@ -94,9 +103,7 @@ function normalizeApps(data) {
  return {
    courseId: data.context_key,
    enabled: data.enabled,
-    features: data.features.map(id => ({
-      id,
-    })),
+    features: normalizeFeatures(data.features, apps),
    appConfig: {
      id: data.providers.active,
      ...normalizePluginConfig(data.plugin_configuration),
--- a/src/pages-and-resources/discussions/data/constants.js
+++ b/src/pages-and-resources/discussions/data/constants.js
@@ -24,3 +24,5 @@ export const today = moment();
 export const active = [today.format('YYYY-MM-DDTHH:mm'), today.add(5, 'hours').format('YYYY-MM-DDTHH:mm')];
 export const upcoming = [today.add(2, 'days').format('YYYY-MM-DD'), today.add(5, 'days').format('YYYY-MM-DD')];
 export const complete = [today.subtract(7, 'days').format('YYYY-MM-DD'), today.subtract(5, 'days').format('YYYY-MM-DD')];
+
+export const FEATURE_TYPES = ['basic', 'partial', 'full', 'common'];
--- a/src/pages-and-resources/discussions/data/redux.test.js
+++ b/src/pages-and-resources/discussions/data/redux.test.js
@@ -17,15 +17,21 @@ const pagesAndResourcesPath = `/course/${courseId}/pages-and-resources`;
 const featuresState = {
  'discussion-page': {
    id: 'discussion-page',
+    featureSupportType: 'basic',
  },
  'embedded-course-sections': {
    id: 'embedded-course-sections',
+    featureSupportType: 'full',
  },
  'wcag-2.1': {
    id: 'wcag-2.1',
+    featureSupportType: 'partial',
+
  },
  'lti-basic-configuration': {
    id: 'lti-basic-configuration',
+    featureSupportType: 'common',
+
  },
 };

--- a/src/pages-and-resources/discussions/factories/mockApiResponses.js
+++ b/src/pages-and-resources/discussions/factories/mockApiResponses.js
@@ -3,10 +3,10 @@ export const generatePiazzaApiResponse = (piazzaAdminOnlyConfig = false) => ({
  enabled: true,
  provider_type: 'piazza',
  features: [
-    'discussion-page',
-    'embedded-course-sections',
-    'wcag-2.1',
-    'lti-basic-configuration',
+    { id: 'discussion-page', feature_support_type: 'basic' },
+    { id: 'embedded-course-sections', feature_support_type: 'full' },
+    { id: 'wcag-2.1', feature_support_type: 'partial' },
+    { id: 'lti-basic-configuration', feature_support_type: 'common' },
  ],
  lti_configuration: {
    lti_1p1_client_key: 'client_key_123',
@@ -79,10 +79,10 @@ export const legacyApiResponse = {
  enabled: true,
  provider_type: 'legacy',
  features: [
-    'discussion-page',
-    'embedded-course-sections',
-    'wcag-2.1',
-    'lti-basic-configuration',
+    { id: 'discussion-page', feature_support_type: 'basic' },
+    { id: 'embedded-course-sections', feature_support_type: 'full' },
+    { id: 'wcag-2.1', feature_support_type: 'partial' },
+    { id: 'lti-basic-configuration', feature_support_type: 'common' },
  ],
  lti_configuration: {},
  plugin_configuration: {