fix: update video page load order (#810)

* fix: asset card/row menu appearing for videos * fix: page load time * fix: video status messages
openedx · Jan 25, 2024 · 90bc242 · 90bc242
1 parent f8aa157
commit 90bc242
Show file tree

Hide file tree

Showing 15 changed files with 58 additions and 38 deletions.
diff --git a/src/data/constants.js b/src/data/constants.js
@@ -13,6 +13,7 @@ export const RequestStatus = /** @type {const} */ ({
   PENDING: 'pending',
   CLEAR: 'clear',
   PARTIAL: 'partial',
+  PARTIAL_FAILURE: 'partial failure',
   NOT_FOUND: 'not-found',
 });
 

diff --git a/src/files-and-videos/files-page/FilesPage.jsx b/src/files-and-videos/files-page/FilesPage.jsx
@@ -87,7 +87,7 @@ const FilesPage = ({
     id: 'activeStatus',
     Header: 'Active',
     accessor: 'activeStatus',
-    Cell: ({ row }) => ActiveColumn({ row }),
+    Cell: ({ row }) => ActiveColumn({ row, pageLoadStatus: loadingStatus }),
     Filter: CheckboxFilter,
     filter: 'exactTextCase',
     filterChoices: [

diff --git a/src/files-and-videos/files-page/FilesPage.test.jsx b/src/files-and-videos/files-page/FilesPage.test.jsx
@@ -496,7 +496,7 @@ describe('FilesAndUploads', () => {
           expect(screen.getByText('Error')).toBeVisible();
         });
 
-        expect(loadingStatus).toEqual(RequestStatus.PARTIAL);
+        expect(loadingStatus).toEqual(RequestStatus.PARTIAL_FAILURE);
         expect(screen.getByText('Failed to load remaining files.')).toBeVisible();
       });
 

diff --git a/src/files-and-videos/files-page/data/thunks.js b/src/files-and-videos/files-page/data/thunks.js
@@ -48,7 +48,7 @@ export function fetchAddtionalAsstets(courseId, totalCount) {
       } catch (error) {
         remainingAssetCount = 0;
         dispatch(updateErrors({ error: 'loading', message: 'Failed to load remaining files.' }));
-        dispatch(updateLoadingStatus({ status: RequestStatus.PARTIAL }));
+        dispatch(updateLoadingStatus({ status: RequestStatus.PARTIAL_FAILURE }));
       }
     }
   };

