feat: suggested UI improvements from bugbash

[MICROBA-1743]

* Use `LearningHeader` component which can display the course information (title, org, number) in the header to the learner (matching existing design).
* Refactor components to introduce a `PageContainer` component in order to support displaying course information in header.
* Refactor the BulkEmailTool component to take advantage of the new course metadata context provider.
* Add tests for new `PageContainer` component.
* Fix existing tests due to refactor.
* Add "Email" label above recipient selection checkboxes to match existing design.

src/components/bulk-email-tool/BulkEmailTool.jsx

@@ -1,19 +1,18 @@
-import React, { useState, useEffect, useRef } from 'react';
+import React, { useRef } from 'react';
 import classnames from 'classnames';
 
 import { useParams } from 'react-router-dom';
-import { Spinner } from '@edx/paragon';
+
 import { ErrorPage } from '@edx/frontend-platform/react';
 import BulkEmailTaskManager from './bulk-email-task-manager/BulkEmailTaskManager';
-import Navigationtabs from '../navigation-tabs/NavigationTabs';
-import { getCohorts, getCourseHomeCourseMetadata } from './data/api';
+import NavigationTabs from '../navigation-tabs/NavigationTabs';
 import useMobileResponsive from '../../utils/useMobileResponsive';
 import BulkEmailForm from './bulk-email-form';
+import { CourseMetadataContext } from '../page-container/PageContainer';
 
 export default function BulkEmailTool() {
   const { courseId } = useParams();
 
-  const [courseMetadata, setCourseMetadata] = useState();
   const isMobile = useMobileResponsive();
   const textEditorRef = useRef();
 
@@ -23,58 +22,23 @@ export default function BulkEmailTool() {
     }
   };
 
-  useEffect(() => {
-    async function fetchTabData() {
-      let metadataResponse;
-      let cohortsResponse;
-      try {
-        metadataResponse = await getCourseHomeCourseMetadata(courseId);
-        cohortsResponse = await getCohorts(courseId);
-      } catch (e) {
-        setCourseMetadata({
-          isStaff: false,
-          tabs: [],
-          cohorts: [],
-        });
-        return;
-      }
-      const { tabs, is_staff: isStaff } = metadataResponse;
-      const { cohorts } = cohortsResponse;
-      setCourseMetadata({
-        isStaff,
-        tabs: [...tabs],
-        cohorts: cohorts.map(({ name }) => name),
-      });
-    }
-    fetchTabData();
-  }, []);
-
-  if (courseMetadata) {
-    return courseMetadata.isStaff ? (
-      <div>
-        <Navigationtabs courseId={courseId} tabData={courseMetadata.tabs} />
-        <div className={classnames({ 'border border-primary-200': !isMobile })}>
-          <div className="row">
-            <BulkEmailForm courseId={courseId} cohorts={courseMetadata.cohorts} editorRef={textEditorRef} />
-          </div>
-          <div className="row">
-            <BulkEmailTaskManager courseId={courseId} copyTextToEditor={copyTextToEditor} />
+  return (
+    <CourseMetadataContext.Consumer>
+      {(courseMetadata) => (courseMetadata.isStaff ? (
+        <div>
+          <NavigationTabs courseId={courseId} tabData={courseMetadata.tabs} />
+          <div className={classnames({ 'border border-primary-200': !isMobile })}>
+            <div className="row">
+              <BulkEmailForm courseId={courseId} cohorts={courseMetadata.cohorts} editorRef={textEditorRef} />
+            </div>
+            <div className="row">
+              <BulkEmailTaskManager courseId={courseId} copyTextToEditor={copyTextToEditor} />
+            </div>
           </div>
         </div>
-      </div>
-    ) : (
-      <ErrorPage />
-    );
-  }
-  return (
-    <div className="d-flex justify-content-center">
-      <Spinner
-        animation="border"
-        variant="primary"
-        role="status"
-        screenreadertext="loading"
-        className="spinner-border spinner-border-lg text-primary p-5 m-5"
-      />
-    </div>
+      ) : (
+        <ErrorPage />
+      ))}
+    </CourseMetadataContext.Consumer>
   );
 }

