Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Show children resources in visualization #230

Merged
merged 5 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,94 +1,33 @@
"use client";

import type { RouterOutputs } from "@ctrlplane/api";
import type * as SCHEMA from "@ctrlplane/db/schema";
import type { EdgeTypes, NodeTypes } from "reactflow";
import React from "react";
import { compact } from "lodash";
import ReactFlow, {
ReactFlowProvider,
useEdgesState,
useNodesState,
} from "reactflow";

import { useLayoutAndFitView } from "~/app/[workspaceSlug]/(app)/_components/reactflow/layout";
import { DepEdge } from "./DepEdge";
import {
createEdgeFromProviderToResource,
createEdgesFromResourceToEnvironments,
} from "./edges";
import { EnvironmentNode } from "./nodes/EnvironmentNode";
import { ProviderNode } from "./nodes/ProviderNode";
import { ResourceNode } from "./nodes/ResourceNode";
import { edgeTypes, getEdges } from "./edges";
import { getNodes, nodeTypes } from "./nodes/nodes";

type Relationships = NonNullable<RouterOutputs["resource"]["relationships"]>;

type ResourceVisualizationDiagramProps = {
resource: SCHEMA.Resource;
relationships: Relationships;
};

enum NodeType {
Resource = "resource",
Environment = "environment",
Provider = "provider",
}

const nodeTypes: NodeTypes = {
[NodeType.Resource]: ResourceNode,
[NodeType.Environment]: EnvironmentNode,
[NodeType.Provider]: ProviderNode,
};
const edgeTypes: EdgeTypes = { default: DepEdge };

