Merge remote-tracking branch 'origin' into zacharyb/job-target-metadata

ctrlplanedev · Sep 30, 2024 · 33cf379 · 33cf379
2 parents 523e6ed + 8dceea0
commit 33cf379
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 68 deletions.
diff --git a/...webservice/src/app/[workspaceSlug]/_components/target-condition/TargetConditionDialog.tsx b/...webservice/src/app/[workspaceSlug]/_components/target-condition/TargetConditionDialog.tsx
@@ -1,5 +1,5 @@
 import type { TargetCondition } from "@ctrlplane/validators/targets";
-import React, { useState } from "react";
+import React, { useEffect, useState } from "react";
 
 import { Button } from "@ctrlplane/ui/button";
 import {
@@ -37,6 +37,14 @@ export const TargetConditionDialog: React.FC<TargetConditionDialogProps> = ({
     condition ?? defaultCondition,
   );
 
+  // there is some weirdness with the sidebar environment panel,
+  // where the condition doesn't update properly. this useEffect
+  // guarantees that the local condition is always in sync with the
+  // condition prop.
+  useEffect(() => {
+    setLocalCondition(condition ?? defaultCondition);
+  }, [condition]);
+
   return (
     <Dialog open={open} onOpenChange={setOpen}>
       <DialogTrigger asChild>{children}</DialogTrigger>

diff --git a/...ice/src/app/[workspaceSlug]/systems/[systemSlug]/environments/SidebarEnvironmentPanel.tsx b/...ice/src/app/[workspaceSlug]/systems/[systemSlug]/environments/SidebarEnvironmentPanel.tsx
@@ -1,5 +1,6 @@
 "use client";
 
+import type { TargetCondition } from "@ctrlplane/validators/targets";
 import { useEffect } from "react";
 import { useParams } from "next/navigation";
 import { IconInfoCircle, IconPlant } from "@tabler/icons-react";
@@ -16,13 +17,9 @@ import {
   useForm,
 } from "@ctrlplane/ui/form";
 import { Input } from "@ctrlplane/ui/input";
+import { Label } from "@ctrlplane/ui/label";
 import { Separator } from "@ctrlplane/ui/separator";
 import { Textarea } from "@ctrlplane/ui/textarea";
-import {
-  isDefaultCondition,
-  isValidTargetCondition,
-  targetCondition,
-} from "@ctrlplane/validators/targets";
 
 import { api } from "~/trpc/react";
 import { TargetConditionBadge } from "../../../_components/target-condition/TargetConditionBadge";
@@ -32,7 +29,6 @@ import { usePanel } from "./SidepanelContext";
 const environmentForm = z.object({
   name: z.string(),
   description: z.string().default(""),
-  targetFilter: targetCondition.optional(),
 });
 
 export const SidebarEnvironmentPanel: React.FC = () => {
@@ -49,7 +45,6 @@ export const SidebarEnvironmentPanel: React.FC = () => {
     defaultValues: {
       name: node.data.label,
       description: node.data.description,
-      targetFilter: node.data.targetFilter,
     },
   });
 
@@ -62,18 +57,15 @@ export const SidebarEnvironmentPanel: React.FC = () => {
     form.reset({
       name: node.data.label,
       description: node.data.description,
-      targetFilter: node.data.targetFilter,
     });
   }, [node, form]);
 
-  const { targetFilter } = form.watch();
-
   const targets = api.target.byWorkspaceId.list.useQuery(
     {
       workspaceId: workspace.data?.id ?? "",
-      filter: targetFilter,
+      filter: node.data.targetFilter,
     },
-    { enabled: workspace.data != null && targetFilter != null },
+    { enabled: workspace.data != null && node.data.targetFilter != null },
   );
 
   const utils = api.useUtils();
@@ -83,25 +75,39 @@ export const SidebarEnvironmentPanel: React.FC = () => {
       const node = nodes.find((n) => n.id === selectedNodeId);
       if (!node) return nodes;
 
-      const { targetFilter } = values;
+      update
+        .mutateAsync({
+          id: node.id,
+          data: { ...values },
+        })
+        .then(() =>
+          utils.environment.bySystemId.invalidate(node.data.systemId),
+        );
+      return nodes.map((n) =>
+        n.id === selectedNodeId
+          ? {
+              ...n,
+              data: {
+                ...n.data,
+                ...values,
+                label: values.name,
+              },
+            }
+          : n,
+      );
+    });
+  });
 
