From 90bc242ddd1b5495052245a7400c73981bb47578 Mon Sep 17 00:00:00 2001 From: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com> Date: Thu, 25 Jan 2024 17:04:34 -0500 Subject: [PATCH] fix: update video page load order (#810) * fix: asset card/row menu appearing for videos * fix: page load time * fix: video status messages --- src/data/constants.js | 1 + src/files-and-videos/files-page/FilesPage.jsx | 2 +- .../files-page/FilesPage.test.jsx | 2 +- src/files-and-videos/files-page/data/thunks.js | 2 +- src/files-and-videos/generic/EditFileErrors.jsx | 2 +- src/files-and-videos/generic/FileMenu.jsx | 6 +++--- src/files-and-videos/generic/FileTable.jsx | 2 ++ .../generic/table-components/GalleryCard.jsx | 4 +++- .../table-custom-columns/ActiveColumn.jsx | 8 +++++--- .../table-custom-columns/MoreInfoColumn.jsx | 6 +++--- .../videos-page/VideoThumbnail.jsx | 8 ++++++-- src/files-and-videos/videos-page/VideosPage.jsx | 11 ++++++++--- src/files-and-videos/videos-page/data/api.js | 15 +++++++++++++++ src/files-and-videos/videos-page/data/thunks.js | 16 ++++++++-------- src/files-and-videos/videos-page/data/utils.js | 11 ----------- 15 files changed, 58 insertions(+), 38 deletions(-) diff --git a/src/data/constants.js b/src/data/constants.js index c025510b4..2448504fa 100644 --- 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 index 55b0ee23e..1d9c7872b 100644 --- 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 index bae1edb63..ec15f993f 100644 --- 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 index c51b51b0c..ded413170 100644 --- 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 index 3f42defd0..4a2e1e909 100644 --- a/src/files-and-videos/generic/EditFileErrors.jsx +++ b/src/files-and-videos/generic/EditFileErrors.jsx @@ -19,7 +19,7 @@ const EditFileErrors = ({ 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 })} diff --git a/src/files-and-videos/generic/FileMenu.jsx b/src/files-and-videos/generic/FileMenu.jsx index 6a25fc891..3ce9ed7ad 100644 --- 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" /> - {wrapperType === 'video' ? ( + {fileType === 'video' ? ( 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 index f58f5c29f..9e302647e 100644 --- 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 index 0ad792464..138f489e9 100644 --- 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 index 84fdf127b..1519ba4e9 100644 --- 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 ( { )} /> @@ -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 index 7e359ce30..3932416f9 100644 --- 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 = ({ - {wrapperType === 'video' ? ( + {fileType === 'video' ? ( { @@ -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 (
{allowThumbnailUpload &&
} - {showThumbnail && !thumbnailError ? ( + {showThumbnail && !thumbnailError && pageLoadStatus === RequestStatus.SUCCESSFUL ? (
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 index 58bae471f..a7137679f 100644 --- 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 index 3066e4025..570f18270 100644 --- 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 index 4fcd247c8..149c2bfc6 100644 --- 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,13 +39,6 @@ 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, @@ -56,7 +46,6 @@ export const updateFileValues = (files, isNewFile) => { wrapperType, dateAdded: created.toString(), usageLocations: isNewFile ? [] : null, - status: uploadStatus, thumbnail, transcriptStatus, activeStatus: 'inactive',