Skip to content
Closed
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
13 changes: 13 additions & 0 deletions packages/junior-dashboard/e2e/automation-editor-fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,26 @@ export async function mockAutomationEditor(
{
namespace: "github",
type: "issue",
identifier: {
template: "{owner}/{repo}#{number}",
parts: {
owner: { kind: "string", description: "Repository owner" },
repo: { kind: "string", description: "Repository name" },
number: { kind: "number", description: "Issue number" },
},
},
supportedEvents: ["issue.closed", "issue.opened"],
matchFields: {
state: {
kind: "string",
description: "Issue state",
enum: ["open", "closed"],
},
closedBy: {
kind: "string",
description: "Login of who closed the issue",
events: ["issue.closed"],
},
},
},
],
Expand Down
23 changes: 21 additions & 2 deletions packages/junior-dashboard/e2e/automation-editor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,32 @@ test("keeps Event edits through validation, failed saves, and concurrent changes
return route.fallback();
});
await page.goto(`${dashboard.baseURL}/automations/event-1/edit`);
await expect(page.getByLabel("Resource identifier")).toHaveValue(
"getsentry/junior#42",
const identifier = page.getByRole("group", { name: "Resource identifier" });
await expect(identifier.getByLabel("owner", { exact: true })).toHaveValue(
"getsentry",
);
await expect(identifier.getByLabel("repo", { exact: true })).toHaveValue(
"junior",
);
const issueNumber = identifier.getByLabel("number", { exact: true });
await expect(issueNumber).toHaveValue("42");
await screenshot(page, "automation-event-editor");
// A wrong part names itself, and fixing it restores the exact identifier.
await issueNumber.fill("42a");
await expect(
identifier.getByText(
'"number" must be a positive whole number, such as 123.',
),
).toBeVisible();
await expect(issueNumber).toHaveAttribute("aria-invalid", "true");
await issueNumber.fill("42");
await expect(identifier.getByText("getsentry/junior#42")).toBeVisible();
// Only issue.closed sets closedBy, so the filter hides for issue.opened.
await expect(page.getByLabel("closedBy", { exact: true })).toBeVisible();
await page
.getByRole("checkbox", { name: "issue · opened", exact: true })
.check();
await expect(page.getByLabel("closedBy", { exact: true })).toHaveCount(0);
await page.getByLabel("state", { exact: true }).selectOption("closed");
await page.getByLabel("Instruction", { exact: true }).fill("");
await page.getByRole("button", { name: "Save changes" }).click();
Expand Down
2 changes: 1 addition & 1 deletion packages/junior-dashboard/e2e/automations.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,6 @@ test("pauses and resumes from details, keeps failures visible, and limits action
),
).toBeVisible();
await expect(
page.getByText("Enable its plugin or choose another event."),
page.getByText("Edit the trigger, or enable its plugin again."),
).toBeVisible();
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { AutomationLifecycle } from "./AutomationLifecycle";
import { AutomationRecentRuns } from "./AutomationRecentRuns";
import { AutomationStatus } from "./AutomationStatus";
import {
automationEventConditions,
automationEventFilters,
automationEventNames,
automationScheduleLabel,
formatAutomationRunTime,
Expand Down Expand Up @@ -98,8 +98,8 @@ function AutomationDetailsContent({
range: TimeRangeDays;
}) {
const location = useLocation();
const conditions =
automation.kind === "event" ? automationEventConditions(automation) : "";
const filters =
automation.kind === "event" ? automationEventFilters(automation) : "";
return (
<div className="grid min-w-0 gap-5">
<AutomationStatus automation={automation} />
Expand All @@ -119,9 +119,9 @@ function AutomationDetailsContent({
<p className="m-0 mt-1 text-dashboard-text-muted">
{automationEventNames(automation)}
</p>
{conditions ? (
{filters ? (
<p className="m-0 mt-1 break-words text-xs text-dashboard-text-muted">
{conditions}
{filters}
</p>
) : null}
</>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { useState } from "react";
import { useQuery } from "@tanstack/react-query";
import {
eventTriggerProblems,
eventsWithoutMatchField,
} from "@sentry/junior-plugin-api";
import {
automationEventCatalogSchema,
type AutomationEdit,
Expand All @@ -15,11 +19,17 @@ import {
} from "../../components/ui/alert-dialog";
import { Button } from "../../components/ui/button";
import { Input } from "../../components/ui/input";
import { Field, FieldDescription, FieldLabel } from "../../components/ui/field";
import {
Field,
FieldDescription,
FieldError,
FieldLabel,
} from "../../components/ui/field";
import { NativeSelect } from "../../components/ui/native-select";
import { FormNotice } from "../../components/FormNotice";
import { Checkbox } from "../../components/ui/checkbox";
import { fetchDashboardJson } from "../../http";
import { AutomationIdentifierFields } from "./AutomationIdentifierFields";

type Trigger = Extract<AutomationEdit, { kind: "event" }>["trigger"];

Expand All @@ -42,25 +52,25 @@ export function AutomationEventFields(props: {
),
retry: false,
});
const resource = catalog.data?.find(
(item) =>
item.namespace === props.value.namespace &&
item.type === props.value.resourceType,
const sourceResourceTypes =
catalog.data?.filter((item) => item.namespace === props.value.namespace) ??
[];
const resource = sourceResourceTypes.find(
(item) => item.type === props.value.resourceType,
);
const problems = catalog.data
? eventTriggerProblems(sourceResourceTypes, props.value)
: [];
// A filter that a selected event does not set is fixable here. Other
// problems mean the catalog changed after the trigger was saved.
const unsupported =
!resource ||
props.value.events.some(
(event) => !resource.supportedEvents.includes(event),
) ||
Object.entries(props.value.match ?? {}).some(([key, value]) => {
const field = resource.matchFields?.[key];
if (!field) return true;
return (Array.isArray(value) ? value : [value]).some(
(item) =>
typeof item !== field.kind ||
(field.enum && !field.enum.includes(String(item))),
);
});
!resource || problems.some((problem) => problem.reason !== "field_not_set");
const matchFields = Object.entries(resource?.matchFields ?? {});
const shownMatchFields = matchFields.filter(
([key, field]) =>
props.value.match?.[key] !== undefined ||
eventsWithoutMatchField(field, props.value.events).length === 0,
);
const setMatch = (
key: string,
value: NonNullable<Trigger["match"]>[string] | undefined,
Expand All @@ -84,6 +94,13 @@ export function AutomationEventFields(props: {
<FormNotice title="Saved trigger · Kept unchanged">
Some saved values are not in the current catalog. You can edit other
fields, or select a new resource type to replace the trigger.
{problems.length > 0 ? (
<ul className="my-2 list-disc pl-5">
{problems.map((problem) => (
<li key={problem.message}>{problem.message}</li>
))}
</ul>
) : null}
<pre className="whitespace-pre-wrap break-all text-xs">
{JSON.stringify(props.value, null, 2)}
</pre>
Expand Down Expand Up @@ -136,7 +153,7 @@ export function AutomationEventFields(props: {
<AlertDialogContent>
<AlertDialogTitle>Replace the saved trigger?</AlertDialogTitle>
<AlertDialogDescription>
Its resource, events, and conditions will be cleared.
Its resource, events, and filters will be cleared.
</AlertDialogDescription>
<AlertDialogFooter>
<AlertDialogCancel>Cancel</AlertDialogCancel>
Expand Down Expand Up @@ -165,22 +182,34 @@ export function AutomationEventFields(props: {
}
/>
</Field>
<Field>
<FieldLabel htmlFor="trigger-identifier">
Resource identifier
</FieldLabel>
<Input
size="comfortable"
id="trigger-identifier"
{resource.identifier ? (
<AutomationIdentifierFields
key={`${resource.namespace}:${resource.type}`}
format={resource.identifier}
value={props.value.identifier}
onChange={(e) =>
props.onChange({ ...props.value, identifier: e.target.value })
onChange={(identifier) =>
props.onChange({ ...props.value, identifier })
}
/>
<FieldDescription id="trigger-identifier-help">
Use the exact provider resource identifier, not its display name.
</FieldDescription>
</Field>
) : (
<Field>
<FieldLabel htmlFor="trigger-identifier">
Resource identifier
</FieldLabel>
<Input
size="comfortable"
id="trigger-identifier"
value={props.value.identifier}
onChange={(e) =>
props.onChange({ ...props.value, identifier: e.target.value })
}
/>
<FieldDescription id="trigger-identifier-help">
Use the exact provider resource identifier, not its display
name.
</FieldDescription>
</Field>
)}
<fieldset>
<legend className="mb-2 text-sm font-semibold">
Run on these events
Expand Down Expand Up @@ -216,74 +245,81 @@ export function AutomationEventFields(props: {
))}
</div>
</fieldset>
{Object.keys(resource.matchFields ?? {}).length ? (
{matchFields.length ? (
<fieldset className="grid gap-4">
<legend className="mb-3 text-sm font-semibold">Only when</legend>
<legend className="mb-3 text-sm font-semibold">Filters</legend>
<p className="m-0 text-xs leading-relaxed text-dashboard-text-muted">
All conditions must match. Leave a field blank to match any
value. Existing lists match any one listed value.
All filters must match. Leave a filter blank to match any value.
Existing lists match any one listed value.
{shownMatchFields.length < matchFields.length
? " Filters that a selected event does not set are hidden."
: null}
</p>
{Object.entries(resource.matchFields ?? {}).map(
([key, field]) => {
const value = props.value.match?.[key];
return (
<Field key={key}>
<FieldLabel htmlFor={`match-${key}`}>{key}</FieldLabel>
{Array.isArray(value) ? (
<div className="text-sm">
<span className="break-all">{value.join(", ")}</span>{" "}
<Button onClick={() => setMatch(key, undefined)}>
Remove condition
</Button>
</div>
) : field.kind === "boolean" || field.enum ? (
<NativeSelect
id={`match-${key}`}
value={value === undefined ? "" : String(value)}
onChange={(e) =>
setMatch(
key,
e.target.value === ""
? undefined
: field.kind === "boolean"
? e.target.value === "true"
: e.target.value,
)
}
>
<option value="">Any value</option>
{(field.enum ?? ["true", "false"]).map((v) => (
<option key={v}>{v}</option>
))}
</NativeSelect>
) : (
<Input
size="comfortable"
id={`match-${key}`}
type={field.kind === "number" ? "number" : "text"}
step="any"
value={value === undefined ? "" : String(value)}
onChange={(e) =>
setMatch(
key,
e.target.value === ""
? undefined
: field.kind === "number"
? e.target.valueAsNumber
: e.target.value,
)
}
/>
)}
{field.description ? (
<FieldDescription id={`match-${key}-help`}>
{field.description}
</FieldDescription>
) : null}
</Field>
);
},
)}
{shownMatchFields.map(([key, field]) => {
const value = props.value.match?.[key];
const problem = problems.find(
(candidate) => candidate.field === key,
);
return (
<Field key={key}>
<FieldLabel htmlFor={`match-${key}`}>{key}</FieldLabel>
{Array.isArray(value) ? (
<div className="text-sm">
<span className="break-all">{value.join(", ")}</span>{" "}
<Button onClick={() => setMatch(key, undefined)}>
Remove filter
</Button>
</div>
) : field.kind === "boolean" || field.enum ? (
<NativeSelect
id={`match-${key}`}
value={value === undefined ? "" : String(value)}
onChange={(e) =>
setMatch(
key,
e.target.value === ""
? undefined
: field.kind === "boolean"
? e.target.value === "true"
: e.target.value,
)
}
>
<option value="">Any value</option>
{(field.enum ?? ["true", "false"]).map((v) => (
<option key={v}>{v}</option>
))}
</NativeSelect>
) : (
<Input
size="comfortable"
id={`match-${key}`}
type={field.kind === "number" ? "number" : "text"}
step="any"
value={value === undefined ? "" : String(value)}
onChange={(e) =>
setMatch(
key,
e.target.value === ""
? undefined
: field.kind === "number"
? e.target.valueAsNumber
: e.target.value,
)
}
/>
)}
{field.description ? (
<FieldDescription id={`match-${key}-help`}>
{field.description}
</FieldDescription>
) : null}
{problem ? (
<FieldError>{problem.message}</FieldError>
) : null}
</Field>
);
})}
</fieldset>
) : null}
</>
Expand Down
Loading
Loading