fix: more code smells (#522)

* fix: set land in app html fixture * fix: rephrase in question * fix: add readonly props * fix: more code smell issues * fix: done with code smells
graasp · Nov 29, 2024 · eae2d87 · eae2d87
1 parent 6855f41
commit eae2d87
Show file tree

Hide file tree

Showing 39 changed files with 127 additions and 246 deletions.
diff --git a/cypress/fixtures/apps/app.html b/cypress/fixtures/apps/app.html
@@ -1,5 +1,5 @@
-<!DOCTYPE html>
-<html>
+<!doctype html>
+<html lang="en">
   <head>
     <meta charset="utf-8" />
     <meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1" />

diff --git a/cypress/fixtures/fileLinks.ts b/cypress/fixtures/fileLinks.ts
@@ -1,6 +1,6 @@
 // we use real links for items
 // because we cannot return fixture urls?
-// todo: internal links? host our own links?
+// Question: internal links? host our own links?
 
 export const MOCK_IMAGE_URL = 'https://picsum.photos/100';
 export const MOCK_VIDEO_URL =

diff --git a/src/config/env.ts b/src/config/env.ts
@@ -24,5 +24,6 @@ export const H5P_INTEGRATION_URL =
   import.meta.env.VITE_GRAASP_H5P_INTEGRATION_URL ||
   `${API_HOST}/p/h5p-integration`;
 
-// todo: we should host the pdf player assets inside the public directory here instead of at another bucket
+// Question: should we host the pdf player assets inside the public directory here instead of at another bucket ?
+// Are there any security implications if it is hosted on the same domain as the app code ?
 export const GRAASP_ASSETS_URL = import.meta.env.VITE_GRAASP_ASSETS_URL;
diff --git a/src/main.tsx b/src/main.tsx
@@ -5,8 +5,11 @@ import { useTranslation } from 'react-i18next';
 import { ToastContainer } from 'react-toastify';
 import 'react-toastify/dist/ReactToastify.css';
 
-import { CssBaseline } from '@mui/material';
-import { Direction, ThemeProvider as MuiThemeProvider } from '@mui/material';
+import {
+  CssBaseline,
+  Direction,
+  ThemeProvider as MuiThemeProvider,
+} from '@mui/material';
 
 import { BUILDER_ITEMS_PREFIX, ClientHostManager, Context } from '@graasp/sdk';
 import rtlPlugin from '@graasp/stylis-plugin-rtl';
@@ -94,7 +97,7 @@ const getCacheForDirection = (direction?: Direction): EmotionCache =>
     stylisPlugins: [prefixer, ...(direction === 'rtl' ? [rtlPlugin] : [])],
   });
 
