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
31 changes: 18 additions & 13 deletions static/app/views/detectors/components/details/common/actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
makeMonitorBasePathname,
makeMonitorDetailsPathname,
} from 'sentry/views/detectors/pathnames';
import {detectorTypeIsUserCreateable} from 'sentry/views/detectors/utils/detectorTypeConfig';
import {useCanEditDetector} from 'sentry/views/detectors/utils/useCanEditDetector';

export function DisableDetectorAction({detector}: {detector: Detector}) {
Expand Down Expand Up @@ -59,19 +60,23 @@ export function EditDetectorAction({detector}: {detector: Detector}) {
projectId: detector.projectId,
});

const permissionTooltipText = tct(
'You do not have permission to edit this monitor. Ask your organization owner or manager to [settingsLink:enable monitor access] for you.',
{
settingsLink: (
<Link
to={{
pathname: `/settings/${organization.slug}/`,
hash: 'alertsMemberWrite',
}}
/>
),
}
);
const permissionTooltipText = detectorTypeIsUserCreateable(detector.type)
Copy link
Member

Choose a reason for hiding this comment

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

Won't this only show if you're unable to edit the monitor?

Copy link
Member

Choose a reason for hiding this comment

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

oh canEdit inherited behavior huh

Copy link
Member

Choose a reason for hiding this comment

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

nvm I see I think

? tct(
'You do not have permission to edit this monitor. Ask your organization owner or manager to [settingsLink:enable monitor access] for you.',
{
settingsLink: (
<Link
to={{
pathname: `/settings/${organization.slug}/`,
hash: 'alertsMemberWrite',
}}
/>
),
}
)
: t(
'This monitor is managed by Sentry. Only organization owners and managers can edit it.'
Copy link
Member

Choose a reason for hiding this comment

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

wait, is this right?

Copy link
Member Author

Choose a reason for hiding this comment

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

For now yeah, only org owners/managers can access the edit screen for error monitors

);

return (
<LinkButton
Expand Down
2 changes: 1 addition & 1 deletion static/app/views/detectors/list/allMonitors.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export default function AllMonitors() {
return (
<SentryDocumentTitle title={TITLE}>
<WorkflowEngineListLayout
actions={<DetectorListActions />}
actions={<DetectorListActions detectorType={null} />}
title={TITLE}
description={DESCRIPTION}
docsUrl={DOCS_URL}
Expand Down
50 changes: 45 additions & 5 deletions static/app/views/detectors/list/common/detectorListActions.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,50 @@
import {Link} from '@sentry/scraps/link';

import {LinkButton} from 'sentry/components/core/button/linkButton';
import {Flex} from 'sentry/components/core/layout';
import {ALL_ACCESS_PROJECTS} from 'sentry/constants/pageFilters';
import {IconAdd} from 'sentry/icons';
import {t} from 'sentry/locale';
import {t, tct} from 'sentry/locale';
import type {Organization} from 'sentry/types/organization';
import type {DetectorType} from 'sentry/types/workflowEngine/detectors';
import useOrganization from 'sentry/utils/useOrganization';
import usePageFilters from 'sentry/utils/usePageFilters';
import {MonitorFeedbackButton} from 'sentry/views/detectors/components/monitorFeedbackButton';
import {makeMonitorCreatePathname} from 'sentry/views/detectors/pathnames';
import {detectorTypeIsUserCreateable} from 'sentry/views/detectors/utils/detectorTypeConfig';
import {useCanCreateDetector} from 'sentry/views/detectors/utils/useCanCreateDetector';

interface DetectorListActionsProps {
detectorType: DetectorType | null;
children?: React.ReactNode;
/**
* Pass a detector type to skip type selection on the create monitor page
*/
detectorType?: DetectorType;
}

function getPermissionTooltipText({
organization,
detectorType,
}: {
detectorType: DetectorType | null;
organization: Organization;
}) {
const noPermissionText = tct(
'You do not have permission to create monitors. Ask your organization owner or manager to [settingsLink:enable monitor access] for you.',
{
settingsLink: (
<Link
to={{
pathname: `/settings/${organization.slug}/`,
hash: 'alertsMemberWrite',
}}
/>
),
}
);

if (!detectorType || detectorTypeIsUserCreateable(detectorType)) {
return noPermissionText;
}

return t('This monitor type is managed by Sentry.');
}

export function DetectorListActions({detectorType, children}: DetectorListActionsProps) {
Expand All @@ -24,6 +54,7 @@ export function DetectorListActions({detectorType, children}: DetectorListAction
const createPath = makeMonitorCreatePathname(organization.slug);
const project = selection.projects.find(pid => pid !== ALL_ACCESS_PROJECTS);
const createQuery = detectorType ? {project, detectorType} : {project};
const canCreateDetector = useCanCreateDetector(detectorType);

return (
<Flex gap="sm">
Expand All @@ -37,6 +68,15 @@ export function DetectorListActions({detectorType, children}: DetectorListAction
priority="primary"
icon={<IconAdd />}
size="sm"
disabled={!canCreateDetector}
title={
canCreateDetector
? undefined
: getPermissionTooltipText({
organization,
detectorType,
})
}
>
{t('Create Monitor')}
</LinkButton>
Expand Down
2 changes: 1 addition & 1 deletion static/app/views/detectors/list/myMonitors.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export default function MyMonitorsList() {
return (
<SentryDocumentTitle title={TITLE}>
<WorkflowEngineListLayout
actions={<DetectorListActions />}
actions={<DetectorListActions detectorType={null} />}
title={TITLE}
description={DESCRIPTION}
docsUrl={DOCS_URL}
Expand Down
15 changes: 15 additions & 0 deletions static/app/views/detectors/utils/useCanCreateDetector.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import {hasEveryAccess} from 'sentry/components/acl/access';
import type {DetectorType} from 'sentry/types/workflowEngine/detectors';
import useOrganization from 'sentry/utils/useOrganization';

import {detectorTypeIsUserCreateable} from './detectorTypeConfig';

export function useCanCreateDetector(detectorType: DetectorType | null) {
const organization = useOrganization();

if (!detectorType) {
return hasEveryAccess(['alerts:write'], {organization});
}

return detectorTypeIsUserCreateable(detectorType);
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Inconsistent authorization allows monitor creation.

When detectorType is provided and user-createable, the hook returns true without checking the user's alerts:write permission. This allows users without proper permissions to see an enabled "Create Monitor" button for user-createable detector types. The permission check is only performed when detectorType is null, creating an inconsistent authorization check.

Fix in Cursor Fix in Web

}
Loading