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 latest release by deployment and environment #169

Merged
merged 9 commits into from
Oct 26, 2024
Merged
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
@@ -148,13 +148,13 @@ const ReleaseSelector: React.FC<{

const DeploymentNode: React.FC<
NodeProps<
Deployment & { latestRelease: { id: string; version: string } | null }
Deployment & { latestActiveRelease: { id: string; version: string } | null }
>
> = ({ data }) => {
const { getEdges, setEdges } = useReactFlow();

const [selectedRelease, setSelectedRelease] = useState(
data.latestRelease?.id,
data.latestActiveRelease?.id,
);
const releases = api.release.list.useQuery({ deploymentId: data.id });
const release = releases.data?.items.find((r) => r.id === selectedRelease);
@@ -163,7 +163,7 @@ const DeploymentNode: React.FC<
useEffect(() => {
if (release == null) return;
const deps = release.releaseDependencies.filter(isPresent);
if (data.latestRelease == null) return;
if (data.latestActiveRelease == null) return;

const edges = getEdges().filter((e) => e.source !== data.id);

@@ -193,7 +193,7 @@ const DeploymentNode: React.FC<
<div>
{releases.data != null && (
<ReleaseSelector
value={selectedRelease ?? data.latestRelease?.id ?? ""}
value={selectedRelease ?? data.latestActiveRelease?.id ?? ""}
onChange={setSelectedRelease}
releases={releases.data.items}
/>
@@ -222,7 +222,7 @@ const edgeTypes = { default: DepEdge };
const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms));
const DependencyDiagram: React.FC<{
deployments: Array<
Deployment & { latestRelease: { id: string; version: string } | null }
Deployment & { latestActiveRelease: { id: string; version: string } | null }
>;
}> = ({ deployments }) => {
const [nodes, _, onNodesChange] = useNodesState(
@@ -262,7 +262,7 @@ const DependencyDiagram: React.FC<{

export const Diagram: React.FC<{
deployments: Array<
Deployment & { latestRelease: { id: string; version: string } | null }
Deployment & { latestActiveRelease: { id: string; version: string } | null }
>;
}> = ({ deployments }) => {
return (
12 changes: 10 additions & 2 deletions apps/webservice/src/app/[workspaceSlug]/dependencies/page.tsx
Original file line number Diff line number Diff line change
@@ -15,13 +15,21 @@ export default async function Dependencies({

if (
deployments.length === 0 ||
deployments.some((d) => d.latestRelease != null)
deployments.some((d) => d.latestActiveReleases != null)
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 incorrect condition logic for showing getting started guide.

The current condition shows the getting started guide when there ARE active releases, which seems backwards. The getting started guide should typically be shown when there are no active releases.

Apply one of these fixes:

- deployments.some((d) => d.latestActiveReleases != null)
+ !deployments.some((d) => d.latestActiveReleases != null)

Or alternatively:

- deployments.some((d) => d.latestActiveReleases != null)
+ deployments.every((d) => d.latestActiveReleases == null)
📝 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
deployments.some((d) => d.latestActiveReleases != null)
!deployments.some((d) => d.latestActiveReleases != null)

)
return <DependenciesGettingStarted />;

const transformedDeployments = deployments.map((deployment) => ({
...deployment,
latestActiveRelease: deployment.latestActiveReleases && {
id: deployment.latestActiveReleases.id,
version: deployment.latestActiveReleases.version,
},
}));

return (
<div className="h-full">
<Diagram deployments={deployments} />
<Diagram deployments={transformedDeployments} />
</div>
);
}
Original file line number Diff line number Diff line change
@@ -121,12 +121,13 @@ const DeploymentTable: React.FC<{
environments: Array<Environment & { targets: Target[] }>;
deployments: Array<
Deployment & {
latestRelease: {
activeReleases: Array<{
id: string;
name: string;
version: string;
createdAt: Date;
} | null;
environmentId: string;
}> | null;
}
>;
workspaceSlug: string;
@@ -177,6 +178,9 @@ const DeploymentTable: React.FC<{
</td>

{environments.map((env, envIdx) => {
const release =
r.activeReleases?.find((r) => r.environmentId === env.id) ??
null;
return (
<td
key={env.id}
@@ -188,7 +192,7 @@ const DeploymentTable: React.FC<{
)}
>
<ReleaseCell
release={r.latestRelease}
release={release}
environment={env}
deployment={r}
workspaceSlug={workspaceSlug}
41 changes: 27 additions & 14 deletions packages/api/src/router/deployment.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import type { Tx } from "@ctrlplane/db";
import _ from "lodash";
import { isPresent } from "ts-is-present";
import { z } from "zod";

import {
@@ -30,7 +32,7 @@ import { JobStatus } from "@ctrlplane/validators/jobs";
import { createTRPCRouter, protectedProcedure } from "../trpc";
import { deploymentVariableRouter } from "./deployment-variable";

const latestReleaseSubQuery = (db: Tx) =>
const latestActiveReleaseSubQuery = (db: Tx) =>
db
.select({
id: release.id,
@@ -39,13 +41,15 @@ const latestReleaseSubQuery = (db: Tx) =>
createdAt: release.createdAt,
name: release.name,
config: release.config,
environmentId: releaseJobTrigger.environmentId,

rank: sql<number>`ROW_NUMBER() OVER (PARTITION BY deployment_id ORDER BY created_at DESC)`.as(
rank: sql<number>`ROW_NUMBER() OVER (PARTITION BY ${release.deploymentId}, ${releaseJobTrigger.environmentId} ORDER BY ${release.createdAt} DESC)`.as(
"rank",
),
})
.from(release)
.as("release");
.innerJoin(releaseJobTrigger, eq(releaseJobTrigger.releaseId, release.id))
.as("active_releases");

export const deploymentRouter = createTRPCRouter({
variable: deploymentVariableRouter,
@@ -203,20 +207,26 @@ export const deploymentRouter = createTRPCRouter({
.on({ type: "system", id: input }),
})
.query(async ({ ctx, input }) => {
const latestRelease = latestReleaseSubQuery(ctx.db);
const activeRelease = latestActiveReleaseSubQuery(ctx.db);
return ctx.db
.select()
.from(deployment)
.leftJoin(
latestRelease,
activeRelease,
and(
eq(latestRelease.deploymentId, deployment.id),
eq(latestRelease.rank, 1),
eq(activeRelease.deploymentId, deployment.id),
eq(activeRelease.rank, 1),
),
)
.where(eq(deployment.systemId, input))
.then((r) =>
r.map((row) => ({ ...row.deployment, latestRelease: row.release })),
.then((ts) =>
_.chain(ts)
.groupBy((t) => t.deployment.id)
.map((t) => ({
...t[0]!.deployment,
activeReleases: t.map((a) => a.active_releases).filter(isPresent),
}))
.value(),
Comment on lines +222 to +229
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

Handle potential empty groups to prevent runtime errors

The use of t[0]! assumes that each group has at least one element. This could lead to runtime errors if t is empty.

Apply this diff to add a null check:

            .groupBy((t) => t.deployment.id)
            .map((t) => ({
-              ...t[0]!.deployment,
+              ...(t[0]?.deployment ?? {}),
               activeReleases: t.map((a) => a.active_releases).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
.then((ts) =>
_.chain(ts)
.groupBy((t) => t.deployment.id)
.map((t) => ({
...t[0]!.deployment,
activeReleases: t.map((a) => a.active_releases).filter(isPresent),
}))
.value(),
.then((ts) =>
_.chain(ts)
.groupBy((t) => t.deployment.id)
.map((t) => ({
...(t[0]?.deployment ?? {}),
activeReleases: t.map((a) => a.active_releases).filter(isPresent),
}))
.value(),

);
}),

@@ -304,21 +314,24 @@ export const deploymentRouter = createTRPCRouter({
})
.input(z.string().uuid())
.query(async ({ ctx, input }) => {
const latestRelease = latestReleaseSubQuery(ctx.db);
const activeRelease = latestActiveReleaseSubQuery(ctx.db);
return ctx.db
.select()
.from(deployment)
.innerJoin(system, eq(system.id, deployment.systemId))
.leftJoin(
latestRelease,
activeRelease,
and(
eq(latestRelease.deploymentId, deployment.id),
eq(latestRelease.rank, 1),
eq(activeRelease.deploymentId, deployment.id),
eq(activeRelease.rank, 1),
),
)
.where(eq(system.workspaceId, input))
.then((r) =>
r.map((row) => ({ ...row.deployment, latestRelease: row.release })),
r.map((row) => ({
...row.deployment,
latestActiveReleases: row.active_releases,
})),
Comment on lines +331 to +334
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 inconsistent property naming

The property name latestActiveReleases is inconsistent with activeReleases used in the bySystemId procedure. This could cause confusion and maintenance issues.

Apply this diff to maintain consistency:

          r.map((row) => ({
            ...row.deployment,
-           latestActiveReleases: row.active_releases,
+           activeReleases: row.active_releases,
          })),
📝 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
r.map((row) => ({
...row.deployment,
latestActiveReleases: row.active_releases,
})),
r.map((row) => ({
...row.deployment,
activeReleases: row.active_releases,
})),

);
}),
});
8 changes: 4 additions & 4 deletions packages/job-dispatch/src/release-job-trigger.ts
Original file line number Diff line number Diff line change
@@ -114,17 +114,17 @@ class ReleaseJobTriggerBuilder {
}

async _values() {
const latestReleaseSubQuery = this._releaseSubQuery();
const latestActiveReleaseSubQuery = this._releaseSubQuery();
const releaseJobTriggers = this.releaseIds
? this._baseQuery().innerJoin(
release,
eq(release.deploymentId, deployment.id),
)
: this._baseQuery().innerJoin(
latestReleaseSubQuery,
latestActiveReleaseSubQuery,
and(
eq(latestReleaseSubQuery.deploymentId, deployment.id),
eq(latestReleaseSubQuery.rank, 1),
eq(latestActiveReleaseSubQuery.deploymentId, deployment.id),
eq(latestActiveReleaseSubQuery.rank, 1),
),
);


Unchanged files with check annotations Beta

EXPOSE 3000
ENV PORT=3000
ENV AUTH_TRUST_HOST=true

Check warning on line 60 in apps/webservice/Dockerfile

GitHub Actions / build (linux/amd64)

Sensitive data should not be used in the ARG or ENV commands

SecretsUsedInArgOrEnv: Do not use ARG or ENV instructions for sensitive data (ENV "AUTH_TRUST_HOST") More info: https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/
ENV NODE_ENV=production
ENV HOSTNAME=0.0.0.0