Skip to content

Commit

Permalink
add code suggestions
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
andre-code committed Oct 17, 2024
1 parent 70836e9 commit be32e65
Show file tree
Hide file tree
Showing 13 changed files with 108 additions and 156 deletions.
47 changes: 23 additions & 24 deletions client/src/components/Logs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* limitations under the License.
*/

import React, { useEffect } from "react";
import React, { ReactNode, useEffect } from "react";
import {
Button,
Modal,
Expand Down Expand Up @@ -335,13 +335,27 @@ const EnvironmentLogs = ({ name, annotations }: EnvironmentLogsProps) => {
);
};

const cleanAnnotations = NotebooksHelper.cleanAnnotations(
annotations
) as NotebookAnnotations;

const modalTitle = !cleanAnnotations.renkuVersion && (
<div className="fs-5 fw-normal">
<small>
{cleanAnnotations["namespace"]}/{cleanAnnotations["projectName"]} [
{cleanAnnotations["branch"]}@
{cleanAnnotations["commit-sha"].substring(0, 8)}]
</small>
</div>
);

return (
<EnvironmentLogsPresent
fetchLogs={fetchLogs}
toggleLogs={toggleLogs}
logs={logs}
name={name}
annotations={annotations}
title={modalTitle}
/>
);
};
Expand All @@ -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<string, unknown>;
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 && (
<div className="fs-5 fw-normal">
<small>
{cleanAnnotations["namespace"]}/{cleanAnnotations["projectName"]} [
{cleanAnnotations["branch"]}@
{cleanAnnotations["commit-sha"].substring(0, 8)}]
</small>
</div>
);

return (
<Modal
isOpen={!!logs.show}
Expand All @@ -400,8 +400,7 @@ const EnvironmentLogsPresent = ({
toggleLogs(name);
}}
>
<div>Logs</div>
{modalTitle}
{title}
</ModalHeader>
<ModalBody>
<div className="mx-2">
Expand All @@ -410,6 +409,6 @@ const EnvironmentLogsPresent = ({
</ModalBody>
</Modal>
);
};
}

export { EnvironmentLogs, EnvironmentLogsPresent, SessionLogs };
47 changes: 4 additions & 43 deletions client/src/components/LogsV2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
);
Expand All @@ -47,50 +46,12 @@ export const EnvironmentLogsV2 = ({ name }: EnvironmentLogsPropsV2) => {
};

return (
<EnvironmentLogsPresentV2
<EnvironmentLogsPresent
fetchLogs={fetchLogs}
toggleLogs={toggleLogs}
logs={logs}
name={name}
title="Logs"
/>
);
};
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 (
<Modal
isOpen={!!logs.show}
className="modal-xl"
scrollable={true}
toggle={() => {
toggleLogs(name);
}}
>
<ModalHeader
className="header-multiline"
toggle={() => {
toggleLogs(name);
}}
>
<div>Logs</div>
</ModalHeader>
<ModalBody>
<div className="mx-2">
<SessionLogs fetchLogs={fetchLogs} logs={logs} name={name} />
</div>
</ModalBody>
</Modal>
);
};
12 changes: 4 additions & 8 deletions client/src/features/admin/AddSessionEnvironmentButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 2 additions & 6 deletions client/src/features/admin/UpdateSessionEnvironmentButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 2 additions & 6 deletions client/src/features/admin/adminSessions.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
54 changes: 31 additions & 23 deletions client/src/features/dashboardV2/DashboardV2Sessions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -36,37 +36,45 @@ export default function DashboardV2Sessions() {
return <SessionDashboardList sessions={sessions} />;
}

const LoadingState = () => (
<div className={cx("d-flex", "flex-column", "mx-auto")}>
<Loader />
<p className={cx("mx-auto", "my-3")}>Retrieving sessions...</p>
</div>
);
function LoadingState() {
return (
<div className={cx("d-flex", "flex-column", "mx-auto")}>
<Loader />
<p className={cx("mx-auto", "my-3")}>Retrieving sessions...</p>
</div>
);
}

const ErrorState = ({
function ErrorState({
error,
}: {
error: FetchBaseQueryError | SerializedError | undefined;
}) => (
<div>
<p>Cannot show sessions.</p>
<RtkErrorAlert error={error} />
</div>
);
}) {
return (
<div>
<p>Cannot show sessions.</p>
<RtkErrorAlert error={error} />
</div>
);
}

const NoSessionsState = () => <div>No running sessions.</div>;
function NoSessionsState() {
return <div>No running sessions.</div>;
}

const SessionDashboardList = ({
function SessionDashboardList({
sessions,
}: {
sessions: SessionList | undefined;
}) => (
<ListGroup flush data-cy="dashboard-session-list">
{sessions?.map((session) => (
<DashboardSession key={session.name} session={session} />
))}
</ListGroup>
);
}) {
return (
<ListGroup flush data-cy="dashboard-session-list">
{sessions?.map((session) => (
<DashboardSession key={session.name} session={session} />
))}
</ListGroup>
);
}

interface DashboardSessionProps {
session: SessionV2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
32 changes: 14 additions & 18 deletions client/src/features/sessionsV2/PauseOrDeleteSessionModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,32 +19,27 @@
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,
useNavigate,
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";
Expand Down Expand Up @@ -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 (
<>
<ModalBody>
Expand All @@ -273,13 +269,13 @@ function PauseSessionModalContent({
session (new and edited files) will be preserved while the session is
paused.
</p>
{session?.status?.will_hibernate_at &&
session?.status?.will_hibernate_at?.length > 0 && (
{/* TODO: Uncomment when hibernatedSecondsThreshold is available
{ hibernationThreshold > 0 && (
<InfoAlert dismissible={false} timeout={0}>
Please note that paused session are deleted after{" "}
Please note that paused sessions are deleted after{" "}
{hibernationThreshold} of inactivity.
</InfoAlert>
)}
)} */}
<div className="my-2">
<Button
className={cx("float-right", "p-0")}
Expand Down
Loading

0 comments on commit be32e65

Please sign in to comment.