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.

Sign up for GitHub

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: Visualization diagram v2 #228

Merged
merged 8 commits into from
Nov 25, 2024
Merged

fix: Visualization diagram v2 #228

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
342fc9e
init
adityachoudhari26 Nov 24, 2024
6504257
more change
adityachoudhari26 Nov 24, 2024
eb2bde5
clenaup
adityachoudhari26 Nov 24, 2024
9d79f40
smaller skeleont
adityachoudhari26 Nov 24, 2024
f7583f4
add explicit typing for lint
adityachoudhari26 Nov 24, 2024
e5e1bbf
Merge branch 'main' of github.com:sizzldev/ctrlplane into visualizati…
adityachoudhari26 Nov 24, 2024
7905c34
fix typing
adityachoudhari26 Nov 24, 2024
47c2446
updatge
adityachoudhari26 Nov 25, 2024
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
42 changes: 40 additions & 2 deletions apps/webservice/src/app/[workspaceSlug]/(app)/(targets)/targets/[targetId]/ReleaseCell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,19 @@ import type {
} from "@ctrlplane/db/schema";
import Link from "next/link";
import { useParams } from "next/navigation";
import { IconCircleCheck, IconClock, IconLoader2 } from "@tabler/icons-react";
import {
IconAlertCircle,
IconCircleCheck,
IconCircleDashed,
IconCircleX,
IconClock,
IconLoader2,
} from "@tabler/icons-react";
import { format } from "date-fns";

import { JobStatus } from "@ctrlplane/validators/jobs";

