Skip to content
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
4 changes: 4 additions & 0 deletions static/app/components/workflowEngine/ui/actionMetadata.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ export const ActionMetadata: Partial<
name: t('Slack'),
icon: <StyledPluginIcon pluginId="slack" size={ICON_SIZE} />,
},
[ActionType.SLACK_STAGING]: {
name: t('Slack (Staging)'),
icon: <StyledPluginIcon pluginId="slack" size={ICON_SIZE} />,
},
[ActionType.WEBHOOK]: {
name: t('Webhook'),
icon: <StyledPluginIcon pluginId="webhooks" size={ICON_SIZE} />,
Expand Down
1 change: 1 addition & 0 deletions static/app/types/workflowEngine/actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export enum ActionTarget {

export enum ActionType {
SLACK = 'slack',
SLACK_STAGING = 'slack_staging',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The new SLACK_STAGING action is missing an entry in ActionMetadata, causing it to be omitted from UI summaries or displayed as "undefined" in automation names.
Severity: MEDIUM

Suggested Fix

Add an entry for ActionType.SLACK_STAGING to the ActionMetadata map in static/app/components/workflowEngine/ui/actionMetadata.tsx. This entry should define the name and icon for the new action type, ensuring it is displayed correctly throughout the UI.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/types/workflowEngine/actions.tsx#L38

Potential issue: The pull request introduces a new action type, `SLACK_STAGING`, but
fails to add a corresponding entry for it in the `ActionMetadata` map. This omission
leads to two distinct UI bugs. First, when an automation's actions are summarized,
`SLACK_STAGING` actions will be silently filtered out and not displayed because their
name resolves to `undefined`. Second, when generating an automation's name, if a
`targetDisplay` is not configured for the action, the name can render as "Notify via
undefined". This results in an incomplete and confusing user experience for automations
utilizing the new action type.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added

MSTEAMS = 'msteams',
DISCORD = 'discord',
PAGERDUTY = 'pagerduty',
Expand Down
9 changes: 9 additions & 0 deletions static/app/views/automations/components/actionNodes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,15 @@ export const actionNodesMap = new Map<ActionType, ActionNode>([
validate: validateSlackAction,
},
],
[
ActionType.SLACK_STAGING,
{
label: t('Slack (Staging)'),
action: SlackNode,
details: SlackDetails,
validate: validateSlackAction,
},
],
[
ActionType.WEBHOOK,
{
Expand Down
24 changes: 24 additions & 0 deletions static/app/views/automations/new.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ describe('AutomationNewSettings', () => {
handlerGroup: ActionGroup.OTHER,
integrations: undefined,
}),
ActionHandlerFixture({
type: ActionType.SLACK_STAGING,
handlerGroup: ActionGroup.NOTIFICATION,
integrations: [{id: 'slack-staging-1', name: 'My Slack Staging Workspace'}],
}),
],
});

Expand Down Expand Up @@ -332,6 +337,16 @@ describe('AutomationNewSettings', () => {
delay: null,
});

await addAction('Slack (Staging)');
{
const stagingTargets = screen.getAllByRole('textbox', {name: 'Target'});
const stagingTarget = stagingTargets.at(-1);
expect(stagingTarget).toBeDefined();
await userEvent.type(stagingTarget as HTMLElement, '#staging-alerts', {
delay: null,
});
}

await addAction('Discord');
await userEvent.type(screen.getByPlaceholderText('channel ID or URL'), '123', {
delay: null,
Expand Down Expand Up @@ -490,6 +505,15 @@ describe('AutomationNewSettings', () => {
targetDisplay: null,
},
},
slack_staging: {
type: 'slack_staging',
integrationId: 'slack-staging-1',
config: {
targetType: 'specific',
targetIdentifier: '',
targetDisplay: '#staging-alerts',
},
},
};

await waitFor(() => expect(saveWorkflow).toHaveBeenCalled());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {Detector} from 'sentry/types/workflowEngine/detectors';
const METRIC_DETECTOR_SUPPORTED_ACTIONS = new Set<ActionType>([
ActionType.EMAIL,
ActionType.SLACK,
ActionType.SLACK_STAGING,
ActionType.MSTEAMS,
ActionType.PAGERDUTY,
ActionType.OPSGENIE,
Expand Down
Loading