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
34 changes: 31 additions & 3 deletions static/app/actionCreators/uptime.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,27 @@ import {
} from 'sentry/actionCreators/indicator';
import type {Client} from 'sentry/api';
import {t} from 'sentry/locale';
import type {Organization} from 'sentry/types/organization';
import type RequestError from 'sentry/utils/requestError/requestError';
import type {UptimeRule} from 'sentry/views/alerts/rules/uptime/types';

export async function updateUptimeRule(
api: Client,
orgId: string,
org: Organization,
uptimeRule: UptimeRule,
data: Partial<UptimeRule>
): Promise<UptimeRule | null> {
addLoadingMessage();

try {
const resp = await api.requestPromise(
`/projects/${orgId}/${uptimeRule.projectSlug}/uptime/${uptimeRule.id}/`,
{method: 'PUT', data}
`/projects/${org.slug}/${uptimeRule.projectSlug}/uptime/${uptimeRule.detectorId}/`,
{
method: 'PUT',
data,
// TODO(epurkhiser): Can be removed once these APIs only take detectors
query: {useDetectorId: 1},
}
);
clearIndicators();
return resp;
Expand All @@ -42,3 +48,25 @@ export async function updateUptimeRule(

return null;
}

export async function deleteUptimeRule(
api: Client,
org: Organization,
uptimeRule: UptimeRule
) {
addLoadingMessage('Deleting uptime alert rule...');

try {
await api.requestPromise(
`/projects/${org.slug}/${uptimeRule.projectSlug}/uptime/${uptimeRule.detectorId}/`,
{
method: 'DELETE',
// TODO(epurkhiser): Can be removed once these APIs only take detectors
query: {useDetectorId: 1},
}
);
clearIndicators();
} catch (_err) {
addErrorMessage(t('Error deleting rule'));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,9 @@ describe('Uptime Data Section', () => {
});

const event = EventFixture({
tags: [
{
key: 'uptime_rule',
value: '1234',
},
],
occurrence: {
evidenceData: {detectorId: 1234},
},
});

render(<UptimeDataSection event={event} group={group} project={project} />);
Expand Down Expand Up @@ -94,12 +91,9 @@ describe('Uptime Data Section', () => {
});

const event = EventFixture({
tags: [
{
key: 'uptime_rule',
value: '1234',
},
],
occurrence: {
evidenceData: {detectorId: 1234},
},
});

render(<UptimeDataSection event={event} group={group} project={project} />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export function UptimeDataSection({group, event, project}: Props) {
const now = useMemo(() => new Date(), []);

const isResolved = group.status === GroupStatus.RESOLVED;
const alertRuleId = event.tags.find(tag => tag.key === 'uptime_rule')?.value;
const detectorId: number | undefined = event.occurrence?.evidenceData.detectorId;

const elementRef = useRef<HTMLDivElement>(null);
const {width: containerWidth} = useDimensions<HTMLDivElement>({elementRef});
Expand All @@ -122,19 +122,19 @@ export function UptimeDataSection({group, event, project}: Props) {
}, [timeWindow, timelineWidth, since, until, event, now]);

const {data: uptimeStats, isPending} = useUptimeMonitorStats({
ruleIds: alertRuleId ? [alertRuleId] : [],
detectorIds: detectorId ? [String(detectorId)] : [],
timeWindowConfig,
});
const bucketedData = alertRuleId ? (uptimeStats?.[alertRuleId] ?? []) : [];
const bucketedData = detectorId ? (uptimeStats?.[detectorId] ?? []) : [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Mismatched Data Types in API Response

The detectorId is a number, but the uptimeStats object is keyed by strings because the API request uses string IDs. Accessing uptimeStats with the numeric detectorId will always fail, causing the uptime data to appear empty.

Fix in Cursor Fix in Web

Copy link
Member Author

Choose a reason for hiding this comment

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

this should probably always be a number


const actions = (
<ButtonBar>
{defined(alertRuleId) && (
{defined(detectorId) && (
<LinkButton
icon={<IconSettings />}
size="xs"
to={makeAlertsPathname({
path: `/rules/uptime/${project.slug}/${alertRuleId}/details/`,
path: `/rules/uptime/${project.slug}/${detectorId}/details/`,
organization,
})}
>
Expand Down
2 changes: 1 addition & 1 deletion static/app/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1545,7 +1545,7 @@ function buildRoutes(): RouteObject[] {
deprecatedRouteProps: true,
children: [
{
path: ':projectId/:uptimeRuleId/details/',
path: ':projectId/:detectorId/details/',
component: make(() => import('sentry/views/alerts/rules/uptime/details')),
deprecatedRouteProps: true,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ describe('AlertRulesList', () => {
renderGlobalModal();

const deleteMock = MockApiClient.addMockResponse({
url: `/projects/${organization.slug}/${project.slug}/uptime/${uptimeRule.id}/`,
url: `/projects/${organization.slug}/${project.slug}/uptime/${uptimeRule.detectorId}/?useDetectorId=1`,
method: 'DELETE',
body: {},
});
Expand Down
20 changes: 14 additions & 6 deletions static/app/views/alerts/list/rules/alertRulesList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -149,19 +149,27 @@ function AlertRulesList() {
};

const handleDeleteRule = async (projectId: string, rule: CombinedAlerts) => {
// TODO(epurkhiser): To be removed when uptime rules use detector ID as the `id`
const id = rule.type === CombinedAlertType.UPTIME ? rule.detectorId : rule.id;

const deleteEndpoints: Record<CombinedAlertType, string> = {
[CombinedAlertType.ISSUE]: `/projects/${organization.slug}/${projectId}/rules/${rule.id}/`,
[CombinedAlertType.METRIC]: `/organizations/${organization.slug}/alert-rules/${rule.id}/`,
[CombinedAlertType.UPTIME]: `/projects/${organization.slug}/${projectId}/uptime/${rule.id}/`,
[CombinedAlertType.CRONS]: `/projects/${organization.slug}/${projectId}/monitors/${rule.id}/`,
[CombinedAlertType.ISSUE]: `/projects/${organization.slug}/${projectId}/rules/${id}/`,
[CombinedAlertType.METRIC]: `/organizations/${organization.slug}/alert-rules/${id}/`,
[CombinedAlertType.UPTIME]: `/projects/${organization.slug}/${projectId}/uptime/${id}/?useDetectorId=1`,
[CombinedAlertType.CRONS]: `/projects/${organization.slug}/${projectId}/monitors/${id}/`,
};

try {
await api.requestPromise(deleteEndpoints[rule.type], {method: 'DELETE'});
setApiQueryData<Array<CombinedAlerts | null>>(
queryClient,
getAlertListQueryKey(organization.slug, location.query),
data => data?.filter(r => r?.id !== rule.id && r?.type !== rule.type)
data =>
data?.filter(r => {
// TODO(epurkhiser): To be removed when uptime rules use detector ID as the `id`
const rId = r?.type === CombinedAlertType.UPTIME ? r.detectorId : r?.id;
return rId !== id && r?.type !== rule.type;
})
);
refetch();
addSuccessMessage(t('Deleted rule'));
Expand Down Expand Up @@ -283,7 +291,7 @@ function AlertRulesList() {
return (
<RuleListRow
// Metric and issue alerts can have the same id
key={`${keyPrefix}-${rule.id}`}
key={`${keyPrefix}-${rule.type === CombinedAlertType.UPTIME ? rule.detectorId : rule.id}`}
projectsLoaded={initiallyLoaded}
projects={projects as Project[]}
rule={rule}
Expand Down
9 changes: 6 additions & 3 deletions static/app/views/alerts/list/rules/row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,11 @@ function RuleListRow({
[CombinedAlertType.CRONS]: 'crons-rules',
} satisfies Record<CombinedAlertType, string>;

// TODO(epurkhiser): To be removed when uptime rules use detector ID as the `id`
const ruleId = rule.type === CombinedAlertType.UPTIME ? rule.detectorId : rule.id;

const editLink = makeAlertsPathname({
path: `/${editKey[rule.type]}/${slug}/${rule.id}/`,
path: `/${editKey[rule.type]}/${slug}/${ruleId}/`,
organization,
});

Expand All @@ -111,7 +114,7 @@ function RuleListRow({
}),
query: {
project: slug,
duplicateRuleId: rule.id,
duplicateRuleId: ruleId,
createFromDuplicate: 'true',
referrer: 'alert_stream',
},
Expand Down Expand Up @@ -260,7 +263,7 @@ function RuleListRow({
});
case CombinedAlertType.UPTIME:
return makeAlertsPathname({
path: `/rules/uptime/${rule.projectSlug}/${rule.id}/details/`,
path: `/rules/uptime/${rule.projectSlug}/${rule.detectorId}/details/`,
organization,
});
default:
Expand Down
15 changes: 11 additions & 4 deletions static/app/views/alerts/rules/uptime/details.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ describe('UptimeAlertDetails', () => {
body: [],
});
MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/issues/?limit=1&project=${project.id}&query=issue.type%3Auptime_domain_failure%20tags%5Buptime_rule%5D%3A1`,
url: `/organizations/org-slug/issues/?limit=1&project=2&query=issue.type%3Auptime_domain_failure%20title%3A%22Downtime%20detected%20for%20https%3A%2F%2Fsentry.io%2F%22`,
body: [],
});
MockApiClient.addMockResponse({
url: `/projects/${organization.slug}/${project.slug}/uptime/1/checks/`,
query: {useDetectorId: '1'},
body: [],
});
MockApiClient.addMockResponse({
Expand All @@ -37,13 +38,14 @@ describe('UptimeAlertDetails', () => {
it('renders', async () => {
MockApiClient.addMockResponse({
url: `/projects/${organization.slug}/${project.slug}/uptime/1/`,
query: {useDetectorId: '1'},
body: UptimeRuleFixture({name: 'Uptime Test Rule'}),
});

render(
<UptimeAlertDetails
{...routerProps}
params={{...routerProps.params, uptimeRuleId: '1'}}
params={{...routerProps.params, detectorId: '1'}}
/>,
{organization}
);
Expand All @@ -53,13 +55,14 @@ describe('UptimeAlertDetails', () => {
it('shows a message for invalid uptime alert', async () => {
MockApiClient.addMockResponse({
url: `/projects/${organization.slug}/${project.slug}/uptime/2/`,
query: {useDetectorId: '1'},
statusCode: 404,
});

render(
<UptimeAlertDetails
{...routerProps}
params={{...routerProps.params, uptimeRuleId: '2'}}
params={{...routerProps.params, detectorId: '2'}}
/>,
{organization}
);
Expand All @@ -71,24 +74,27 @@ describe('UptimeAlertDetails', () => {
it('disables and enables the rule', async () => {
MockApiClient.addMockResponse({
url: `/projects/${organization.slug}/${project.slug}/uptime/2/`,
query: {useDetectorId: '1'},
statusCode: 404,
});
MockApiClient.addMockResponse({
url: `/projects/${organization.slug}/${project.slug}/uptime/1/`,
query: {useDetectorId: '1'},
body: UptimeRuleFixture({name: 'Uptime Test Rule'}),
});

render(
<UptimeAlertDetails
{...routerProps}
params={{...routerProps.params, uptimeRuleId: '1'}}
params={{...routerProps.params, detectorId: '1'}}
/>,
{organization}
);
await screen.findByText('Uptime Test Rule');

const disableMock = MockApiClient.addMockResponse({
url: `/projects/${organization.slug}/${project.slug}/uptime/1/`,
query: {useDetectorId: '1'},
method: 'PUT',
body: UptimeRuleFixture({name: 'Uptime Test Rule', status: 'disabled'}),
});
Expand All @@ -105,6 +111,7 @@ describe('UptimeAlertDetails', () => {

const enableMock = MockApiClient.addMockResponse({
url: `/projects/${organization.slug}/${project.slug}/uptime/1/`,
query: {useDetectorId: '1'},
method: 'PUT',
body: UptimeRuleFixture({name: 'Uptime Test Rule', status: 'active'}),
});
Expand Down
16 changes: 8 additions & 8 deletions static/app/views/alerts/rules/uptime/details.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ import {UptimeChecksTable} from './uptimeChecksTable';
import {UptimeIssues} from './uptimeIssues';

interface UptimeAlertDetailsProps
extends RouteComponentProps<{projectId: string; uptimeRuleId: string}> {}
extends RouteComponentProps<{detectorId: string; projectId: string}> {}

export default function UptimeAlertDetails({params}: UptimeAlertDetailsProps) {
const api = useApi();
const organization = useOrganization();
const queryClient = useQueryClient();

const {projectId, uptimeRuleId} = params;
const {projectId, detectorId} = params;

const {projects, fetching: loadingProject} = useProjects({slugs: [projectId]});
const project = projects.find(({slug}) => slug === projectId);
Expand All @@ -54,10 +54,10 @@ export default function UptimeAlertDetails({params}: UptimeAlertDetailsProps) {
data: uptimeRule,
isPending,
isError,
} = useUptimeRule({projectSlug: projectId, uptimeRuleId});
} = useUptimeRule({projectSlug: projectId, detectorId});

const {data: uptimeSummaries} = useUptimeMonitorSummaries({ruleIds: [uptimeRuleId]});
const summary = uptimeSummaries?.[uptimeRuleId];
const {data: uptimeSummaries} = useUptimeMonitorSummaries({detectorIds: [detectorId]});
const summary = uptimeSummaries?.[detectorId];

// Only display the missed window legend when there are visible missed window
// check-ins in the timeline
Expand Down Expand Up @@ -95,7 +95,7 @@ export default function UptimeAlertDetails({params}: UptimeAlertDetailsProps) {
}

const handleUpdate = async (data: Partial<UptimeRule>) => {
const resp = await updateUptimeRule(api, organization.slug, uptimeRule, data);
const resp = await updateUptimeRule(api, organization, uptimeRule, data);

if (resp !== null) {
setUptimeRuleData({
Expand Down Expand Up @@ -157,7 +157,7 @@ export default function UptimeAlertDetails({params}: UptimeAlertDetailsProps) {
disabled={!canEdit}
title={canEdit ? undefined : permissionTooltipText}
to={makeAlertsPathname({
path: `/uptime-rules/${project.slug}/${uptimeRuleId}/`,
path: `/uptime-rules/${project.slug}/${detectorId}/`,
organization,
})}
>
Expand Down Expand Up @@ -190,7 +190,7 @@ export default function UptimeAlertDetails({params}: UptimeAlertDetailsProps) {
</Alert.Container>
)}
<DetailsTimeline uptimeRule={uptimeRule} onStatsLoaded={checkHasUnknown} />
<UptimeIssues project={project} ruleId={uptimeRuleId} />
<UptimeIssues project={project} uptimeRule={uptimeRule} />
<UptimeChecksTable uptimeRule={uptimeRule} />
</Layout.Main>
<Layout.Side>
Expand Down
7 changes: 4 additions & 3 deletions static/app/views/alerts/rules/uptime/detailsTimeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,21 @@ interface Props {
}

export function DetailsTimeline({uptimeRule, onStatsLoaded}: Props) {
const {detectorId} = uptimeRule;
const elementRef = useRef<HTMLDivElement>(null);
const {width: containerWidth} = useDimensions<HTMLDivElement>({elementRef});
const timelineWidth = useDebouncedValue(containerWidth, 500);

const timeWindowConfig = useTimeWindowConfig({timelineWidth});

const {data: uptimeStats} = useUptimeMonitorStats({
ruleIds: [uptimeRule.id],
detectorIds: [uptimeRule.detectorId],
timeWindowConfig,
});

useEffect(
() => uptimeStats?.[uptimeRule.id] && onStatsLoaded?.(uptimeStats[uptimeRule.id]!),
[onStatsLoaded, uptimeStats, uptimeRule.id]
() => uptimeStats?.[detectorId] && onStatsLoaded?.(uptimeStats[detectorId]),
[onStatsLoaded, uptimeStats, detectorId]
);

return (
Expand Down
Loading
Loading