export const ResourceVisualizationDiagram: React.FC<
ResourceVisualizationDiagramProps
> = ({ resource, relationships }) => {
const { workspace, provider } = relationships;
const { systems } = workspace;
> = ({ relationships }) => {
const [nodes, _, onNodesChange] = useNodesState<{ label: string }>(
compact([
{
id: resource.id,
type: NodeType.Resource,
data: { ...resource, label: resource.identifier },
position: { x: 0, y: 0 },
},
...systems.flatMap((system) =>
system.environments.map((env) => ({
id: env.id,
type: NodeType.Environment,
data: {
environment: {
...env,
deployments: system.deployments,
resource,
},
label: `${system.name}/${env.name}`,
},
position: { x: 0, y: 0 },
})),
),
provider != null && {
id: provider.id,
type: NodeType.Provider,
data: { ...provider, label: provider.name },
position: { x: 0, y: 0 },
},
]),
getNodes(relationships),
);

const resourceToEnvEdges = createEdgesFromResourceToEnvironments(
resource,
systems.flatMap((s) => s.environments),
);
const providerEdge = createEdgeFromProviderToResource(provider, resource);

const [edges, __, onEdgesChange] = useEdgesState(
compact([...resourceToEnvEdges, providerEdge]),
);
const [edges, __, onEdgesChange] = useEdgesState(getEdges(relationships));

const setReactFlowInstance = useLayoutAndFitView(nodes, { direction: "LR" });
const setReactFlowInstance = useLayoutAndFitView(nodes, { direction: "TB" });

return (
<ReactFlow
Expand Down
Original file line number Diff line number Diff line change
@@ -1,40 +1,118 @@
import type { RouterOutputs } from "@ctrlplane/api";
import type * as SCHEMA from "@ctrlplane/db/schema";
import type { EdgeTypes } from "reactflow";
import { MarkerType } from "reactflow";
import colors from "tailwindcss/colors";
import { isPresent } from "ts-is-present";

import { DepEdge } from "./DepEdge";

type Provider = SCHEMA.ResourceProvider & {
google: SCHEMA.ResourceProviderGoogle | null;
};

const markerEnd = {
type: MarkerType.Arrow,
color: colors.neutral[700],
color: colors.neutral[800],
};

export const createEdgesFromResourceToEnvironments = (
export const edgeTypes: EdgeTypes = { default: DepEdge };

const createEdgesFromResourceToEnvironments = (
resource: SCHEMA.Resource,
environments: SCHEMA.Environment[],
) =>
environments.map((environment) => ({
id: `${resource.id}-${environment.id}`,
source: resource.id,
target: environment.id,
style: { stroke: colors.neutral[700] },
style: { stroke: colors.neutral[800] },
markerEnd,
label: "in",
}));

export const createEdgeFromProviderToResource = (
const createEdgeFromProviderToResource = (
provider: Provider | null,
resource: SCHEMA.Resource,
) =>
provider != null
? {
id: `${provider.id}-${resource.id}`,
source: provider.id,
source: `${provider.id}-${resource.id}`,
target: resource.id,
style: { stroke: colors.neutral[700] },
style: { stroke: colors.neutral[800] },
markerEnd,
label: "discovered",
}
: null;

type Relationships = NonNullable<RouterOutputs["resource"]["relationships"]>;

const createEdgesFromEnvironmentToDeployments = (
environments: SCHEMA.Environment[],
deployments: SCHEMA.Deployment[],
) =>
environments
.flatMap((e) => deployments.map((d) => ({ e, d })))
.map(({ e, d }) => ({
id: `${e.id}-${d.id}`,
source: e.id,
target: `${e.id}-${d.id}`,
label: "deploys",
style: { stroke: colors.neutral[800] },
markerEnd,
}));

const createEdgesFromDeploymentsToResources = (relationships: Relationships) =>
relationships.map((resource) => {
const { parent } = resource;
if (parent == null) return null;

const allReleaseJobTriggers = relationships
.flatMap((r) => r.workspace.systems)
.flatMap((s) => s.environments)
.flatMap((e) => e.latestActiveReleases)
.map((rel) => rel.releaseJobTrigger);
Comment on lines +71 to +75
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add null checks for nested property access

The flatMap operations could throw runtime errors if any of the nested properties are undefined.

Apply these improvements:

    const allReleaseJobTriggers = relationships
-     .flatMap((r) => r.workspace.systems)
-     .flatMap((s) => s.environments)
-     .flatMap((e) => e.latestActiveReleases)
-     .map((rel) => rel.releaseJobTrigger);
+     .flatMap((r) => r.workspace?.systems ?? [])
+     .flatMap((s) => s?.environments ?? [])
+     .flatMap((e) => e?.latestActiveReleases ?? [])
+     .map((rel) => rel?.releaseJobTrigger)
+     .filter(isPresent);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const allReleaseJobTriggers = relationships
.flatMap((r) => r.workspace.systems)
.flatMap((s) => s.environments)
.flatMap((e) => e.latestActiveReleases)
.map((rel) => rel.releaseJobTrigger);
const allReleaseJobTriggers = relationships
.flatMap((r) => r.workspace?.systems ?? [])
.flatMap((s) => s?.environments ?? [])
.flatMap((e) => e?.latestActiveReleases ?? [])
.map((rel) => rel?.releaseJobTrigger)
.filter(isPresent);


const releaseJobTrigger = allReleaseJobTriggers.find(
(j) => j.jobId === parent.jobId,
);
if (releaseJobTrigger == null) return null;

const { deploymentId } = releaseJobTrigger.release;
const { environmentId } = releaseJobTrigger;

return {
id: `${releaseJobTrigger.jobId}-${resource.id}`,
source: `${environmentId}-${deploymentId}`,
target: resource.id,
style: { stroke: colors.neutral[800] },
markerEnd,
label: "created",
};
});

export const getEdges = (relationships: Relationships) => {
const resourceToEnvEdges = relationships.flatMap((r) =>
createEdgesFromResourceToEnvironments(
r,
r.workspace.systems.flatMap((s) => s.environments),
),
);
const environmentToDeploymentEdges = relationships.flatMap((r) =>
r.workspace.systems.flatMap((s) =>
createEdgesFromEnvironmentToDeployments(s.environments, s.deployments),
),
);
const providerEdges = relationships.flatMap((r) =>
r.provider != null ? [createEdgeFromProviderToResource(r.provider, r)] : [],
);
const deploymentEdges = createEdgesFromDeploymentsToResources(relationships);

return [
...resourceToEnvEdges,
...environmentToDeploymentEdges,
...providerEdges,
...deploymentEdges,
].filter(isPresent);
};
Comment on lines +95 to +118
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Add null checks and optimize edge creation

The edge creation has two issues:

  1. Unsafe property access in nested flatMap operations
  2. Multiple iterations over the relationships array

Apply these improvements:

export const getEdges = (relationships: Relationships) => {
+ // Pre-compute systems and environments to avoid multiple iterations
+ const systemsAndEnvs = relationships.flatMap((r) => 
+   r.workspace?.systems?.map(s => ({
+     system: s,
+     environments: s?.environments ?? [],
+     deployments: s?.deployments ?? []
+   })) ?? []
+ );

  const resourceToEnvEdges = relationships.flatMap((r) =>
    createEdgesFromResourceToEnvironments(
      r,
-     r.workspace.systems.flatMap((s) => s.environments),
+     systemsAndEnvs.flatMap(({ environments }) => environments),
    ),
  );

  const environmentToDeploymentEdges = systemsAndEnvs.flatMap(
-   r.workspace.systems.flatMap((s) =>
      createEdgesFromEnvironmentToDeployments(
        s.environments,
        s.deployments
      ),
    ),
  );

  const providerEdges = relationships.flatMap((r) =>
    r.provider != null ? [createEdgeFromProviderToResource(r.provider, r)] : [],
  );

  const deploymentEdges = createEdgesFromDeploymentsToResources(relationships);

  return [
    ...resourceToEnvEdges,
    ...environmentToDeploymentEdges,
    ...providerEdges,
    ...deploymentEdges,
  ].filter(isPresent);
};

Committable suggestion skipped: line range outside the PR's diff.

Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import type * as SCHEMA from "@ctrlplane/db/schema";
import type { NodeProps } from "reactflow";
import React from "react";
import { Handle, Position } from "reactflow";

import { cn } from "@ctrlplane/ui";
import { JobStatus, JobStatusReadable } from "@ctrlplane/validators/jobs";

import { useJobDrawer } from "~/app/[workspaceSlug]/(app)/_components/job-drawer/useJobDrawer";
import { api } from "~/trpc/react";
import { ReleaseIcon } from "../../ReleaseCell";

type DeploymentNodeProps = NodeProps<{
label: string;
deployment: SCHEMA.Deployment;
environment: SCHEMA.Environment;
resource: SCHEMA.Resource;
}>;

export const DeploymentNode: React.FC<DeploymentNodeProps> = ({ data }) => {
const { deployment, environment, resource } = data;
const { setJobId } = useJobDrawer();

const resourceId = resource.id;
const environmentId = environment.id;
const latestActiveReleasesQ =
api.resource.activeReleases.byResourceAndEnvironmentId.useQuery(
{ resourceId, environmentId },
{ refetchInterval: 5_000 },
);
const latestActiveReleases = latestActiveReleasesQ.data ?? [];
const activeRelease = latestActiveReleases.find(
(r) => r.releaseJobTrigger.release.deploymentId === deployment.id,
);

const isInProgress = latestActiveReleases.some(
(r) => r.releaseJobTrigger.job.status === JobStatus.InProgress,
);
const isPending = latestActiveReleases.some(
(r) => r.releaseJobTrigger.job.status === JobStatus.Pending,
);
const isCompleted = latestActiveReleases.every(
(r) => r.releaseJobTrigger.job.status === JobStatus.Completed,
);
Comment on lines +36 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve status computation logic robustness.

The current status computation has several potential issues:

  1. Status checks are not mutually exclusive
  2. Missing handling for failed/error states
  3. The order of checks could affect status determination

Consider this more robust approach:

- const isInProgress = latestActiveReleases.some(
-   (r) => r.releaseJobTrigger.job.status === JobStatus.InProgress,
- );
- const isPending = latestActiveReleases.some(
-   (r) => r.releaseJobTrigger.job.status === JobStatus.Pending,
- );
- const isCompleted = latestActiveReleases.every(
-   (r) => r.releaseJobTrigger.job.status === JobStatus.Completed,
- );

+ const getDeploymentStatus = (releases: typeof latestActiveReleases) => {
+   if (releases.length === 0) return 'none';
+   if (releases.some(r => r.releaseJobTrigger.job.status === JobStatus.Failed)) {
+     return 'failed';
+   }
+   if (releases.some(r => r.releaseJobTrigger.job.status === JobStatus.InProgress)) {
+     return 'in_progress';
+   }
+   if (releases.some(r => r.releaseJobTrigger.job.status === JobStatus.Pending)) {
+     return 'pending';
+   }
+   if (releases.every(r => r.releaseJobTrigger.job.status === JobStatus.Completed)) {
+     return 'completed';
+   }
+   return 'unknown';
+ };
+ 
+ const status = getDeploymentStatus(latestActiveReleases);
+ const isInProgress = status === 'in_progress';
+ const isPending = status === 'pending';
+ const isCompleted = status === 'completed';
+ const isFailed = status === 'failed';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const isInProgress = latestActiveReleases.some(
(r) => r.releaseJobTrigger.job.status === JobStatus.InProgress,
);
const isPending = latestActiveReleases.some(
(r) => r.releaseJobTrigger.job.status === JobStatus.Pending,
);
const isCompleted = latestActiveReleases.every(
(r) => r.releaseJobTrigger.job.status === JobStatus.Completed,
);
const getDeploymentStatus = (releases: typeof latestActiveReleases) => {
if (releases.length === 0) return 'none';
if (releases.some(r => r.releaseJobTrigger.job.status === JobStatus.Failed)) {
return 'failed';
}
if (releases.some(r => r.releaseJobTrigger.job.status === JobStatus.InProgress)) {
return 'in_progress';
}
if (releases.some(r => r.releaseJobTrigger.job.status === JobStatus.Pending)) {
return 'pending';
}
if (releases.every(r => r.releaseJobTrigger.job.status === JobStatus.Completed)) {
return 'completed';
}
return 'unknown';
};
const status = getDeploymentStatus(latestActiveReleases);
const isInProgress = status === 'in_progress';
const isPending = status === 'pending';
const isCompleted = status === 'completed';
const isFailed = status === 'failed';


const releaseJobTrigger = activeRelease?.releaseJobTrigger;

return (
<>
<div
className={cn(
"relative flex w-[250px] flex-col gap-2 rounded-md border border-neutral-800 bg-neutral-900/30 px-4 py-1",
isInProgress && "border-blue-500",
isPending && "border-neutral-500",
isCompleted && "border-green-500",
releaseJobTrigger == null && "border-neutral-800",
releaseJobTrigger != null && "cursor-pointer",
)}
onClick={() => {
if (releaseJobTrigger != null) setJobId(releaseJobTrigger.job.id);
}}
>
<div className="flex h-14 items-center">
<div className="flex min-w-0 flex-1 flex-grow items-center gap-2">
<span className="truncate">{deployment.name}</span>
</div>
{releaseJobTrigger != null && (
<div className="flex flex-shrink-0 items-center gap-2">
<ReleaseIcon job={releaseJobTrigger.job} />
<div className="text-sm">
<div>{releaseJobTrigger.release.version}</div>
<div>{JobStatusReadable[releaseJobTrigger.job.status]}</div>
</div>
</div>
)}
{releaseJobTrigger == null && (
<div className="flex flex-shrink-0 justify-end pr-4 text-sm text-muted-foreground">
No active job
</div>
)}
</div>
</div>
<Handle
type="target"
className={cn(
"h-2 w-2 rounded-full border border-neutral-800 bg-neutral-800",
isInProgress && "border-blue-500",
isPending && "border-neutral-500",
isCompleted && "border-green-500",
releaseJobTrigger == null && "border-neutral-800",
)}
position={Position.Top}
/>
<Handle
type="source"
position={Position.Bottom}
className={cn(
"h-2 w-2 rounded-full border border-neutral-800 bg-neutral-800",
isInProgress && "border-blue-500",
isPending && "border-neutral-500",
isCompleted && "border-green-500",
releaseJobTrigger == null && "border-neutral-800",
)}
/>
</>
);
Comment on lines +48 to +106
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance render logic with better code organization and accessibility.

Several improvements could be made to the render logic:

  1. Duplicate styling logic between div and handles
  2. Missing accessibility attributes
  3. Click handler could be extracted

Consider these improvements:

+ const getStatusStyles = (status: {
+   isInProgress: boolean;
+   isPending: boolean;
+   isCompleted: boolean;
+   hasJobTrigger: boolean;
+ }) => cn(
+   "border-neutral-800",
+   status.isInProgress && "border-blue-500",
+   status.isPending && "border-neutral-500",
+   status.isCompleted && "border-green-500",
+   !status.hasJobTrigger && "border-neutral-800",
+ );

+ const handleJobClick = (jobId: string | undefined) => {
+   if (jobId != null) setJobId(jobId);
+ };

  return (
    <>
      <div
+       role="button"
+       tabIndex={releaseJobTrigger ? 0 : -1}
+       aria-label={`Deployment: ${deployment.name}`}
        className={cn(
          "relative flex w-[250px] flex-col gap-2 rounded-md border bg-neutral-900/30 px-4 py-1",
-         isInProgress && "border-blue-500",
-         isPending && "border-neutral-500",
-         isCompleted && "border-green-500",
-         releaseJobTrigger == null && "border-neutral-800",
+         getStatusStyles({
+           isInProgress,
+           isPending,
+           isCompleted,
+           hasJobTrigger: releaseJobTrigger != null
+         }),
          releaseJobTrigger != null && "cursor-pointer",
        )}
-       onClick={() => {
-         if (releaseJobTrigger != null) setJobId(releaseJobTrigger.job.id);
-       }}
+       onClick={() => handleJobClick(releaseJobTrigger?.job.id)}
+       onKeyDown={(e) => {
+         if (e.key === 'Enter' || e.key === ' ') {
+           e.preventDefault();
+           handleJobClick(releaseJobTrigger?.job.id);
+         }
+       }}
      >

Committable suggestion skipped: line range outside the PR's diff.

};
Loading
Loading