From be32e653df830d26af677e146d1b211c4ef40db1 Mon Sep 17 00:00:00 2001 From: Andrea Cordoba Date: Thu, 17 Oct 2024 14:48:01 +0200 Subject: [PATCH] add code suggestions - use named functions - refactor working and mount directory handling with optional chaining and fallback to undefined - remove forced URL generation in ensureHTTPS function when the input is not a valid URL. - ensure session data is fetched when loading ShowSessionPage --- client/src/components/Logs.tsx | 47 ++++++++-------- client/src/components/LogsV2.tsx | 47 ++-------------- .../admin/AddSessionEnvironmentButton.tsx | 12 ++--- .../admin/UpdateSessionEnvironmentButton.tsx | 8 +-- .../src/features/admin/adminSessions.utils.ts | 8 +-- .../dashboardV2/DashboardV2Sessions.tsx | 54 +++++++++++-------- .../components/StartSessionProgressBar.tsx | 2 +- .../sessionsV2/PauseOrDeleteSessionModal.tsx | 32 +++++------ .../SessionShowPage/ShowSessionPage.tsx | 16 +++--- .../features/sessionsV2/SessionStartPage.tsx | 21 ++++---- .../SessionButton/ActiveSessionButton.tsx | 5 +- .../src/features/sessionsV2/session.utils.ts | 11 ++-- .../sessionsV2/useSessionLaunchState.hook.ts | 1 - 13 files changed, 108 insertions(+), 156 deletions(-) diff --git a/client/src/components/Logs.tsx b/client/src/components/Logs.tsx index 9d4992186..d7f6d66c9 100644 --- a/client/src/components/Logs.tsx +++ b/client/src/components/Logs.tsx @@ -16,7 +16,7 @@ * limitations under the License. */ -import React, { useEffect } from "react"; +import React, { ReactNode, useEffect } from "react"; import { Button, Modal, @@ -335,13 +335,27 @@ const EnvironmentLogs = ({ name, annotations }: EnvironmentLogsProps) => { ); }; + const cleanAnnotations = NotebooksHelper.cleanAnnotations( + annotations + ) as NotebookAnnotations; + + const modalTitle = !cleanAnnotations.renkuVersion && ( +
+ + {cleanAnnotations["namespace"]}/{cleanAnnotations["projectName"]} [ + {cleanAnnotations["branch"]}@ + {cleanAnnotations["commit-sha"].substring(0, 8)}] + +
+ ); + return ( ); }; @@ -353,38 +367,24 @@ const EnvironmentLogs = ({ name, annotations }: EnvironmentLogsProps) => { * @param {function} toggleLogs - toggle logs visibility and fetch logs on show * @param {object} logs - log object from redux store enhanced with `show` property * @param {string} name - server name - * @param {object} annotations - list of annotations + * @param {ReactNode | string} title - modal title */ interface EnvironmentLogsPresentProps { - annotations: Record; + title: ReactNode | string; fetchLogs: IFetchableLogs["fetchLogs"]; logs?: ILogs; name: string; toggleLogs: (name: string) => unknown; } -const EnvironmentLogsPresent = ({ +function EnvironmentLogsPresent({ logs, name, toggleLogs, fetchLogs, - annotations, -}: EnvironmentLogsPresentProps) => { + title, +}: EnvironmentLogsPresentProps) { if (!logs?.show || logs?.show !== name || !logs) return null; - const cleanAnnotations = NotebooksHelper.cleanAnnotations( - annotations - ) as NotebookAnnotations; - - const modalTitle = !cleanAnnotations.renkuVersion && ( -
- - {cleanAnnotations["namespace"]}/{cleanAnnotations["projectName"]} [ - {cleanAnnotations["branch"]}@ - {cleanAnnotations["commit-sha"].substring(0, 8)}] - -
- ); - return ( -
Logs
- {modalTitle} + {title}
@@ -410,6 +409,6 @@ const EnvironmentLogsPresent = ({ ); -}; +} export { EnvironmentLogs, EnvironmentLogsPresent, SessionLogs }; diff --git a/client/src/components/LogsV2.tsx b/client/src/components/LogsV2.tsx index cb4f0600b..8a1e5a8cf 100644 --- a/client/src/components/LogsV2.tsx +++ b/client/src/components/LogsV2.tsx @@ -16,12 +16,11 @@ * limitations under the License. */ -import { Modal, ModalBody, ModalHeader } from "reactstrap"; import { displaySlice } from "../features/display"; import useAppDispatch from "../utils/customHooks/useAppDispatch.hook"; import useAppSelector from "../utils/customHooks/useAppSelector.hook"; import { useGetSessionLogsV2 } from "../utils/customHooks/UseGetSessionLogs"; -import { IFetchableLogs, ILogs, SessionLogs } from "./Logs"; +import { EnvironmentLogsPresent } from "./Logs"; /** * Sessions logs container integrating state and actions V2 @@ -31,7 +30,7 @@ import { IFetchableLogs, ILogs, SessionLogs } from "./Logs"; interface EnvironmentLogsPropsV2 { name: string; } -export const EnvironmentLogsV2 = ({ name }: EnvironmentLogsPropsV2) => { +export default function EnvironmentLogsV2({ name }: EnvironmentLogsPropsV2) { const displayModal = useAppSelector( ({ display }) => display.modals.sessionLogs ); @@ -47,50 +46,12 @@ export const EnvironmentLogsV2 = ({ name }: EnvironmentLogsPropsV2) => { }; return ( - ); -}; -interface EnvironmentLogsPresentV2Props { - fetchLogs: IFetchableLogs["fetchLogs"]; - logs?: ILogs; - name: string; - toggleLogs: (name: string) => unknown; } -const EnvironmentLogsPresentV2 = ({ - logs, - name, - toggleLogs, - fetchLogs, -}: EnvironmentLogsPresentV2Props) => { - if (!logs?.show || logs?.show !== name || !logs) return null; - - return ( - { - toggleLogs(name); - }} - > - { - toggleLogs(name); - }} - > -
Logs
-
- -
- -
-
-
- ); -}; diff --git a/client/src/features/admin/AddSessionEnvironmentButton.tsx b/client/src/features/admin/AddSessionEnvironmentButton.tsx index 37d8fd941..484f26cfa 100644 --- a/client/src/features/admin/AddSessionEnvironmentButton.tsx +++ b/client/src/features/admin/AddSessionEnvironmentButton.tsx @@ -86,15 +86,11 @@ function AddSessionEnvironmentModal({ addSessionEnvironment({ container_image: data.container_image, name: data.name, - default_url: data.default_url?.trim() ? data.default_url : undefined, - description: data.description?.trim() ? data.description : undefined, + default_url: data.default_url?.trim() || undefined, + description: data.description?.trim() || undefined, port: data.port ?? undefined, - working_directory: data.working_directory?.trim() - ? data.working_directory - : undefined, - mount_directory: data.mount_directory?.trim() - ? data.mount_directory - : undefined, + working_directory: data.working_directory?.trim() || undefined, + mount_directory: data.mount_directory?.trim() || undefined, uid: data.uid ?? undefined, gid: data.gid ?? undefined, command: commandParsed.data, diff --git a/client/src/features/admin/UpdateSessionEnvironmentButton.tsx b/client/src/features/admin/UpdateSessionEnvironmentButton.tsx index f5c181fff..685af6bbc 100644 --- a/client/src/features/admin/UpdateSessionEnvironmentButton.tsx +++ b/client/src/features/admin/UpdateSessionEnvironmentButton.tsx @@ -108,12 +108,8 @@ function UpdateSessionEnvironmentModal({ default_url: data.default_url?.trim() ? data.default_url : "", description: data.description?.trim() ? data.description : "", port: data.port ?? undefined, - working_directory: data.working_directory?.trim() - ? data.working_directory - : undefined, - mount_directory: data.mount_directory?.trim() - ? data.mount_directory - : undefined, + working_directory: data.working_directory?.trim() || undefined, + mount_directory: data.mount_directory?.trim() || undefined, uid: data.uid ?? undefined, gid: data.gid ?? undefined, command: commandParsed.data, diff --git a/client/src/features/admin/adminSessions.utils.ts b/client/src/features/admin/adminSessions.utils.ts index e0e8a2c5b..c1d136ecc 100644 --- a/client/src/features/admin/adminSessions.utils.ts +++ b/client/src/features/admin/adminSessions.utils.ts @@ -26,12 +26,8 @@ export function getSessionEnvironmentValues(environment: SessionEnvironment) { description: environment.description, name: environment.name, port: environment.port ?? undefined, - working_directory: environment.working_directory?.trim() - ? environment.working_directory - : undefined, - mount_directory: environment.mount_directory?.trim() - ? environment.mount_directory - : undefined, + working_directory: environment.working_directory?.trim() || undefined, + mount_directory: environment.mount_directory?.trim() || undefined, uid: environment.uid ?? undefined, gid: environment.gid ?? undefined, command: getJSONStringArray(environment.command), diff --git a/client/src/features/dashboardV2/DashboardV2Sessions.tsx b/client/src/features/dashboardV2/DashboardV2Sessions.tsx index 0e220929f..bf145228e 100644 --- a/client/src/features/dashboardV2/DashboardV2Sessions.tsx +++ b/client/src/features/dashboardV2/DashboardV2Sessions.tsx @@ -4,7 +4,7 @@ import cx from "classnames"; import { Link, generatePath } from "react-router-dom-v5-compat"; import { Col, ListGroup, Row } from "reactstrap"; import { Loader } from "../../components/Loader"; -import { EnvironmentLogsV2 } from "../../components/LogsV2"; +import EnvironmentLogsV2 from "../../components/LogsV2"; import { RtkErrorAlert } from "../../components/errors/RtkErrorAlert"; import { useGetSessionsQuery as useGetSessionsQueryV2 } from "../../features/sessionsV2/sessionsV2.api"; import "../../notebooks/Notebooks.css"; @@ -36,37 +36,45 @@ export default function DashboardV2Sessions() { return ; } -const LoadingState = () => ( -
- -

Retrieving sessions...

-
-); +function LoadingState() { + return ( +
+ +

Retrieving sessions...

+
+ ); +} -const ErrorState = ({ +function ErrorState({ error, }: { error: FetchBaseQueryError | SerializedError | undefined; -}) => ( -
-

Cannot show sessions.

- -
-); +}) { + return ( +
+

Cannot show sessions.

+ +
+ ); +} -const NoSessionsState = () =>
No running sessions.
; +function NoSessionsState() { + return
No running sessions.
; +} -const SessionDashboardList = ({ +function SessionDashboardList({ sessions, }: { sessions: SessionList | undefined; -}) => ( - - {sessions?.map((session) => ( - - ))} - -); +}) { + return ( + + {sessions?.map((session) => ( + + ))} + + ); +} interface DashboardSessionProps { session: SessionV2; diff --git a/client/src/features/session/components/StartSessionProgressBar.tsx b/client/src/features/session/components/StartSessionProgressBar.tsx index 8498c54bd..89af35a1a 100644 --- a/client/src/features/session/components/StartSessionProgressBar.tsx +++ b/client/src/features/session/components/StartSessionProgressBar.tsx @@ -72,7 +72,7 @@ export function StartSessionProgressBarV2({ const statusData = session?.status; const description = statusData?.ready_containers && statusData?.total_containers - ? `${statusData?.ready_containers} of ${statusData?.total_containers} containers ready` + ? `${statusData.ready_containers} of ${statusData.total_containers} containers ready` : "Loading containers status"; return ( diff --git a/client/src/features/sessionsV2/PauseOrDeleteSessionModal.tsx b/client/src/features/sessionsV2/PauseOrDeleteSessionModal.tsx index 6f0554880..debd472ab 100644 --- a/client/src/features/sessionsV2/PauseOrDeleteSessionModal.tsx +++ b/client/src/features/sessionsV2/PauseOrDeleteSessionModal.tsx @@ -19,7 +19,6 @@ import { SerializedError } from "@reduxjs/toolkit"; import { FetchBaseQueryError } from "@reduxjs/toolkit/query"; import cx from "classnames"; -import { DateTime } from "luxon"; import { useCallback, useContext, useEffect, useState } from "react"; import { generatePath, @@ -27,24 +26,20 @@ import { useParams, } from "react-router-dom-v5-compat"; import { Button, Modal, ModalBody, ModalFooter, ModalHeader } from "reactstrap"; - -import { InfoAlert } from "../../components/Alert"; -import { Loader } from "../../components/Loader"; import { User } from "../../model/renkuModels.types"; import { NOTIFICATION_TOPICS } from "../../notifications/Notifications.constants"; import { NotificationsManager } from "../../notifications/notifications.types"; import { ABSOLUTE_ROUTES } from "../../routing/routes.constants"; import AppContext from "../../utils/context/appContext"; import useLegacySelector from "../../utils/customHooks/useLegacySelector.hook"; -import { toHumanRelativeDuration } from "../../utils/helpers/DurationUtils"; +import styles from "../session/components/SessionModals.module.scss"; import { useWaitForSessionStatusV2 } from "../session/useWaitForSessionStatus.hook"; import { usePatchSessionMutation, useStopSessionMutation, } from "../sessionsV2/sessionsV2.api"; - -import styles from "../session/components/SessionModals.module.scss"; import { SessionV2 } from "./sessionsV2.types"; +import { Loader } from "../../components/Loader"; interface PauseOrDeleteSessionModalProps { action?: "pause" | "delete"; @@ -258,13 +253,14 @@ function PauseSessionModalContent({ } }, [backUrl, isSuccess, isWaiting, navigate]); - const now = DateTime.utc(); - const hibernationThreshold = session?.status?.will_hibernate_at - ? toHumanRelativeDuration({ - datetime: session?.status?.will_hibernate_at, - now, - }) - : 0; + // TODO: Uncomment when hibernatedSecondsThreshold is available + // const now = DateTime.utc(); + // const hibernationThreshold = session?.status?.hibernatedSecondsThreshold + // ? toHumanRelativeDuration({ + // datetime: session?.status?.hibernatedSecondsThreshold, + // now, + // }) + // : 0; return ( <> @@ -273,13 +269,13 @@ function PauseSessionModalContent({ session (new and edited files) will be preserved while the session is paused.

- {session?.status?.will_hibernate_at && - session?.status?.will_hibernate_at?.length > 0 && ( + {/* TODO: Uncomment when hibernatedSecondsThreshold is available + { hibernationThreshold > 0 && ( - Please note that paused session are deleted after{" "} + Please note that paused sessions are deleted after{" "} {hibernationThreshold} of inactivity. - )} + )} */}