const ReleaseIcon: React.FC<{
export const ReleaseIcon: React.FC<{
job?: Job;
}> = ({ job }) => {
if (job?.status === JobStatus.Pending)
Expand All @@ -37,6 +44,37 @@ const ReleaseIcon: React.FC<{
</div>
);

if (job?.status === JobStatus.Cancelled)
return (
<div className="rounded-full bg-neutral-400 p-1 dark:text-black">
<IconCircleX strokeWidth={2} />
</div>
);

if (job?.status === JobStatus.Failure)
return (
<div className="rounded-full bg-red-400 p-1 dark:text-black">
<IconCircleX strokeWidth={2} />
</div>
);

if (job?.status === JobStatus.Skipped)
return (
<div className="rounded-full bg-neutral-400 p-1 dark:text-black">
<IconCircleDashed strokeWidth={2} />
</div>
);

if (
job?.status === JobStatus.InvalidJobAgent ||
job?.status === JobStatus.InvalidIntegration
)
return (
<div className="rounded-full bg-orange-500 p-1 dark:text-black">
<IconAlertCircle strokeWidth={2} />
</div>
);

return null;
};

Expand Down
12 changes: 7 additions & 5 deletions apps/webservice/src/app/[workspaceSlug]/(app)/(targets)/targets/[targetId]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ import { useMatchSorterWithSearch } from "~/utils/useMatchSorter";
import { ReleaseCell } from "./ReleaseCell";

const DeploymentsTable: React.FC<{ targetId: string }> = ({ targetId }) => {
const jobs = api.job.byResourceId.useQuery(targetId);
const deployments = api.deployment.byTargetId.useQuery(targetId);
const resourceId = targetId;
const jobs = api.job.byResourceId.useQuery(resourceId);
const deployments = api.deployment.byTargetId.useQuery({ resourceId });
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent parameter passing detected in API calls

There are inconsistencies in how resourceId is being passed to API queries:

  • api.job.byResourceId.useQuery() takes resourceId directly
  • api.deployment.byTargetId.useQuery() takes { resourceId } as an object
  • api.deployment.variable.byTargetId.useQuery() takes resourceId directly

Additionally, there's one instance in JobsContent.tsx still using targetId instead of resourceId:

const jobs = api.job.byResourceId.useQuery(targetId);
🔗 Analysis chain

Verify consistent usage of resourceId across the codebase.

Let's ensure the refactoring from targetId to resourceId is consistently applied across all components and API calls.

Also applies to: 110-113

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining instances of targetId in API calls and verify resourceId usage

# Check for remaining targetId usage in API calls
echo "Checking for targetId in API calls..."
rg -A 2 "api\.[a-zA-Z]+\.byTargetId\.useQuery" 

# Check for resourceId usage pattern
echo "Checking resourceId usage pattern..."
rg -A 2 "api\.[a-zA-Z]+\.byResourceId\.useQuery"

# Check for potential inconsistencies in parameter passing
echo "Checking parameter passing patterns..."
rg "\.useQuery\(\{.*resourceId"

Length of output: 4944

return (
<Table className="w-full min-w-max border-separate border-spacing-0">
<TableBody>
Expand Down Expand Up @@ -106,9 +107,10 @@ export default function TargetPage({
}: {
params: { targetId: string };
}) {
const target = api.resource.byId.useQuery(params.targetId);
const jobs = api.job.byResourceId.useQuery(params.targetId);
const deployments = api.deployment.byTargetId.useQuery(params.targetId);
const resourceId = params.targetId;
const target = api.resource.byId.useQuery(resourceId);
const jobs = api.job.byResourceId.useQuery(resourceId);
const deployments = api.deployment.byTargetId.useQuery({ resourceId });
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

Fix error handling and standardize API parameters.

Two issues to address:

  1. The target query result is used in the notFound() check before confirming loading is complete.
  2. The API queries show inconsistent parameter passing patterns.
   const resourceId = params.targetId;
   const target = api.resource.byId.useQuery(resourceId);
   const jobs = api.job.byResourceId.useQuery(resourceId);
-  const deployments = api.deployment.byTargetId.useQuery({ resourceId });
+  const deployments = api.deployment.byTargetId.useQuery(resourceId);

   const unlockTarget = api.resource.unlock.useMutation();
   const lockTarget = api.resource.lock.useMutation();

   const utils = api.useUtils();

   const isLoading = target.isLoading || jobs.isLoading || deployments.isLoading;

-  if (!target.isLoading && target.data == null) notFound();
-  if (isLoading)
+  if (isLoading) {
     return (
       <div className="flex h-full w-full items-center justify-center">
         <IconLoader2 className="h-8 w-8 animate-spin" />
       </div>
     );
+  }
+  
+  if (target.data == null) {
+    notFound();
+  }
📝 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 resourceId = params.targetId;
const target = api.resource.byId.useQuery(resourceId);
const jobs = api.job.byResourceId.useQuery(resourceId);
const deployments = api.deployment.byTargetId.useQuery({ resourceId });
const resourceId = params.targetId;
const target = api.resource.byId.useQuery(resourceId);
const jobs = api.job.byResourceId.useQuery(resourceId);
const deployments = api.deployment.byTargetId.useQuery(resourceId);
const unlockTarget = api.resource.unlock.useMutation();
const lockTarget = api.resource.lock.useMutation();
const utils = api.useUtils();
const isLoading = target.isLoading || jobs.isLoading || deployments.isLoading;
if (isLoading) {
return (
<div className="flex h-full w-full items-center justify-center">
<IconLoader2 className="h-8 w-8 animate-spin" />
</div>
);
}
if (target.data == null) {
notFound();
}


const unlockTarget = api.resource.unlock.useMutation();
const lockTarget = api.resource.lock.useMutation();
Expand Down
7 changes: 3 additions & 4 deletions .../webservice/src/app/[workspaceSlug]/(app)/(targets)/targets/[targetId]/variables/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@ export default function VariablePage({
}: {
params: { targetId: string };
}) {
const deployments = api.deployment.byTargetId.useQuery(params.targetId);
const variables = api.deployment.variable.byTargetId.useQuery(
params.targetId,
);
const resourceId = params.targetId;
const deployments = api.deployment.byTargetId.useQuery({ resourceId });
const variables = api.deployment.variable.byTargetId.useQuery(resourceId);
return (
<div className="">
{deployments.data?.map((deployment) => {
Expand Down
42 changes: 10 additions & 32 deletions ...kspaceSlug]/(app)/(targets)/targets/[targetId]/visualize/ResourceVisualizationDiagram.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,11 @@
import { DepEdge } from "./DepEdge";
import {
createEdgeFromProviderToResource,
createEdgesFromEnvironmentsToSystems,
createEdgesFromResourceToEnvironments,
createEdgesFromSystemsToDeployments,
} from "./edges";
import { DeploymentNode } from "./nodes/DeploymentNode";
import { EnvironmentNode } from "./nodes/EnvironmentNode";
import { ProviderNode } from "./nodes/ProviderNode";
import { ResourceNode } from "./nodes/ResourceNode";
import { SystemNode } from "./nodes/SystemNode";

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

Expand All @@ -36,16 +32,12 @@
Resource = "resource",
Environment = "environment",
Provider = "provider",
System = "system",
Deployment = "deployment",
}

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

Expand All @@ -61,25 +53,18 @@
data: { ...resource, label: resource.identifier },
position: { x: 0, y: 0 },
},
...systems.flatMap((system) =>

Check failure on line 56 in apps/webservice/src/app/[workspaceSlug]/(app)/(targets)/targets/[targetId]/visualize/ResourceVisualizationDiagram.tsx

View workflow job for this annotation

GitHub Actions / Lint 

Unsafe call of a(n) `error` type typed value
system.environments.map((env) => ({

Check failure on line 57 in apps/webservice/src/app/[workspaceSlug]/(app)/(targets)/targets/[targetId]/visualize/ResourceVisualizationDiagram.tsx

View workflow job for this annotation

GitHub Actions / Lint 

Unsafe call of a(n) `any` typed value
id: env.id,
type: NodeType.Environment,
data: { ...env, label: env.name },
position: { x: 0, y: 0 },
})),
),
...systems.map((system) => ({
id: system.id,
type: NodeType.System,
data: { ...system, label: system.name },
position: { x: 0, y: 0 },
})),
...systems.flatMap((system) =>
system.deployments.map((deployment) => ({
id: deployment.id,
type: NodeType.Deployment,
data: { ...deployment, label: deployment.name },
data: {
environment: {
...env,
deployments: system.deployments,
resource,
},
label: `${system.name}/${env.name}`,
},
position: { x: 0, y: 0 },
})),
),
Expand All @@ -94,22 +79,15 @@

const resourceToEnvEdges = createEdgesFromResourceToEnvironments(
resource,
systems.flatMap((s) => s.environments),

Check failure on line 82 in apps/webservice/src/app/[workspaceSlug]/(app)/(targets)/targets/[targetId]/visualize/ResourceVisualizationDiagram.tsx

View workflow job for this annotation

GitHub Actions / Lint 

Unsafe call of a(n) `error` type typed value
);
const envToSystemEdges = createEdgesFromEnvironmentsToSystems(systems);
const systemToDeploymentsEdges = createEdgesFromSystemsToDeployments(systems);
const providerEdge = createEdgeFromProviderToResource(provider, resource);

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

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

return (
<ReactFlow
Expand Down
29 changes: 0 additions & 29 deletions .../webservice/src/app/[workspaceSlug]/(app)/(targets)/targets/[targetId]/visualize/edges.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,35 +24,6 @@ export const createEdgesFromResourceToEnvironments = (
label: "in",
}));

type System = SCHEMA.System & {
environments: SCHEMA.Environment[];
deployments: SCHEMA.Deployment[];
};

export const createEdgesFromEnvironmentsToSystems = (systems: System[]) =>
systems.flatMap((system) =>
system.environments.map((environment) => ({
id: `${environment.id}-${system.id}`,
source: environment.id,
target: system.id,
style: { stroke: colors.neutral[700] },
markerEnd,
label: "part of",
})),
);

export const createEdgesFromSystemsToDeployments = (systems: System[]) =>
systems.flatMap((system) =>
system.deployments.map((deployment) => ({
id: `${system.id}-${deployment.id}`,
source: system.id,
target: deployment.id,
style: { stroke: colors.neutral[700] },
markerEnd,
label: "deploys",
})),
);

export const createEdgeFromProviderToResource = (
provider: Provider | null,
resource: SCHEMA.Resource,
Expand Down
38 changes: 0 additions & 38 deletions ...app/[workspaceSlug]/(app)/(targets)/targets/[targetId]/visualize/nodes/DeploymentNode.tsx
View file
Open in desktop

This file was deleted.

Loading
Loading