From 549b2d8cd83aa8b69e6cc347f8bcdd27570cc45b Mon Sep 17 00:00:00 2001 From: seaerchin Date: Mon, 7 Nov 2022 15:43:07 +0800 Subject: [PATCH 1/3] fix(approvedreviewredirect): removed redir for github users on error --- src/routing/ApprovedReviewRedirect.tsx | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/routing/ApprovedReviewRedirect.tsx b/src/routing/ApprovedReviewRedirect.tsx index bdccaa018..eac0d7469 100644 --- a/src/routing/ApprovedReviewRedirect.tsx +++ b/src/routing/ApprovedReviewRedirect.tsx @@ -2,6 +2,8 @@ import { Greyscale } from "components/Greyscale" import { PropsWithChildren, useEffect } from "react" import { Redirect, RedirectProps, useParams } from "react-router-dom" +import { useLoginContext } from "contexts/LoginContext" + import { useGetReviewRequests } from "hooks/siteDashboardHooks" import { getAxiosErrorMessage } from "utils/axios" @@ -23,17 +25,20 @@ export const ApprovedReviewRedirect = ({ const errorToast = useErrorToast() const warningToast = useWarningToast() + const { userId } = useLoginContext() + const isGithubUser = !!userId + const hasApprovedReviewRequest = reviewRequests && reviewRequests.filter((req) => req.status === "APPROVED").length > 0 useEffect(() => { - if (isError) { + if (!isGithubUser && isError) { errorToast({ description: getAxiosErrorMessage(error), }) } - }, [isError, error, errorToast]) + }, [isError, error, errorToast, isGithubUser]) useEffect(() => { if (hasApprovedReviewRequest) { @@ -44,7 +49,9 @@ export const ApprovedReviewRedirect = ({ } }, [hasApprovedReviewRequest, warningToast]) - return ( + return isGithubUser ? ( + <>{children} + ) : ( {hasApprovedReviewRequest && ( @@ -53,7 +60,7 @@ export const ApprovedReviewRedirect = ({ * If we fail to retrieve the list of review requests, * we take the conservative route and prevent the user from editing. */} - {!hasApprovedReviewRequest && isError && ( + {!hasApprovedReviewRequest && !isGithubUser && isError && ( )} {children} From 627a7c87f2c34ed77cf64f5edc9f60f84dd67de7 Mon Sep 17 00:00:00 2001 From: seaerchin <44049504+seaerchin@users.noreply.github.com> Date: Mon, 13 Feb 2023 15:38:40 +0800 Subject: [PATCH 2/3] fix(media): disallow file extension change (#1173) * refactor(imagepreviewcard): shift util method into separate file * refactor(mediacreation/update): prevent users from being able to change file ext * fix(files): update utilmethod --- .../MediaCreationModal/MediaCreationModal.jsx | 2 +- .../MediaSettingsModal/MediaSettingsModal.jsx | 15 +++++++---- .../MediaSettingsSchema.jsx | 21 +++++++++------ .../Media/components/ImagePreviewCard.tsx | 18 ++----------- src/utils/files.ts | 26 +++++++++++++++++++ src/utils/index.ts | 1 + 6 files changed, 53 insertions(+), 30 deletions(-) create mode 100644 src/utils/files.ts diff --git a/src/components/MediaCreationModal/MediaCreationModal.jsx b/src/components/MediaCreationModal/MediaCreationModal.jsx index 7c115da72..886dd463e 100644 --- a/src/components/MediaCreationModal/MediaCreationModal.jsx +++ b/src/components/MediaCreationModal/MediaCreationModal.jsx @@ -26,7 +26,7 @@ export const MediaCreationModal = ({ const methods = useForm({ mode: "onTouched", resolver: yupResolver(MediaSettingsSchema(existingTitlesArray)), - context: { mediaRoom }, + context: { mediaRoom, isCreate: true }, }) const onMediaUpload = async (event) => { diff --git a/src/components/MediaSettingsModal/MediaSettingsModal.jsx b/src/components/MediaSettingsModal/MediaSettingsModal.jsx index 06bfb15f8..685ceb3fc 100644 --- a/src/components/MediaSettingsModal/MediaSettingsModal.jsx +++ b/src/components/MediaSettingsModal/MediaSettingsModal.jsx @@ -16,7 +16,7 @@ import elementStyles from "styles/isomer-cms/Elements.module.scss" import contentStyles from "styles/isomer-cms/pages/Content.module.scss" import mediaStyles from "styles/isomer-cms/pages/Media.module.scss" -import { getLastItemType } from "utils" +import { getLastItemType, getFileExt, getFileName } from "utils" import { MediaSettingsSchema } from "./MediaSettingsSchema" @@ -61,9 +61,11 @@ export const MediaSettingsModal = ({ useForm({ mode: "onTouched", resolver: yupResolver(MediaSettingsSchema(existingTitlesArray)), - context: { mediaRoom }, + context: { mediaRoom, isCreate }, }) + const fileExt = getFileExt(mediaData?.name || "") + /** ******************************** */ /* useEffects to load data */ /** ******************************** */ @@ -71,7 +73,7 @@ export const MediaSettingsModal = ({ useEffect(() => { if (fileName && mediaData && mediaData.name && mediaData.mediaUrl) { setValue("mediaUrl", mediaData.mediaUrl) - setValue("name", mediaData.name) + setValue("name", getFileName(mediaData.name)) setValue("sha", mediaData.sha) } }, [setValue, mediaData]) @@ -80,9 +82,12 @@ export const MediaSettingsModal = ({ /* handler functions */ /** ******************************** */ - const onSubmit = (data) => { + const onSubmit = ({ name, ...rest }) => { return onProceed({ - data, + data: { + ...rest, + name: `${name}.${fileExt}`, + }, }) } diff --git a/src/components/MediaSettingsModal/MediaSettingsSchema.jsx b/src/components/MediaSettingsModal/MediaSettingsSchema.jsx index 4bb3fc6a4..dca7b2089 100644 --- a/src/components/MediaSettingsModal/MediaSettingsSchema.jsx +++ b/src/components/MediaSettingsModal/MediaSettingsSchema.jsx @@ -13,10 +13,15 @@ export const MediaSettingsSchema = (existingTitlesArray = []) => Yup.object().shape({ name: Yup.string() .required("Title is required") - .test( - "Special characters found", - 'Title cannot contain any of the following special characters: ~%^*+#?./`;{}[]"<>', - (value) => !mediaSpecialCharactersRegexTest.test(value.split(".")[0]) + .when("$isCreate", (isCreate, schema) => + schema.test( + "Special characters found", + 'Title cannot contain any of the following special characters: ~%^*+#?./`;{}[]"<>', + (value) => { + const prefix = isCreate ? value.split(".")[0] : value + return !mediaSpecialCharactersRegexTest.test(prefix) + } + ) ) .test( "File not supported", @@ -34,15 +39,15 @@ export const MediaSettingsSchema = (existingTitlesArray = []) => `Title must be shorter than ${MEDIA_SETTINGS_TITLE_MAX_LENGTH} characters` ) // When this is called, mediaRoom is one of either images or files - .when("$mediaRoom", (mediaRoom, schema) => { - if (mediaRoom === "images") { + .when(["$mediaRoom", "$isCreate"], (mediaRoom, isCreate, schema) => { + if (isCreate && mediaRoom === "images") { return schema.test( "Special characters found", "Title must end with one of the following extensions: 'png', 'jpeg', 'jpg', 'gif', 'tif', 'bmp', 'ico', 'svg'", (value) => imagesSuffixRegexTest.test(value) ) } - if (mediaRoom === "files") { + if (isCreate && mediaRoom === "files") { return schema.test( "Special characters found", "Title must end with the following extensions: 'pdf'", @@ -53,7 +58,7 @@ export const MediaSettingsSchema = (existingTitlesArray = []) => return schema.test( "Invalid case", "This is an invalid value for the mediaRoom type!", - () => false + () => mediaRoom === "files" || mediaRoom === "images" ) }) .notOneOf( diff --git a/src/layouts/Media/components/ImagePreviewCard.tsx b/src/layouts/Media/components/ImagePreviewCard.tsx index 23ce27da8..211e9e951 100644 --- a/src/layouts/Media/components/ImagePreviewCard.tsx +++ b/src/layouts/Media/components/ImagePreviewCard.tsx @@ -13,27 +13,13 @@ import { HStack, } from "@chakra-ui/react" import { ContextMenu } from "components/ContextMenu" -import _ from "lodash" import { BiChevronRight, BiEditAlt, BiFolder, BiTrash } from "react-icons/bi" import { Link as RouterLink, useRouteMatch } from "react-router-dom" import useRedirectHook from "hooks/useRedirectHook" import { CARD_THEME_KEY } from "theme/components/Card" - -const getFileExt = (mediaUrl: string): string => { - // NOTE: If it starts with data, the image is within a private repo. - // Hence, we will extract the portion after the specifier - // till the terminating semi-colon for use as the extension - if (mediaUrl.startsWith("data:image/")) { - return _.takeWhile(mediaUrl.slice(11), (char) => char !== ";").join("") - } - - // Otherwise, this will point to a publicly accessible github url - return ( - mediaUrl.split(".").pop()?.split("?").shift() || "Unknown file extension" - ) -} +import { getFileExt } from "utils" interface ImagePreviewCardProps { name: string @@ -51,7 +37,7 @@ export const ImagePreviewCard = ({ const styles = useMultiStyleConfig(CARD_THEME_KEY, {}) const encodedName = encodeURIComponent(name) const { setRedirectToPage } = useRedirectHook() - const fileExt = getFileExt(mediaUrl) + const fileExt = getFileExt(mediaUrl) || "Unknown file extension" return ( diff --git a/src/utils/files.ts b/src/utils/files.ts new file mode 100644 index 000000000..bbeab239b --- /dev/null +++ b/src/utils/files.ts @@ -0,0 +1,26 @@ +import _ from "lodash" + +export const getFileExt = (mediaUrl: string): string | undefined => { + // NOTE: If it starts with data, the image is within a private repo. + // Hence, we will extract the portion after the specifier + // till the terminating semi-colon for use as the extension + if (mediaUrl.startsWith("data:image/")) { + return _.takeWhile(mediaUrl.slice(11), (char) => char !== ";").join("") + } + + // Otherwise, this will point to a publicly accessible github url + return mediaUrl.split(".").pop()?.split("?").shift() +} + +/** + * @precondition this function must only be called on a qualified filename. + * Ie, calling this function on a `data:...` is invalid and the function will return + * erroneous info. + * + * This function also assumes that the file is not nested within any directory + * @param name the qualified filename of the data + * @returns + */ +export const getFileName = (name: string): string => { + return name.split(".").slice(0, -1).join(".") +} diff --git a/src/utils/index.ts b/src/utils/index.ts index b4af14544..6fbad507d 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -10,3 +10,4 @@ export * from "./validators" export * from "./legacy" export * from "./text" export * from "./date" +export * from "./files" From 0de2251f0190b7b6593ddf67e63be8d9814ebb06 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Mon, 27 Feb 2023 14:26:51 +0800 Subject: [PATCH 3/3] Fix: remove . when no file extension (#1184) * Fix: remove . when no file extension * feat: restriction file extension modification for media upload * Fix: restrict duplicate file names * Fix: media schema * Nit: add comment for behaviour of fileExt --- .../MediaCreationModal/MediaCreationModal.jsx | 20 +++++++++++++++---- .../MediaSettingsModal/MediaSettingsModal.jsx | 4 +++- .../MediaSettingsSchema.jsx | 20 ++----------------- src/layouts/screens/MediaSettingsScreen.jsx | 7 ++++++- 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/components/MediaCreationModal/MediaCreationModal.jsx b/src/components/MediaCreationModal/MediaCreationModal.jsx index 886dd463e..ee2c3f85e 100644 --- a/src/components/MediaCreationModal/MediaCreationModal.jsx +++ b/src/components/MediaCreationModal/MediaCreationModal.jsx @@ -4,12 +4,14 @@ import { MediaSettingsSchema, MediaSettingsModal, } from "components/MediaSettingsModal" -import { useEffect, useRef } from "react" +import { useEffect, useRef, useState } from "react" import { useForm, FormProvider } from "react-hook-form" import { useErrorToast } from "utils/toasts" import { MEDIA_FILE_MAX_SIZE } from "utils/validators" +import { getFileExt, getFileName } from "utils" + // eslint-disable-next-line import/prefer-default-export export const MediaCreationModal = ({ params, @@ -21,7 +23,8 @@ export const MediaCreationModal = ({ const inputFile = useRef(null) const errorToast = useErrorToast() - const existingTitlesArray = mediasData.map((item) => item.name) + const existingTitlesArray = mediasData.map((item) => getFileName(item.name)) + const [fileExt, setFileExt] = useState("") const methods = useForm({ mode: "onTouched", @@ -38,7 +41,9 @@ export const MediaCreationModal = ({ }) } else { mediaReader.onload = () => { - methods.setValue("name", media.name) + const fileName = getFileName(media.name) + setFileExt(getFileExt(media.name)) + methods.setValue("name", fileName) methods.setValue("content", mediaReader.result) } mediaReader.readAsDataURL(media) @@ -68,7 +73,14 @@ export const MediaCreationModal = ({ { + return onProceed({ + data: { + ...submissionData.data, + name: `${submissionData.data.name}.${fileExt}`, + }, + }) + }} mediaRoom={mediaRoom} onClose={onClose} toggleUploadInput={() => inputFile.current.click()} diff --git a/src/components/MediaSettingsModal/MediaSettingsModal.jsx b/src/components/MediaSettingsModal/MediaSettingsModal.jsx index 685ceb3fc..c99b5172d 100644 --- a/src/components/MediaSettingsModal/MediaSettingsModal.jsx +++ b/src/components/MediaSettingsModal/MediaSettingsModal.jsx @@ -64,6 +64,7 @@ export const MediaSettingsModal = ({ context: { mediaRoom, isCreate }, }) + // fileExt is blank for newly created files - mediaData is undefined for the create flow const fileExt = getFileExt(mediaData?.name || "") /** ******************************** */ @@ -86,7 +87,8 @@ export const MediaSettingsModal = ({ return onProceed({ data: { ...rest, - name: `${name}.${fileExt}`, + // Period is appended only if fileExt exists, otherwise MediaCreationModal handles the period and extension appending + name: `${name}${fileExt ? `.${fileExt}` : ""}`, }, }) } diff --git a/src/components/MediaSettingsModal/MediaSettingsSchema.jsx b/src/components/MediaSettingsModal/MediaSettingsSchema.jsx index dca7b2089..6bc9132a9 100644 --- a/src/components/MediaSettingsModal/MediaSettingsSchema.jsx +++ b/src/components/MediaSettingsModal/MediaSettingsSchema.jsx @@ -1,8 +1,6 @@ import * as Yup from "yup" import { - imagesSuffixRegexTest, - filesSuffixRegexTest, mediaSpecialCharactersRegexTest, MEDIA_SETTINGS_TITLE_MIN_LENGTH, MEDIA_SETTINGS_TITLE_MAX_LENGTH, @@ -40,29 +38,15 @@ export const MediaSettingsSchema = (existingTitlesArray = []) => ) // When this is called, mediaRoom is one of either images or files .when(["$mediaRoom", "$isCreate"], (mediaRoom, isCreate, schema) => { - if (isCreate && mediaRoom === "images") { - return schema.test( - "Special characters found", - "Title must end with one of the following extensions: 'png', 'jpeg', 'jpg', 'gif', 'tif', 'bmp', 'ico', 'svg'", - (value) => imagesSuffixRegexTest.test(value) - ) - } - if (isCreate && mediaRoom === "files") { - return schema.test( - "Special characters found", - "Title must end with the following extensions: 'pdf'", - (value) => filesSuffixRegexTest.test(value) - ) - } - return schema.test( "Invalid case", "This is an invalid value for the mediaRoom type!", () => mediaRoom === "files" || mediaRoom === "images" ) }) + .lowercase() .notOneOf( - existingTitlesArray, + existingTitlesArray.map((title) => title.toLowerCase()), "Title is already in use. Please choose a different title." ), }) diff --git a/src/layouts/screens/MediaSettingsScreen.jsx b/src/layouts/screens/MediaSettingsScreen.jsx index 2de91e7d3..5e19e1197 100644 --- a/src/layouts/screens/MediaSettingsScreen.jsx +++ b/src/layouts/screens/MediaSettingsScreen.jsx @@ -4,6 +4,8 @@ import PropTypes from "prop-types" import { useGetMediaFolders } from "hooks/directoryHooks" import { useGetMediaHook, useUpdateMediaHook } from "hooks/mediaHooks" +import { getFileName } from "utils" + export const MediaSettingsScreen = ({ match, onClose }) => { const { params, decodedParams } = match const { data: mediaData } = useGetMediaHook(params) @@ -17,7 +19,10 @@ export const MediaSettingsScreen = ({ match, onClose }) => { params={decodedParams} onClose={onClose} mediaData={mediaData} - mediasData={mediasData} + mediasData={mediasData.map(({ name, ...rest }) => ({ + name: getFileName(name), + ...rest, + }))} onProceed={updateHandler} /> )