-function ThemeWrapper({ children }: ThemeWrapperProps): JSX.Element {
+function ThemeWrapper({ children }: Readonly<ThemeWrapperProps>): JSX.Element {
   // use the hook as it allows to use the correct instance of i18n
   const { i18n: i18nInstance } = useTranslation();
   const direction = i18nInstance.dir(i18nInstance.language);

diff --git a/src/modules/account/common/MemberProfileItem.tsx b/src/modules/account/common/MemberProfileItem.tsx
@@ -11,7 +11,7 @@ export function MemberProfileItem({
   title,
   content,
   contentId,
-}: Props): JSX.Element {
+}: Readonly<Props>): JSX.Element {
   return (
     <Stack direction="row" gap={2} alignItems="center">
       <Typography variant="body1" color="textSecondary">

diff --git a/src/modules/account/home/recentItems/DisplayItems.tsx b/src/modules/account/home/recentItems/DisplayItems.tsx
@@ -17,10 +17,10 @@ const GridWrapper = ({ children }: { children: ReactNode }): JSX.Element => (
 export function DisplayItems({
   items,
   isLoading,
-}: {
+}: Readonly<{
   items?: PackedItem[];
   isLoading: boolean;
-}): ReactNode | null {
+}>): ReactNode | null {
   const { t } = useTranslation(NS.Player);
 
   if (items) {

diff --git a/src/modules/account/settings/EmailPreferenceSwitch.tsx b/src/modules/account/settings/EmailPreferenceSwitch.tsx
@@ -17,7 +17,7 @@ export function EmailPreferenceSwitch({
   id,
   emailFreq,
   onChange,
-}: EmailPreferenceSwitchProps): JSX.Element {
+}: Readonly<EmailPreferenceSwitchProps>): JSX.Element {
   const { t } = useTranslation(NS.Account);
 
   const handleChange = (event: SelectChangeEvent<string>) => {

diff --git a/src/modules/auth/components/LeftContentContainer.tsx b/src/modules/auth/components/LeftContentContainer.tsx
@@ -26,7 +26,9 @@ type Props = {
   children: JSX.Element | JSX.Element[];
 };
 
-export function LeftContentContainer({ children }: Props): JSX.Element {
+export function LeftContentContainer({
+  children,
+}: Readonly<Props>): JSX.Element {
   const { t } = useTranslation(NS.Auth);
 
   return (

diff --git a/src/modules/auth/components/Redirection.tsx b/src/modules/auth/components/Redirection.tsx
@@ -18,7 +18,7 @@ type Props = {
   children: ReactNode;
 };
 
-export function Redirection({ children }: Props) {
+export function Redirection({ children }: Readonly<Props>) {
   const theme = useTheme();
   const { data: member } = hooks.useCurrentMember();
   const { t } = useTranslation(NS.Auth);

diff --git a/src/modules/auth/components/common/ErrorDisplay.tsx b/src/modules/auth/components/common/ErrorDisplay.tsx
@@ -8,9 +8,9 @@ import { ERROR_DISPLAY_ID } from '@/config/selectors';
 
 export function ErrorDisplay({
   error,
-}: {
+}: Readonly<{
   error: Error | null;
-}): JSX.Element | null {
+}>): JSX.Element | null {
   const { t: translateMessages } = useTranslation(NS.Messages);
 
   if (!error) {

diff --git a/src/modules/auth/components/common/PasswordInput.tsx b/src/modules/auth/components/common/PasswordInput.tsx
@@ -19,7 +19,11 @@ type Props = {
   error: string | undefined;
 };
 
-export function PasswordInput({ id, error, form }: Props): JSX.Element {
+export function PasswordInput({
+  id,
+  error,
+  form,
+}: Readonly<Props>): JSX.Element {
   const { t } = useTranslation(NS.Auth);
 
   const [showPassword, setShowPassword] = useState(false);

diff --git a/src/modules/auth/components/layout/CenteredContent.tsx b/src/modules/auth/components/layout/CenteredContent.tsx
@@ -10,10 +10,10 @@ import { styledBox } from '../styles';
 export function CenteredContent({
   children,
   header,
-}: {
+}: Readonly<{
   children: ReactNode;
   header: ReactNode;
-}) {
+}>) {
   return (
     <Stack
       direction="column"

diff --git a/src/modules/auth/components/layout/DialogHeader.tsx b/src/modules/auth/components/layout/DialogHeader.tsx
@@ -8,7 +8,10 @@ type DialogHeaderProps = {
   title: string;
   description?: string | ReactNode;
 };
-export function DialogHeader({ title, description }: DialogHeaderProps) {
+export function DialogHeader({
+  title,
+  description,
+}: Readonly<DialogHeaderProps>) {
   const theme = useTheme();
 
   return (

diff --git a/src/modules/auth/components/leftContent/PlatformContent.tsx b/src/modules/auth/components/leftContent/PlatformContent.tsx
@@ -7,7 +7,7 @@ type Props = {
   color: string;
 };
 
-export function PlatformContent({ Icon, name, text, color }: Props) {
+export function PlatformContent({ Icon, name, text, color }: Readonly<Props>) {
   return (
     <Stack width="100%" direction="row" alignItems="center">
       <Icon primaryColor={color} size={110} />

diff --git a/src/modules/auth/components/register/EmailInput.tsx b/src/modules/auth/components/register/EmailInput.tsx
@@ -27,7 +27,7 @@ export function EmailInput({
   setValue,
   shouldValidate = true,
   autoFocus = false,
-}: Props) {
+}: Readonly<Props>) {
   const { t } = useTranslation(NS.Auth);
 
   const [error, setError] = useState<string | null>(null);

diff --git a/src/modules/auth/components/register/Register.tsx b/src/modules/auth/components/register/Register.tsx
@@ -32,7 +32,7 @@ import {
   REGISTER_SAVE_ACTIONS_ID,
 } from '@/config/selectors';
 
-import { useRecaptcha } from '~auth/context/RecaptchaContext';
+import { executeCaptcha } from '~auth/context/RecaptchaContext';
 import { useMobileAppLogin } from '~auth/hooks/useMobileAppLogin';
 import { AUTH } from '~auth/langs';
 
@@ -52,7 +52,9 @@ type FormElementProps = {
   control: Control<RegisterInputs>;
 };
 
-function EnableAnalyticsForm({ control }: FormElementProps): JSX.Element {
+function EnableAnalyticsForm({
+  control,
+}: Readonly<FormElementProps>): JSX.Element {
   const { field } = useController({ control, name: 'enableSaveActions' });
   const { t } = useTranslation(NS.Auth);
 
@@ -77,7 +79,7 @@ function EnableAnalyticsForm({ control }: FormElementProps): JSX.Element {
   );
 }
 
-export function AgreementForm({ control }: FormElementProps) {
+export function AgreementForm({ control }: Readonly<FormElementProps>) {
   const { t } = useTranslation(NS.Auth);
   const {
     field,
@@ -143,15 +145,14 @@ const defaultRedirection = new URL(
   window.location.origin,
 ).toString();
 
-export function RegisterForm({ search, initialData }: RegisterProps) {
+export function RegisterForm({ search, initialData }: Readonly<RegisterProps>) {
   const { t, i18n } = useTranslation(NS.Auth);
   const { t: translateCommon } = useTranslation(NS.Common, {
     keyPrefix: 'FIELD_ERROR',
   });
 
   const navigate = useNavigate();
   const location = useLocation();
-  const { executeCaptcha } = useRecaptcha();
   const { isMobile, challenge } = useMobileAppLogin();
   const {
     register,

diff --git a/src/modules/auth/components/requestPasswordReset/RequestPasswordReset.tsx b/src/modules/auth/components/requestPasswordReset/RequestPasswordReset.tsx
@@ -18,9 +18,9 @@ import {
 } from '@/config/selectors';
 
 import { HELP_EMAIL } from '~auth/constants';
+import { executeCaptcha } from '~auth/context/RecaptchaContext';
 import { AUTH } from '~auth/langs';
 
-import { useRecaptcha } from '../../context/RecaptchaContext';
 import { EmailAdornment } from '../common/adornments';
 import { CenteredContent } from '../layout/CenteredContent';
 import { DialogHeader } from '../layout/DialogHeader';
@@ -38,7 +38,6 @@ export function RequestPasswordReset() {
     handleSubmit,
     formState: { errors },
   } = useForm<Inputs>();
-  const { executeCaptcha } = useRecaptcha();
 
   const {
     mutate: requestPasswordReset,

diff --git a/src/modules/auth/components/signIn/EmailInput.tsx b/src/modules/auth/components/signIn/EmailInput.tsx
@@ -22,7 +22,7 @@ export function EmailInput({
   form,
   error,
   placeholder,
-}: Props): JSX.Element {
+}: Readonly<Props>): JSX.Element {
   const { t } = useTranslation(NS.Auth);
 
   return (

diff --git a/src/modules/auth/components/signIn/MagicLinkLoginForm.tsx b/src/modules/auth/components/signIn/MagicLinkLoginForm.tsx
@@ -18,7 +18,7 @@ import {
 import { AUTH } from '~auth/langs';
 import { isEmailValid } from '~auth/validation';
 
-import { useRecaptcha } from '../../context/RecaptchaContext';
+import { executeCaptcha } from '../../context/RecaptchaContext';
 import { useMobileAppLogin } from '../../hooks/useMobileAppLogin';
 import { ErrorDisplay } from '../common/ErrorDisplay';
 import { EmailInput } from './EmailInput';
@@ -33,12 +33,13 @@ type MagicLinkLoginFormProps = {
   };
 };
 
-export function MagicLinkLoginForm({ search }: MagicLinkLoginFormProps) {
+export function MagicLinkLoginForm({
+  search,
+}: Readonly<MagicLinkLoginFormProps>) {
   const navigate = useNavigate();
   const location = useLocation();
   const { t } = useTranslation(NS.Auth);
 
-  const { executeCaptcha } = useRecaptcha();
   const {
     register,
     handleSubmit,

diff --git a/src/modules/auth/components/signIn/PasswordLoginForm.tsx b/src/modules/auth/components/signIn/PasswordLoginForm.tsx
@@ -16,7 +16,7 @@ import {
   PASSWORD_SUCCESS_ALERT,
 } from '@/config/selectors';
 
-import { useRecaptcha } from '~auth/context/RecaptchaContext';
+import { executeCaptcha } from '~auth/context/RecaptchaContext';
 import { useMobileAppLogin } from '~auth/hooks/useMobileAppLogin';
 import { AUTH } from '~auth/langs';
 
@@ -35,10 +35,9 @@ type PasswordLoginProps = {
   };
 };
 
-export function PasswordLoginForm({ search }: PasswordLoginProps) {
+export function PasswordLoginForm({ search }: Readonly<PasswordLoginProps>) {
   const { t } = useTranslation(NS.Auth);
   const { isMobile, challenge } = useMobileAppLogin();
-  const { executeCaptcha } = useRecaptcha();
   const {
     register,
     handleSubmit,

diff --git a/src/modules/auth/context/RecaptchaContext.tsx b/src/modules/auth/context/RecaptchaContext.tsx
@@ -1,4 +1,4 @@
-import { createContext, useContext } from 'react';
+import { RECAPTCHA_SITE_KEY } from '@/config/env';
 
 const MOCK_RECAPTCHA_TOKEN = 'development-token';
 
@@ -14,59 +14,36 @@ declare global {
   }
 }
 
-type RecaptchaContextType = {
-  executeCaptcha: (action: string) => Promise<string>;
-};
-
-const RecaptchaContext = createContext<RecaptchaContextType>({
-  executeCaptcha: () => Promise.reject('No Recaptcha context provider found'),
-});
-
-type Props = {
-  children: JSX.Element;
-  siteKey: string;
-};
-
-export function RecaptchaProvider({ children, siteKey }: Props): JSX.Element {
-  const executeCaptcha = (action: string): Promise<string> => {
-    return new Promise<string>((resolve) => {
-      if (!window.grecaptcha) {
-        if (import.meta.env.DEV) {
-          // in dev we resolve to a mock string
-          console.debug('No recaptcha key set-up, using mock value');
-          resolve(MOCK_RECAPTCHA_TOKEN);
-        }
-        resolve('error-recaptcha-not-present');
-      } else {
-        window.grecaptcha.ready?.(async () => {
-          try {
-            const token = await window.grecaptcha.execute(siteKey, { action });
-            resolve(token);
-          } catch (err) {
-            // if we are in dev and the error is that the client id is not set, we resolve to a mock value
-            if (
-              err instanceof Error &&
-              (err.toString().includes('Invalid reCAPTCHA client id') ||
-                err.toString().includes('No reCAPTCHA clients exist')) &&
-              (import.meta.env.DEV || import.meta.env.MODE === 'test')
-            ) {
-              console.debug('No recaptcha key set-up, using mock value');
-              resolve(MOCK_RECAPTCHA_TOKEN);
-            }
-            console.error(err);
-          }
-        });
+export function executeCaptcha(action: string): Promise<string> {
+  return new Promise<string>((resolve) => {
+    if (!window.grecaptcha) {
+      if (import.meta.env.DEV) {
+        // in dev we resolve to a mock string
+        console.debug('No recaptcha key set-up, using mock value');
+        resolve(MOCK_RECAPTCHA_TOKEN);
       }
-    });
-  };
-
-  const value = { executeCaptcha };
-  return (
-    <RecaptchaContext.Provider value={value}>
-      {children}
-    </RecaptchaContext.Provider>
-  );
+      resolve('error-recaptcha-not-present');
+    } else {
+      window.grecaptcha.ready?.(async () => {
+        try {
+          const token = await window.grecaptcha.execute(RECAPTCHA_SITE_KEY, {
+            action,
+          });
+          resolve(token);
+        } catch (err) {
+          // if we are in dev and the error is that the client id is not set, we resolve to a mock value
+          if (
+            err instanceof Error &&
+            (err.toString().includes('Invalid reCAPTCHA client id') ||
+              err.toString().includes('No reCAPTCHA clients exist')) &&
+            (import.meta.env.DEV || import.meta.env.MODE === 'test')
+          ) {
+            console.debug('No recaptcha key set-up, using mock value');
+            resolve(MOCK_RECAPTCHA_TOKEN);
+          }
+          console.error(err);
+        }
+      });
+    }
+  });
 }
-
-export const useRecaptcha = (): RecaptchaContextType =>
-  useContext(RecaptchaContext);