-      if (targetFilter != null && !isValidTargetCondition(targetFilter)) {
-        form.setError("targetFilter", {
-          message:
-            "Invalid target filter, ensure all fields are filled out correctly.",
-        });
-        return nodes;
-      }
+  const onFilterDialogSubmit = (condition: TargetCondition | undefined) =>
+    setNodes((nodes) => {
+      const node = nodes.find((n) => n.id === selectedNodeId);
+      if (!node) return nodes;
 
       update
         .mutateAsync({
           id: node.id,
           data: {
-            ...values,
-            targetFilter:
-              targetFilter != null && isDefaultCondition(targetFilter)
-                ? undefined
-                : targetFilter,
+            targetFilter: condition ?? null,
           },
         })
         .then(() =>
@@ -113,15 +119,12 @@ export const SidebarEnvironmentPanel: React.FC = () => {
               ...n,
               data: {
                 ...n.data,
-                ...values,
-                targetFilter,
-                label: values.name,
+                targetFilter: condition,
               },
             }
           : n,
       );
     });
-  });
 
   return (
     <Form {...form}>
@@ -165,43 +168,31 @@ export const SidebarEnvironmentPanel: React.FC = () => {
           )}
         />
 
-        <FormField
-          control={form.control}
-          name="targetFilter"
-          render={({ field: { onChange, value } }) => (
-            <FormItem>
-              <FormControl>
-                <div className="flex flex-col gap-2">
-                  <FormLabel>
-                    Target Filter (
-                    {targetFilter != null && targets.data != null
-                      ? targets.data.total
-                      : "-"}
-                    )
-                  </FormLabel>
-                  {value == null && (
-                    <span className="text-sm text-muted-foreground">
-                      Add a filter to select targets for this environment.
-                    </span>
-                  )}
-                  {value != null && (
-                    <TargetConditionBadge condition={value} tabbed />
-                  )}
-                  <TargetConditionDialog condition={value} onChange={onChange}>
-                    <Button variant="outline" className="w-fit">
-                      Set targets
-                    </Button>
-                  </TargetConditionDialog>
-                  {form.formState.errors.targetFilter && (
-                    <span className="text-sm text-red-600">
-                      {form.formState.errors.targetFilter.message}
-                    </span>
-                  )}
-                </div>
-              </FormControl>
-            </FormItem>
+        <div className="flex flex-col gap-2">
+          <Label>
+            Target Filter (
+            {node.data.targetFilter != null && targets.data != null
+              ? targets.data.total
+              : "-"}
+            )
+          </Label>
+          {node.data.targetFilter == null && (
+            <span className="text-sm text-muted-foreground">
+              Add a filter to select targets for this environment.
+            </span>
           )}
-        />
+          {node.data.targetFilter != null && (
+            <TargetConditionBadge condition={node.data.targetFilter} tabbed />
+          )}
+          <TargetConditionDialog
+            condition={node.data.targetFilter}
+            onChange={onFilterDialogSubmit}
+          >
+            <Button variant="outline" className="w-fit">
+              Set targets
+            </Button>
+          </TargetConditionDialog>
+        </div>
 
         <div className="flex gap-2">
           <Button type="submit" disabled={update.isPending}>

diff --git a/apps/webservice/src/app/api/github/webhook/workflow/handler.ts b/apps/webservice/src/app/api/github/webhook/workflow/handler.ts
@@ -1,6 +1,6 @@
 import type { WorkflowRunEvent } from "@octokit/webhooks-types";
 
-import { eq, takeFirst } from "@ctrlplane/db";
+import { eq, takeFirstOrNull } from "@ctrlplane/db";
 import { db } from "@ctrlplane/db/client";
 import * as schema from "@ctrlplane/db/schema";
 import { onJobCompletion } from "@ctrlplane/job-dispatch";
@@ -34,7 +34,13 @@ export const handleWorkflowWebhookEvent = async (event: WorkflowRunEvent) => {
     .set({ status })
     .where(eq(schema.job.externalRunId, id.toString()))
     .returning()
-    .then(takeFirst);
+    .then(takeFirstOrNull);
+
+  // Addressing a race condition: When the job is created externally on GitHub,
+  // it triggers a webhook event. However, our system hasn't updated the job with
+  // the externalRunId yet, as it depends on the job's instantiation. Therefore,
+  // the first event lacks the run ID, so we skip it and wait for the next event.
+  if (job == null) return;
 
   if (job.status === JobStatus.Completed) return onJobCompletion(job);
 };