diff --git a/src/files-and-videos/generic/EditFileErrors.jsx b/src/files-and-videos/generic/EditFileErrors.jsx
@@ -19,7 +19,7 @@ const EditFileErrors = ({
     <ErrorAlert
       hideHeading={false}
       dismissError={() => resetErrors({ errorType: 'loading' })}
-      isError={loadingStatus === RequestStatus.FAILED || loadingStatus === RequestStatus.PARTIAL}
+      isError={loadingStatus === RequestStatus.FAILED || loadingStatus === RequestStatus.PARTIAL_FAILURE}
     >
       {intl.formatMessage(messages.errorAlertMessage, { message: errorMessages.loading })}
     </ErrorAlert>

diff --git a/src/files-and-videos/generic/FileMenu.jsx b/src/files-and-videos/generic/FileMenu.jsx
@@ -18,7 +18,7 @@ const FileMenu = ({
   openDeleteConfirmation,
   portableUrl,
   id,
-  wrapperType,
+  fileType,
   // injected
   intl,
 }) => (
@@ -32,7 +32,7 @@ const FileMenu = ({
       alt="file-menu-toggle"
     />
     <Dropdown.Menu>
-      {wrapperType === 'video' ? (
+      {fileType === 'video' ? (
         <Dropdown.Item
           onClick={() => navigator.clipboard.writeText(id)}
         >
@@ -81,7 +81,7 @@ FileMenu.propTypes = {
   openDeleteConfirmation: PropTypes.func.isRequired,
   portableUrl: PropTypes.string,
   id: PropTypes.string.isRequired,
-  wrapperType: PropTypes.string.isRequired,
+  fileType: PropTypes.string.isRequired,
   // injected
   intl: intlShape.isRequired,
 };

diff --git a/src/files-and-videos/generic/FileTable.jsx b/src/files-and-videos/generic/FileTable.jsx
@@ -166,6 +166,7 @@ const FileTable = ({
         thumbnailPreview,
         className,
         original,
+        fileType,
       }}
     />
   );
@@ -179,6 +180,7 @@ const FileTable = ({
       handleBulkDownload,
       handleOpenFileInfo,
       handleOpenDeleteConfirmation,
+      fileType,
     }),
   };
 

diff --git a/src/files-and-videos/generic/table-components/GalleryCard.jsx b/src/files-and-videos/generic/table-components/GalleryCard.jsx
@@ -19,6 +19,7 @@ const GalleryCard = ({
   handleOpenDeleteConfirmation,
   handleOpenFileInfo,
   thumbnailPreview,
+  fileType,
 }) => {
   const lockFile = () => {
     const { locked, id } = original;
@@ -38,7 +39,7 @@ const GalleryCard = ({
               openAssetInfo={() => handleOpenFileInfo(original)}
               portableUrl={original.portableUrl}
               id={original.id}
-              wrapperType={original.wrapperType}
+              fileType={fileType}
               onDownload={() => handleBulkDownload([{
                 original: {
                   id: original.id,
@@ -103,6 +104,7 @@ GalleryCard.propTypes = {
   handleOpenDeleteConfirmation: PropTypes.func.isRequired,
   handleOpenFileInfo: PropTypes.func.isRequired,
   thumbnailPreview: PropTypes.func.isRequired,
+  fileType: PropTypes.func.isRequired,
 };
 
 export default GalleryCard;
diff --git a/src/files-and-videos/generic/table-components/table-custom-columns/ActiveColumn.jsx b/src/files-and-videos/generic/table-components/table-custom-columns/ActiveColumn.jsx
@@ -4,10 +4,11 @@ import { isNil } from 'lodash';
 import { injectIntl, FormattedMessage } from '@edx/frontend-platform/i18n';
 import { Icon, Spinner } from '@edx/paragon';
 import { Check } from '@edx/paragon/icons';
+import { RequestStatus } from '../../../../data/constants';
 
-const ActiveColumn = ({ row }) => {
+const ActiveColumn = ({ row, pageLoadStatus }) => {
   const { usageLocations } = row.original;
-  if (isNil(usageLocations)) {
+  if (isNil(usageLocations) || pageLoadStatus !== RequestStatus.SUCCESSFUL) {
     return (
       <Spinner
         animation="border"
@@ -18,7 +19,7 @@ const ActiveColumn = ({ row }) => {
           <FormattedMessage
             id="authoring.loading"
             defaultMessage="Loading..."
-            description="Screen-reader message for when a page is loading."
+            description="Screen-reader message for when a active column is loading."
           />
         )}
       />
@@ -34,6 +35,7 @@ ActiveColumn.propTypes = {
       usageLocations: PropTypes.arrayOf(PropTypes.string).isRequired,
     }.isRequired,
   }.isRequired,
+  pageLoadStatus: PropTypes.string.isRequired,
 };
 
 export default injectIntl(ActiveColumn);
diff --git a/src/files-and-videos/generic/table-components/table-custom-columns/MoreInfoColumn.jsx b/src/files-and-videos/generic/table-components/table-custom-columns/MoreInfoColumn.jsx
@@ -20,6 +20,7 @@ const MoreInfoColumn = ({
   handleBulkDownload,
   handleOpenFileInfo,
   handleOpenDeleteConfirmation,
+  fileType,
   // injected
   intl,
 }) => {
@@ -31,7 +32,6 @@ const MoreInfoColumn = ({
     locked,
     portableUrl,
     id,
-    wrapperType,
     displayName,
     downloadLink,
   } = row.original;
@@ -54,7 +54,7 @@ const MoreInfoColumn = ({
         <Menu
           className="more-info-menu"
         >
-          {wrapperType === 'video' ? (
+          {fileType === 'video' ? (
             <MenuItem
               as={Button}
               variant="tertiary"
@@ -137,13 +137,13 @@ MoreInfoColumn.propTypes = {
       locked: PropTypes.bool,
       portableUrl: PropTypes.string,
       id: PropTypes.string.isRequired,
-      wrapperType: PropTypes.string,
     }.isRequired,
   }).isRequired,
   handleLock: PropTypes.func,
   handleBulkDownload: PropTypes.func.isRequired,
   handleOpenFileInfo: PropTypes.func.isRequired,
   handleOpenDeleteConfirmation: PropTypes.func.isRequired,
+  fileType: PropTypes.string.isRequired,
   // injected
   intl: intlShape.isRequired,
 };

diff --git a/src/files-and-videos/videos-page/VideoThumbnail.jsx b/src/files-and-videos/videos-page/VideoThumbnail.jsx
@@ -10,6 +10,8 @@ import {
 } from '@edx/paragon';
 import { FileInput, useFileInput } from '../generic';
 import messages from './messages';
+import { VIDEO_SUCCESS_STATUSES } from './data/constants';
+import { RequestStatus } from '../../data/constants';
 
 const VideoThumbnail = ({
   thumbnail,
@@ -19,6 +21,7 @@ const VideoThumbnail = ({
   handleAddThumbnail,
   videoImageSettings,
   status,
+  pageLoadStatus,
   // injected
   intl,
 }) => {
@@ -38,14 +41,14 @@ const VideoThumbnail = ({
   }
   const supportedFiles = videoImageSettings?.supportedFileFormats
     ? Object.values(videoImageSettings.supportedFileFormats) : null;
-  const isUploaded = status === 'Success';
+  const isUploaded = VIDEO_SUCCESS_STATUSES.includes(status);
 
   const showThumbnail = allowThumbnailUpload && thumbnail && isUploaded;
 
   return (
     <div data-testid={`video-thumbnail-${id}`} className="video-thumbnail row justify-content-center align-itmes-center">
       {allowThumbnailUpload && <div className="thumbnail-overlay" />}
-      {showThumbnail && !thumbnailError ? (
+      {showThumbnail && !thumbnailError && pageLoadStatus === RequestStatus.SUCCESSFUL ? (
         <div className="border rounded">
           <Image
             style={imageSize}
@@ -110,6 +113,7 @@ VideoThumbnail.propTypes = {
     supportedFileFormats: PropTypes.shape({}),
   }).isRequired,
   status: PropTypes.string.isRequired,
+  pageLoadStatus: PropTypes.string.isRequired,
   // injected
   intl: intlShape.isRequired,
 };

diff --git a/src/files-and-videos/videos-page/VideosPage.jsx b/src/files-and-videos/videos-page/VideosPage.jsx
@@ -107,7 +107,12 @@ const VideosPage = ({
     usageErrorMessages: errorMessages.usageMetrics,
     fileType: 'video',
   };
-  const thumbnailPreview = (props) => VideoThumbnail({ ...props, handleAddThumbnail, videoImageSettings });
+  const thumbnailPreview = (props) => VideoThumbnail({
+    ...props,
+    pageLoadStatus: loadingStatus,
+    handleAddThumbnail,
+    videoImageSettings,
+  });
   const infoModalSidebar = (video, activeTab, setActiveTab) => (
     VideoInfoModalSidebar({ video, activeTab, setActiveTab })
   );
@@ -128,7 +133,7 @@ const VideosPage = ({
     id: 'activeStatus',
     Header: 'Active',
     accessor: 'activeStatus',
-    Cell: ({ row }) => ActiveColumn({ row }),
+    Cell: ({ row }) => ActiveColumn({ row, pageLoadStatus: loadingStatus }),
     Filter: CheckboxFilter,
     filter: 'exactTextCase',
     filterChoices: [
@@ -147,7 +152,7 @@ const VideosPage = ({
   };
   const processingStatusColumn = {
     id: 'status',
-    Header: '',
+    Header: 'Status',
     accessor: 'status',
     Cell: ({ row }) => StatusColumn({ row }),
     Filter: CheckboxFilter,

diff --git a/src/files-and-videos/videos-page/data/api.js b/src/files-and-videos/videos-page/data/api.js
@@ -31,6 +31,21 @@ export async function getVideos(courseId) {
   };
 }
 
+export async function getAllUsagePaths({ courseId, videos, updateModel }) {
+  const apiPromises = videos.map(video => getAuthenticatedHttpClient()
+    .get(`${getVideosUrl(courseId)}/${video.id}/usage`, { videoId: video.id }));
+  const results = await Promise.allSettled(apiPromises);
+  results.forEach(result => {
+    const data = camelCaseObject(result.value.data);
+    const { videoId } = result.value.config;
+    if (data) {
+      updateModel(data, videoId);
+    } else {
+      updateModel({ usageLocation: [] }, videoId);
+    }
+  });
+}
+
 /**
  * Fetches the course custom pages for provided course
  * @param {string} courseId

diff --git a/src/files-and-videos/videos-page/data/thunks.js b/src/files-and-videos/videos-page/data/thunks.js
@@ -21,6 +21,7 @@ import {
   deleteTranscriptPreferences,
   setTranscriptCredentials,
   setTranscriptPreferences,
+  getAllUsagePaths,
 } from './api';
 import {
   setVideoIds,
@@ -37,8 +38,7 @@ import {
 
 import { updateFileValues } from './utils';
 
-async function fetchUsageLocation(videoId, dispatch, courseId, isLast) {
-  const { usageLocations } = await getVideoUsagePaths({ videoId, courseId });
+function updateUsageLocation(videoId, dispatch, usageLocations) {
   const activeStatus = usageLocations?.length > 0 ? 'active' : 'inactive';
 
   dispatch(updateModel({
@@ -49,9 +49,6 @@ async function fetchUsageLocation(videoId, dispatch, courseId, isLast) {
       activeStatus,
     },
   }));
-  if (isLast) {
-    dispatch(updateLoadingStatus({ courseId, status: RequestStatus.SUCCESSFUL }));
-  }
 }
 
 export function fetchVideos(courseId) {
@@ -72,10 +69,13 @@ export function fetchVideos(courseId) {
         dispatch(setVideoIds({
           videoIds: parsedVideos.map(video => video.id),
         }));
-        parsedVideos.forEach(async (video, indx) => {
-          const isLast = parsedVideos.length - 1 === indx;
-          fetchUsageLocation(video.id, dispatch, courseId, isLast);
+        dispatch(updateLoadingStatus({ courseId, status: RequestStatus.PARTIAL }));
+        await getAllUsagePaths({
+          courseId,
+          videos: parsedVideos,
+          updateModel: (apiData, videoId) => updateUsageLocation(videoId, dispatch, apiData.usageLocations),
         });
+        dispatch(updateLoadingStatus({ courseId, status: RequestStatus.SUCCESSFUL }));
       }
     } catch (error) {
       if (error.response && error.response.status === 403) {

diff --git a/src/files-and-videos/videos-page/data/utils.js b/src/files-and-videos/videos-page/data/utils.js
@@ -7,8 +7,6 @@ import {
   MAX_WIDTH,
   MIN_HEIGHT,
   MIN_WIDTH,
-  VIDEO_PROCESSING_STATUSES,
-  VIDEO_SUCCESS_STATUSES,
 } from './constants';
 
 ensureConfig([
@@ -23,7 +21,6 @@ export const updateFileValues = (files, isNewFile) => {
       clientVideoId,
       created,
       courseVideoImageUrl,
-      status,
       transcripts,
     } = file;
 
@@ -42,21 +39,13 @@ export const updateFileValues = (files, isNewFile) => {
     }
     const transcriptStatus = transcripts?.length > 0 ? 'transcribed' : 'notTranscribed';
 
-    let uploadStatus = status;
-    if (VIDEO_SUCCESS_STATUSES.includes(status)) {
-      uploadStatus = 'Success';
-    } else if (VIDEO_PROCESSING_STATUSES.includes(status)) {
-      uploadStatus = 'Processing';
-    }
-
     updatedFiles.push({
       ...file,
       displayName: clientVideoId,
       id: edxVideoId,
       wrapperType,
       dateAdded: created.toString(),
       usageLocations: isNewFile ? [] : null,
-      status: uploadStatus,
       thumbnail,
       transcriptStatus,
       activeStatus: 'inactive',