Vapt: merge back into tracking branch (#1195)

* fix(approvedreviewredirect): removed redir for github users on error * 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 * 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 --------- Co-authored-by: seaerchin <[email protected]> Co-authored-by: seaerchin <[email protected]>
isomerpages · Mar 7, 2023 · d7e47fd · d7e47fd
1 parent 9fcaf84
commit d7e47fd
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 55 deletions.
diff --git 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,12 +23,13 @@ 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",
     resolver: yupResolver(MediaSettingsSchema(existingTitlesArray)),
-    context: { mediaRoom },
+    context: { mediaRoom, isCreate: true },
   })
 
   const onMediaUpload = async (event) => {
@@ -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 = ({
         <MediaSettingsModal
           params={params}
           mediasData={mediasData}
-          onProceed={onProceed}
+          onProceed={(submissionData) => {
+            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
@@ -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,17 +61,20 @@ export const MediaSettingsModal = ({
     useForm({
       mode: "onTouched",
       resolver: yupResolver(MediaSettingsSchema(existingTitlesArray)),
-      context: { mediaRoom },
+      context: { mediaRoom, isCreate },
     })
 
+  // fileExt is blank for newly created files - mediaData is undefined for the create flow
+  const fileExt = getFileExt(mediaData?.name || "")
+
   /** ******************************** */
   /*     useEffects to load data     */
   /** ******************************** */
 
   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 +83,13 @@ export const MediaSettingsModal = ({
   /*     handler functions    */
   /** ******************************** */
 
-  const onSubmit = (data) => {
+  const onSubmit = ({ name, ...rest }) => {
     return onProceed({
-      data,
+      data: {
+        ...rest,
+        // 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
@@ -1,8 +1,6 @@
 import * as Yup from "yup"
 
 import {
-  imagesSuffixRegexTest,
-  filesSuffixRegexTest,
   mediaSpecialCharactersRegexTest,
   MEDIA_SETTINGS_TITLE_MIN_LENGTH,
   MEDIA_SETTINGS_TITLE_MAX_LENGTH,
@@ -13,10 +11,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,30 +37,16 @@ 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") {
-          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") {
-          return schema.test(
-            "Special characters found",
-            "Title must end with the following extensions: 'pdf'",
-            (value) => filesSuffixRegexTest.test(value)
-          )
-        }
-
+      .when(["$mediaRoom", "$isCreate"], (mediaRoom, isCreate, schema) => {
         return schema.test(
           "Invalid case",
           "This is an invalid value for the mediaRoom type!",
-          () => false
+          () => 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/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 (
     <Box position="relative">

diff --git 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}
     />
   )

diff --git 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}</>
+  ) : (
     <Greyscale isActive={isLoading || !reviewRequests}>
       {hasApprovedReviewRequest && (
         <Redirect {...rest} to={`/sites/${siteName}/dashboard`} />
@@ -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 && (
         <Redirect {...rest} to="/sites" />
       )}
       {children}

diff --git a/src/utils/files.ts 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
@@ -10,3 +10,4 @@ export * from "./validators"
 export * from "./legacy"
 export * from "./text"
 export * from "./date"
+export * from "./files"