Skip to content

feat(crons): Disallow teams not part of project to be selected for notifications #69073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 18, 2024
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
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import {OrganizationFixture} from 'sentry-fixture/organization';
import {ProjectFixture} from 'sentry-fixture/project';
import {TeamFixture} from 'sentry-fixture/team';
import {UserFixture} from 'sentry-fixture/user';

import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
import {render, screen, userEvent, within} from 'sentry-test/reactTestingLibrary';
import selectEvent from 'sentry-test/selectEvent';

import MemberListStore from 'sentry/stores/memberListStore';
Expand Down Expand Up @@ -104,4 +105,36 @@ describe('SentryMemberTeamSelectorField', () => {
await userEvent.click(screen.getByLabelText('Clear choices'));
expect(mock).toHaveBeenCalledWith([], expect.anything());
});

it('disables teams not associated with project', async () => {
const project = ProjectFixture();
const teamWithProject = TeamFixture({projects: [project], slug: 'my-team'});
const teamWithoutProject = TeamFixture({id: '2', slug: 'disabled-team'});
TeamStore.init();
TeamStore.loadInitialData([teamWithProject, teamWithoutProject]);

const mock = jest.fn();
render(
<SentryMemberTeamSelectorField
label="Select Owner"
onChange={mock}
memberOfProjectSlug={project.slug}
name="team-or-member"
multiple
/>
);

await selectEvent.openMenu(screen.getByRole('textbox', {name: 'Select Owner'}));
expect(
within(
(await screen.findByText('My Teams')).parentElement as HTMLElement
).getByText('#my-team')
).toBeInTheDocument();

expect(
within(
(await screen.findByText('Disabled Teams')).parentElement as HTMLElement
).getByText('#disabled-team')
).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import {useContext, useEffect, useMemo} from 'react';
import partition from 'lodash/partition';
import groupBy from 'lodash/groupBy';

import Avatar from 'sentry/components/avatar';
import {Tooltip} from 'sentry/components/tooltip';
import {t} from 'sentry/locale';
import type {Team} from 'sentry/types';
import type {DetailedTeam, Team} from 'sentry/types';
import {useMembers} from 'sentry/utils/useMembers';
import {useTeams} from 'sentry/utils/useTeams';
import {useTeamsById} from 'sentry/utils/useTeamsById';
Expand All @@ -17,6 +18,10 @@ import SelectField from './selectField';
// projects can be passed as a direct prop as well
export interface RenderFieldProps extends SelectFieldProps<any> {
avatarSize?: number;
/**
* Ensures the only selectable teams and members are members of the given project
*/
memberOfProjectSlug?: string;
/**
* Use the slug as the select field value. Without setting this the numeric id
* of the project will be used.
Expand All @@ -27,6 +32,7 @@ export interface RenderFieldProps extends SelectFieldProps<any> {
function SentryMemberTeamSelectorField({
avatarSize = 20,
placeholder = t('Choose Teams and Members'),
memberOfProjectSlug,
...props
}: RenderFieldProps) {
const {form} = useContext(FormContext);
Expand Down Expand Up @@ -85,10 +91,33 @@ function SentryMemberTeamSelectorField({
leadingItems: <Avatar team={team} size={avatarSize} />,
});

const [myTeams, otherTeams] = partition(teams, team => team.isMember);
const makeDisabledTeamOption = (team: Team) => ({
...makeTeamOption(team),
disabled: true,
label: (
<Tooltip
position="left"
title={t('%s is not a member of the selected project', `#${team.slug}`)}
>
#{team.slug}
</Tooltip>
),
});

const myTeamOptions = myTeams.map(makeTeamOption);
const otherTeamOptions = otherTeams.map(makeTeamOption);
// TODO(davidenwang): Fix the team type here to avoid this type cast: `as DetailedTeam[]`
const {disabledTeams, memberTeams, otherTeams} = groupBy(
teams as DetailedTeam[],
team =>
memberOfProjectSlug && !team.projects.some(({slug}) => memberOfProjectSlug === slug)
? 'disabledTeams'
: team.isMember
? 'memberTeams'
: 'otherTeams'
);

const myTeamOptions = memberTeams?.map(makeTeamOption) ?? [];
const otherTeamOptions = otherTeams?.map(makeTeamOption) ?? [];
const disabledTeamOptions = disabledTeams?.map(makeDisabledTeamOption) ?? [];

// TODO(epurkhiser): This is an unfortunate hack right now since we don't
// actually load members anywhere and the useMembers and useTeams hook don't
Expand Down Expand Up @@ -128,6 +157,10 @@ function SentryMemberTeamSelectorField({
label: t('Other Teams'),
options: otherTeamOptions,
},
{
label: t('Disabled Teams'),
options: disabledTeamOptions,
},
]}
{...props}
/>
Expand Down
19 changes: 12 additions & 7 deletions static/app/views/monitors/components/monitorForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -504,13 +504,18 @@ function MonitorForm({
{t('Customize this monitors notification configuration in Alerts')}
</AlertLink>
)}
<SentryMemberTeamSelectorField
label={t('Notify')}
help={t('Send notifications to a member or team.')}
name="alertRule.targets"
multiple
menuPlacement="auto"
/>
<Observer>
{() => (
<SentryMemberTeamSelectorField
label={t('Notify')}
help={t('Send notifications to a member or team.')}
name="alertRule.targets"
memberOfProjectSlug={form.current.getValue('project')?.toString()}
multiple
menuPlacement="auto"
/>
)}
</Observer>
<Observer>
{() => {
const selectedAssignee = form.current.getValue('alertRule.targets');
Expand Down
Loading