src/components/bulk-email-tool/bulk-email-form/BulkEmailForm.jsx

@@ -115,6 +115,13 @@ export default function BulkEmailForm(props) {
         }}
       />
       <Form>
+        <p className="h2">
+          <FormattedMessage
+            id="bulk.email.tool.label"
+            defaultMessage="Email"
+            description="Tool label. Describes the function of the tool (to send email)."
+          />
+        </p>
         <BulkEmailRecipient
           selectedGroups={selectedRecipients}
           handleCheckboxes={onRecipientChange}

src/components/bulk-email-tool/test/BulkEmailTool.test.jsx

@@ -5,20 +5,15 @@ import React from 'react';
 import { Factory } from 'rosie';
 import { render, screen, cleanup } from '../../../setupTest';
 import BulkEmailTool from '../BulkEmailTool';
-import { getCourseHomeCourseMetadata, getCohorts } from '../data/api';
-import '../data/__factories__/courseMetadata.factory';
-import '../data/__factories__/cohort.factory';
+import { CourseMetadataContext } from '../../page-container/PageContainer';
+import '../../page-container/data/__factories__/cohort.factory';
+import '../../page-container/data/__factories__/courseMetadata.factory';
 
 jest.mock('../text-editor/TextEditor');
 jest.mock('../bulk-email-task-manager/api', () => ({
   getInstructorTasks: jest.fn(() => ({ tasks: {} })),
   getEmailTaskHistory: jest.fn(() => ({ tasks: {} })),
 }));
