feat: show transparent overlay over rendered chart while re-fetching …

…data
openedx · Sep 5, 2024 · 3f786ef · 3f786ef
1 parent 7b5e690
commit 3f786ef
Show file tree

Hide file tree

Showing 14 changed files with 133 additions and 75 deletions.
diff --git a/src/components/AdvanceAnalyticsV2/ProgressOverlay.jsx b/src/components/AdvanceAnalyticsV2/ProgressOverlay.jsx
@@ -14,7 +14,7 @@ const ProgressOverlay = ({ isError, message }) => (
 
 ProgressOverlay.propTypes = {
   isError: PropTypes.bool.isRequired,
-  message: PropTypes.string.isRequired,
+  message: PropTypes.string,
 };
 
 export default ProgressOverlay;
diff --git a/src/components/AdvanceAnalyticsV2/charts/ChartWrapper.jsx b/src/components/AdvanceAnalyticsV2/charts/ChartWrapper.jsx
@@ -1,41 +1,45 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import ProgressOverlay from '../ProgressOverlay';
+import classNames from 'classnames';
+import {
+  Spinner,
+} from '@openedx/paragon';
 import ScatterChart from './ScatterChart';
 import LineChart from './LineChart';
 import BarChart from './BarChart';
+import EmptyChart from './EmptyChart';
 
 const ChartWrapper = ({
-  isLoading,
+  isFetching,
   isError,
   chartType,
   chartProps,
   loadingMessage,
 }) => {
-  if (isLoading || isError) {
-    return (
-      <ProgressOverlay
-        isError={isError}
-        message={loadingMessage}
-      />
-    );
+  if (isError) {
+    return <EmptyChart />;
   }
 
-  const renderChart = () => {
-    const chartMap = {
-      ScatterChart: <ScatterChart {...chartProps} />,
-      LineChart: <LineChart {...chartProps} />,
-      BarChart: <BarChart {...chartProps} />,
-    };
-
-    return chartMap[chartType];
+  const chartMap = {
+    ScatterChart: <ScatterChart {...chartProps} />,
+    LineChart: <LineChart {...chartProps} />,
+    BarChart: <BarChart {...chartProps} />,
   };
 
-  return renderChart();
+  return (
+    <div className={classNames('analytics-chart-container', { chartType }, { 'is-fetching': isFetching })}>
+      {isFetching && (
+        <div className="spinner-centered">
+          <Spinner animation="border" screenReaderText={loadingMessage} />
+        </div>
+      )}
+      {chartProps.data && chartMap[chartType]}
+    </div>
+  );
 };
 
 ChartWrapper.propTypes = {
-  isLoading: PropTypes.bool.isRequired,
+  isFetching: PropTypes.bool.isRequired,
   isError: PropTypes.bool.isRequired,
   chartType: PropTypes.oneOf(['ScatterChart', 'LineChart', 'BarChart']).isRequired,
   chartProps: PropTypes.object.isRequired,

diff --git a/src/components/AdvanceAnalyticsV2/data/hooks.js b/src/components/AdvanceAnalyticsV2/data/hooks.js
@@ -24,8 +24,8 @@ export const useEnterpriseAnalyticsData = ({
       key,
       requestOptions,
     ),
-    staleTime: 0.5 * (1000 * 60 * 60), // 30 minutes. Length of time before your data becomes stale
-    cacheTime: 0.75 * (1000 * 60 * 60), // 45 minutes. Length of time before inactive data gets removed from the cache
+    staleTime: 0.5 * (1000 * 60 * 60), // 30 minutes. The time in milliseconds after data is considered stale.
+    cacheTime: 0.75 * (1000 * 60 * 60), // 45 minutes. Cache data will be garbage collected after this duration.
     keepPreviousData: true,
     ...queryOptions,
   });

diff --git a/src/components/AdvanceAnalyticsV2/data/hooks.test.jsx b/src/components/AdvanceAnalyticsV2/data/hooks.test.jsx
@@ -68,7 +68,7 @@ describe('useEnterpriseAnalyticsData', () => {
 
     expect(result.current).toEqual(
       expect.objectContaining({
-        isLoading: true,
+        isFetching: true,
         error: null,
         data: undefined,
       }),
@@ -89,7 +89,7 @@ describe('useEnterpriseAnalyticsData', () => {
       },
     );
     expect(result.current).toEqual(expect.objectContaining({
-      isLoading: false,
+      isFetching: false,
       error: null,
       data: camelCaseObject(mockAnalyticsCompletionsChartsData),
     }));
@@ -115,7 +115,7 @@ describe('useEnterpriseAnalyticsData', () => {
 
     expect(result.current).toEqual(
       expect.objectContaining({
-        isLoading: true,
+        isFetching: true,
         error: null,
         data: undefined,
       }),
@@ -136,7 +136,7 @@ describe('useEnterpriseAnalyticsData', () => {
       },
     );
     expect(result.current).toEqual(expect.objectContaining({
-      isLoading: false,
+      isFetching: false,
       error: null,
       data: camelCaseObject(mockAnalyticsLeaderboardTableData),
     }));

diff --git a/src/components/AdvanceAnalyticsV2/styles/index.scss b/src/components/AdvanceAnalyticsV2/styles/index.scss
@@ -1,3 +1,27 @@
 .analytics-stat-number {
     font-size: 2.5rem;
 }
+
+.analytics-chart-container {
+    position: relative;
+    min-height: 40vh;
+
+    &.is-fetching::before {
+        content: "";
+        position: absolute;
+        top: 0;
+        left: 0;
+        width: 100%;
+        height: 100%;
+        background-color: rgba($white, .7);
+        z-index: 1;
+    }
+
+    .spinner-centered {
+        position: absolute;
+        top: 50%;
+        left: 50%;
+        transform: translate(-50%, -50%);
+        z-index: 2;
+    }
+}
diff --git a/src/components/AdvanceAnalyticsV2/tabs/AnalyticsTable.jsx b/src/components/AdvanceAnalyticsV2/tabs/AnalyticsTable.jsx
@@ -21,7 +21,7 @@ const AnalyticsTable = ({
   const [currentPage, setCurrentPage] = useState(0);
 
   const {
-    isLoading, data, isPreviousData,
+    isFetching, data,
   } = useEnterpriseAnalyticsData({
     enterpriseCustomerUUID: enterpriseId,
     key: analyticsDataTableKeys[name],
@@ -55,7 +55,7 @@ const AnalyticsTable = ({
           isDownloadCSV={enableCSVDownload}
         />
         <DataTable
-          isLoading={isLoading || isPreviousData}
+          isLoading={isFetching}
           isPaginated
           manualPagination
           initialState={{

diff --git a/src/components/AdvanceAnalyticsV2/tabs/Completions.jsx b/src/components/AdvanceAnalyticsV2/tabs/Completions.jsx
@@ -13,7 +13,7 @@ const Completions = ({
   const intl = useIntl();
 
   const {
-    isLoading, isError, data,
+    isFetching, isError, data,
   } = useEnterpriseAnalyticsData({
     enterpriseCustomerUUID: enterpriseId,
     key: ANALYTICS_TABS.COMPLETIONS,
@@ -47,7 +47,7 @@ const Completions = ({
           isDownloadCSV
         />
         <ChartWrapper
-          isLoading={isLoading}
+          isFetching={isFetching}
           isError={isError}
           chartType="LineChart"
           chartProps={{
@@ -89,7 +89,7 @@ const Completions = ({
           isDownloadCSV
         />
         <ChartWrapper
-          isLoading={isLoading}
+          isFetching={isFetching}
           isError={isError}
           chartType="BarChart"
           chartProps={{
@@ -134,7 +134,7 @@ const Completions = ({
           isDownloadCSV
         />
         <ChartWrapper
-          isLoading={isLoading}
+          isFetching={isFetching}
           isError={isError}
           chartType="BarChart"
           chartProps={{

diff --git a/src/components/AdvanceAnalyticsV2/tabs/Completions.test.jsx b/src/components/AdvanceAnalyticsV2/tabs/Completions.test.jsx
@@ -11,7 +11,7 @@ import Completions from './Completions';
 import { queryClient } from '../../test/testUtils';
 import EnterpriseDataApiService from '../../../data/services/EnterpriseDataApiService';
 
-const mockAnalyticsData = {
+const mockAnalyticsTableData = {
   next: null,
   previous: null,
   count: 2,
@@ -32,12 +32,15 @@ const mockAnalyticsData = {
     },
   ],
 };
+const mockAnalyticsChartsData = {
+  completionsOverTime: [],
+  topCoursesByCompletions: [],
+  topSubjectsByCompletions: [],
+};
 
 jest.spyOn(EnterpriseDataApiService, 'fetchAdminAnalyticsData');
 const axiosMock = new MockAdapter(axios);
 getAuthenticatedHttpClient.mockReturnValue(axios);
-axiosMock.onAny().reply(200);
-axios.get = jest.fn(() => Promise.resolve({ data: mockAnalyticsData }));
 
 jest.mock('../charts/LineChart', () => {
   const MockedLineChart = () => <div>Mocked LineChart</div>;
@@ -50,7 +53,14 @@ jest.mock('../charts/BarChart', () => {
 });
 
 describe('Completions Component', () => {
+  afterEach(() => {
+    axiosMock.reset();
+  });
+
   test('renders all charts correctly', async () => {
+    axiosMock.onGet(/\/completions\/stats(\?.*)/).reply(200, mockAnalyticsChartsData);
+    axiosMock.onGet(/\/completions(\?.*)/).reply(200, mockAnalyticsTableData);
+
     const { container } = render(
       <QueryClientProvider client={queryClient()}>
         <IntlProvider locale="en">
@@ -103,7 +113,7 @@ describe('Completions Component', () => {
 
       // ensure the correct number of rows are rendered (including header row)
       const rows = screen.getAllByRole('row');
-      expect(rows).toHaveLength(mockAnalyticsData.count + 1); // +1 for header row
+      expect(rows).toHaveLength(mockAnalyticsTableData.count + 1); // +1 for header row
 
       // validate header row
       const columnHeaders = within(rows[0]).getAllByRole('columnheader');
@@ -112,7 +122,7 @@ describe('Completions Component', () => {
       });
 
       // validate content of each data row
-      mockAnalyticsData.results.forEach((user, index) => {
+      mockAnalyticsTableData.results.forEach((user, index) => {
         const rowCells = within(rows[index + 1]).getAllByRole('cell'); // Skip header row
         expect(rowCells[0]).toHaveTextContent(user.email);
         expect(rowCells[1]).toHaveTextContent(user.course_title);
@@ -124,10 +134,9 @@ describe('Completions Component', () => {
   test('renders charts with correct loading messages', () => {
     jest.mock('../data/hooks', () => ({
       useEnterpriseAnalyticsTableData: jest.fn().mockReturnValue({
-        isLoading: true,
+        isFetching: true,
         data: null,
         isError: false,
-        isFetching: false,
         error: null,
       }),
     }));

diff --git a/src/components/AdvanceAnalyticsV2/tabs/Engagements.jsx b/src/components/AdvanceAnalyticsV2/tabs/Engagements.jsx
@@ -12,7 +12,7 @@ const Engagements = ({
 }) => {
   const intl = useIntl();
   const {
-    isLoading, isError, data,
+    isFetching, isError, data,
   } = useEnterpriseAnalyticsData({
     enterpriseCustomerUUID: enterpriseId,
     key: ANALYTICS_TABS.ENGAGEMENTS,
@@ -44,7 +44,7 @@ const Engagements = ({
           isDownloadCSV
         />
         <ChartWrapper
-          isLoading={isLoading}
+          isFetching={isFetching}
           isError={isError}
           chartType="LineChart"
           chartProps={{
@@ -84,7 +84,7 @@ const Engagements = ({
           isDownloadCSV
         />
         <ChartWrapper
-          isLoading={isLoading}
+          isFetching={isFetching}
           isError={isError}
           chartType="BarChart"
           chartProps={{
@@ -127,11 +127,11 @@ const Engagements = ({
           isDownloadCSV
         />
         <ChartWrapper
-          isLoading={isLoading}
+          isFetching={isFetching}
           isError={isError}
           chartType="BarChart"
           chartProps={{
-            data: data?.topCoursesByEngagement,
+            data: data?.topSubjectsByEngagement,
             xKey: 'courseSubject',
             yKey: 'count',
             colorKey: 'enrollType',

diff --git a/src/components/AdvanceAnalyticsV2/tabs/Engagements.test.jsx b/src/components/AdvanceAnalyticsV2/tabs/Engagements.test.jsx
@@ -11,7 +11,7 @@ import Engagements from './Engagements';
 import { queryClient } from '../../test/testUtils';
 import EnterpriseDataApiService from '../../../data/services/EnterpriseDataApiService';
 
-const mockAnalyticsData = {
+const mockAnalyticsTableData = {
   next: null,
   previous: null,
   count: 2,
@@ -34,12 +34,15 @@ const mockAnalyticsData = {
     },
   ],
 };
+const mockAnalyticsChartsData = {
+  engagementsOverTime: [],
+  topCoursesByEngagement: [],
+  topSubjectsByEngagement: [],
+};
 
 jest.spyOn(EnterpriseDataApiService, 'fetchAdminAnalyticsData');
 const axiosMock = new MockAdapter(axios);
 getAuthenticatedHttpClient.mockReturnValue(axios);
-axiosMock.onAny().reply(200);
-axios.get = jest.fn(() => Promise.resolve({ data: mockAnalyticsData }));
 
 jest.mock('../charts/LineChart', () => {
   const MockedLineChart = () => <div>Mocked LineChart</div>;
@@ -52,7 +55,14 @@ jest.mock('../charts/BarChart', () => {
 });
 
 describe('Engagements Component', () => {
-  test('renders all sections with correct classes and content', async () => {
+  afterEach(() => {
+    axiosMock.reset();
+  });
+
+  test('renders all charts correctly', async () => {
+    axiosMock.onGet(/\/engagements\/stats(\?.*)/).reply(200, mockAnalyticsChartsData);
+    axiosMock.onGet(/\/engagements(\?.*)/).reply(200, mockAnalyticsTableData);
+
     const { container } = render(
       <QueryClientProvider client={queryClient()}>
         <IntlProvider locale="en">
@@ -105,7 +115,7 @@ describe('Engagements Component', () => {
 
       // ensure the correct number of rows are rendered (including header row)
       const rows = screen.getAllByRole('row');
-      expect(rows).toHaveLength(mockAnalyticsData.count + 1); // +1 for header row
+      expect(rows).toHaveLength(mockAnalyticsTableData.count + 1); // +1 for header row
 
       // validate header row
       const columnHeaders = within(rows[0]).getAllByRole('columnheader');
@@ -114,7 +124,7 @@ describe('Engagements Component', () => {
       });
 
       // validate content of each data row
-      mockAnalyticsData.results.forEach((user, index) => {
+      mockAnalyticsTableData.results.forEach((user, index) => {
         const rowCells = within(rows[index + 1]).getAllByRole('cell'); // Skip header row
         expect(rowCells[0]).toHaveTextContent(user.email);
         expect(rowCells[1]).toHaveTextContent(user.course_title);
@@ -127,10 +137,9 @@ describe('Engagements Component', () => {
   test('renders charts with correct loading messages', () => {
     jest.mock('../data/hooks', () => ({
       useEnterpriseAnalyticsTableData: jest.fn().mockReturnValue({
-        isLoading: true,
+        isFetching: true,
         data: null,
         isError: false,
-        isFetching: false,
         error: null,
       }),
     }));