-jest.mock('../data/api', () => ({
-  __esModule: true,
-  getCourseHomeCourseMetadata: jest.fn(() => {}),
-  getCohorts: jest.fn(() => {}),
-}));
 jest.mock('react-router-dom', () => ({
   ...jest.requireActual('react-router-dom'),
   useParams: jest.fn(() => ({
@@ -29,20 +24,59 @@ jest.mock('react-router-dom', () => ({
 describe('BulkEmailTool', () => {
   beforeEach(() => jest.resetModules());
   afterEach(cleanup);
-  test('BulkEmailTool renders properly when given course metadata', async () => {
-    const courseMetadata = Factory.build('courseMetadata');
-    const cohorts = { cohorts: [Factory.build('cohort'), Factory.build('cohort')] };
-    getCourseHomeCourseMetadata.mockImplementation(() => courseMetadata);
-    getCohorts.mockImplementation(() => cohorts);
-    render(<BulkEmailTool />);
+
+  /**
+   * Utility function to munge course data in the form the components expect.
+   *
+   */
+  function buildCourseMetadata(cohortData, courseData) {
+    const {
+      org, number, title, tabs, is_staff: isStaff,
+    } = courseData;
+    const { cohorts } = cohortData;
+
+    return {
+      org,
+      number,
+      title,
+      isStaff,
+      tabs: [...tabs],
+      cohorts: cohorts.map(({ name }) => name),
+    };
+  }
+
+  /**
+   * Function that wraps component under test in a context provider. This allows us to make the data needed for testing
+   * availble to the component under test.
+   */
+  function renderBulkEmailTool(courseMetadata) {
+    return render(
+      <CourseMetadataContext.Provider value={courseMetadata}>
+        <BulkEmailTool />
+      </CourseMetadataContext.Provider>,
+    );
+  }
+
+  test('BulkEmailTool renders properly when given course metadata through context', async () => {
+    const cohorts = { cohorts: [] };
+    const courseInfo = Factory.build('courseMetadata');
+    const courseMetadata = buildCourseMetadata(cohorts, courseInfo);
+    renderBulkEmailTool(courseMetadata);
+    // verify all tab data expected is displayed within our component
     expect(await screen.findByText('Course')).toBeTruthy();
+    expect(await screen.findByText('Discussion')).toBeTruthy();
+    expect(await screen.findByText('Wiki')).toBeTruthy();
+    expect(await screen.findByText('Progress')).toBeTruthy();
+    expect(await screen.findByText('Instructor')).toBeTruthy();
+    expect(await screen.findByText('Dates')).toBeTruthy();
   });
+
   test('BulkEmailTool renders error page on no staff user', async () => {
-    const courseMetadata = Factory.build('courseMetadata', { is_staff: false });
-    const cohorts = { cohorts: [Factory.build('cohort'), Factory.build('cohort')] };
-    getCourseHomeCourseMetadata.mockImplementation(() => courseMetadata);
-    getCohorts.mockImplementation(() => cohorts);
-    render(<BulkEmailTool />);
+    const cohorts = { cohorts: [] };
+    const courseInfo = Factory.build('courseMetadata', { is_staff: false });
+    const courseMetadata = buildCourseMetadata(cohorts, courseInfo);
+    renderBulkEmailTool(courseMetadata);
+    // verify error page is displayed for user without staff permissions
     expect(
       await screen.findByText('An unexpected error occurred. Please click the button below to refresh the page.'),
     ).toBeTruthy();

src/components/page-container/PageContainer.jsx

@@ -0,0 +1,87 @@
+import React, { useState, useEffect } from 'react';
+import PropTypes from 'prop-types';
+import { useParams } from 'react-router-dom';
+
+import { LearningHeader as Header } from '@edx/frontend-component-header';
+import Footer from '@edx/frontend-component-footer';
+import { Spinner } from '@edx/paragon';
+
+import { getCohorts, getCourseHomeCourseMetadata } from './data/api';
+
+export const CourseMetadataContext = React.createContext();
+
+export default function PageContainer(props) {
+  const { children } = props;
+  const { courseId } = useParams();
+
+  const [courseMetadata, setCourseMetadata] = useState();
+
+  useEffect(() => {
+    async function fetchCourseMetadata() {
+      let metadataResponse;
+      let cohortsResponse;
+
+      try {
+        metadataResponse = await getCourseHomeCourseMetadata(courseId);
+        cohortsResponse = await getCohorts(courseId);
+      } catch (e) {
+        setCourseMetadata({
+          org: '',
+          number: '',
+          title: '',
+          isStaff: false,
+          tabs: [],
+          cohorts: [],
+        });
+        return;
+      }
+
+      const {
+        org, number, title, tabs, is_staff: isStaff,
+      } = metadataResponse;
+      const { cohorts } = cohortsResponse;
+
+      setCourseMetadata({
+        org,
+        number,
+        title,
+        isStaff,
+        tabs: [...tabs],
+        cohorts: cohorts.map(({ name }) => name),
+      });
+    }
+    fetchCourseMetadata();
+  }, []);
+
+  if (courseMetadata) {
+    return (
+      <CourseMetadataContext.Provider value={courseMetadata}>
+        <>
+          <Header
+            courseOrg={courseMetadata.org}
+            courseNumber={courseMetadata.number}
+            courseTitle={courseMetadata.title}
+          />
+          {children}
+          <Footer />
+        </>
+      </CourseMetadataContext.Provider>
+    );
+  }
+
+  return (
+    <div className="d-flex justify-content-center">
+      <Spinner
+        animation="border"
+        variant="primary"
+        role="status"
+        screenreadertext="loading"
+        className="spinner-border spinner-border-lg text-primary p-5 m-5"
+      />
+    </div>
+  );
+}
+
+PageContainer.propTypes = {
+  children: PropTypes.node.isRequired,
+};

src/components/page-container/data/__factories__/courseMetadata.factory.js

@@ -9,6 +9,7 @@ export default Factory.define('courseMetadata')
     original_user_is_staff: false,
     number: 'DemoX',
     org: 'edX',
+    title: 'Demonstration Course',
     verified_mode: {
       upgrade_url: 'test',
       price: 10,

src/components/page-container/data/api.js

@@ -1,14 +1,8 @@
 import { getConfig } from '@edx/frontend-platform';
 import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
 
-const InstructorApiBaseUrl = `${getConfig().LMS_BASE_URL}/api/instructor/v1`;
 const courseHomeBaseUrl = `${getConfig().LMS_BASE_URL}/api/course_home/v1/course_metadata`;
 
-export default async function getInstructorData(courseId) {
-  const InstructorApiUrl = `${InstructorApiBaseUrl}/tasks/${courseId} `;
-  return getAuthenticatedHttpClient().get(InstructorApiUrl);
-}
-
 export async function getCourseHomeCourseMetadata(courseId) {
   const courseHomeMetadataUrl = `${courseHomeBaseUrl}/${courseId}`;
   const { data } = await getAuthenticatedHttpClient().get(courseHomeMetadataUrl);

src/components/page-container/test/PageContainer.test.jsx

@@ -0,0 +1,64 @@
+/**
+ * @jest-environment jsdom
+ */
+import React from 'react';
+import { Factory } from 'rosie';
+import {
+  act, cleanup, render, screen,
+} from '../../../setupTest';
+
+import PageContainer from '../PageContainer';
+import { getCohorts, getCourseHomeCourseMetadata } from '../data/api';
+import '../data/__factories__/cohort.factory';
+import '../data/__factories__/courseMetadata.factory';
+
+jest.mock('../data/api', () => ({
+  __esModule: true,
+  getCohorts: jest.fn(() => {}),
+  getCourseHomeCourseMetadata: jest.fn(() => {}),
+}));
+jest.mock('react-router-dom', () => ({
+  ...jest.requireActual('react-router-dom'),
+  useParams: jest.fn(() => ({
+    courseId: 'test-course-id',
+  })),
+}));
+
+describe('PageContainer', () => {
+  beforeEach(() => jest.resetModules());
+  afterEach(cleanup);
+
+  test('PageContainer renders properly when given course metadata', async () => {
+    await act(async () => {
+      const cohorts = { cohorts: [Factory.build('cohort'), Factory.build('cohort')] };
+      const courseMetadata = Factory.build('courseMetadata');
+
+      getCohorts.mockImplementation(() => cohorts);
+      getCourseHomeCourseMetadata.mockImplementation(() => courseMetadata);
+
+      render(<PageContainer />);
+
+      // Look for the org, title, and number of the course, which should be displayed in the Header.
+      expect(await screen.findByText(`${courseMetadata.org} ${courseMetadata.number}`)).toBeTruthy();
+      expect(await screen.findByText(courseMetadata.title)).toBeTruthy();
+    });
+  });
+
+  test('PageContainer renders children nested within it.', async () => {
+    await act(async () => {
+      const cohorts = { cohorts: [Factory.build('cohort'), Factory.build('cohort')] };
+      const courseMetadata = Factory.build('courseMetadata');
+
+      getCohorts.mockImplementation(() => cohorts);
+      getCourseHomeCourseMetadata.mockImplementation(() => courseMetadata);
+
+      render(
+        <PageContainer>
+          <span>Test Text</span>
+        </PageContainer>,
+      );
+
+      expect(await screen.findByText('Test Text')).toBeTruthy();
+    });
+  });
+});

src/index.jsx

@@ -7,25 +7,28 @@ import {
 import { AppProvider, AuthenticatedPageRoute, ErrorPage } from '@edx/frontend-platform/react';
 import ReactDOM from 'react-dom';
 
-import Header, { messages as headerMessages } from '@edx/frontend-component-header';
-import Footer, { messages as footerMessages } from '@edx/frontend-component-footer';
+import { messages as headerMessages } from '@edx/frontend-component-header';
+import { messages as footerMessages } from '@edx/frontend-component-footer';
 
 import { Switch } from 'react-router-dom';
 import appMessages from './i18n';
 
 import './index.scss';
 import BulkEmailTool from './components/bulk-email-tool';
+import PageContainer from './components/page-container/PageContainer';
 
 subscribe(APP_READY, () => {
   ReactDOM.render(
     <AppProvider>
-      <Header />
-      <div className="container">
+      <div className="pb-3 container">
         <Switch>
-          <AuthenticatedPageRoute path="/courses/:courseId/bulk_email" component={BulkEmailTool} />
+          <AuthenticatedPageRoute path="/courses/:courseId/bulk_email">
+            <PageContainer>
+              <BulkEmailTool />
+            </PageContainer>
+          </AuthenticatedPageRoute>
         </Switch>
       </div>
-      <Footer />
     </AppProvider>,
     document.getElementById('root'),
   );