Skip to content

ref(hybrid-cloud): Restrict notification settings UX to a single organization #50279

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 42 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
d54060e
feat(hc): make NotificationSetting.target_id nullable
vbro May 17, 2023
f02200a
fix typing
vbro May 17, 2023
b9480e6
set fetch_actor=False in test_user_notification_settings
vbro May 17, 2023
a6f69ff
rebase and update migration
vbro May 18, 2023
0a0d0f0
Refactor the notification settings page to restrict viewing notificat…
mikejihbe Jun 4, 2023
c7245ed
Fix up some FE tests
mikejihbe Jun 6, 2023
416ca87
Merge branch 'master' into ihbe/notification-settings-ux
mikejihbe Jun 16, 2023
02ddcaa
Fix up more notification fine tune settings UI
mikejihbe Jun 16, 2023
5144248
Merge branch 'master' into ihbe/notification-settings-ux
mikejihbe Jun 22, 2023
2bf272b
Fix up functionality. Styles are still a little borked
mikejihbe Jun 23, 2023
5502823
Fix some bugs
mikejihbe Jun 23, 2023
dfd80a7
Fix styles
mikejihbe Jun 23, 2023
1f625f0
cleanup
mikejihbe Jun 25, 2023
f3c92ea
Merge branch 'master' into ihbe/notification-settings-ux
mikejihbe Jun 27, 2023
1009f4b
Merge branch 'master' into alter_notification_actor_nullable
mikejihbe Jun 27, 2023
ea0ad80
Add filter query implementation
mikejihbe Jun 28, 2023
accf98d
don't hash rpc notifications
mikejihbe Jun 28, 2023
35653e2
Undo stable tests. This needs my other notification PR for that
mikejihbe Jun 28, 2023
f24e27d
Merge branch 'master' into alter_notification_actor_nullable
mikejihbe Jun 28, 2023
20b8208
Add argument assertion
mikejihbe Jun 28, 2023
f98fe25
Factor actor id out of all notification updates
mikejihbe Jul 3, 2023
06966f8
Merge branch 'master' into alter_notification_actor_nullable
mikejihbe Jul 3, 2023
aadddb9
Fix up some more callsites in tests
mikejihbe Jul 3, 2023
c2b9402
Fix what I hope is the last bug here
mikejihbe Jul 3, 2023
37c79e1
:knife: regenerate mypy module blocklist
getsantry[bot] Jul 3, 2023
e96b605
More test fixes
mikejihbe Jul 3, 2023
4227037
Fix final test
mikejihbe Jul 3, 2023
8476518
Merge branch 'alter_notification_actor_nullable' into ihbe/notificati…
mikejihbe Jul 3, 2023
ecbffa1
Merge branch 'master' into ihbe/notification-settings-ux
mikejihbe Jul 3, 2023
5e79b80
Fix types. And more tests
mikejihbe Jul 3, 2023
f973268
Merge branch 'master' into ihbe/notification-settings-ux
mikejihbe Jul 5, 2023
c973c3b
Fix team tests
mikejihbe Jul 5, 2023
ad82f05
Feedback
corps Jul 11, 2023
bca8f6c
Duplicate out new serializer backend
corps Jul 11, 2023
7098000
Merge branch 'master' into ihbe/notification-settings-ux
corps Jul 11, 2023
236e000
Tiny fixes
corps Jul 11, 2023
3f22062
Fixes
corps Jul 12, 2023
ba88e43
More matcher fixes
corps Jul 12, 2023
1aba6cc
Moo
corps Jul 12, 2023
4de4c2a
Update static/app/views/settings/account/notifications/notificationSe…
corps Jul 13, 2023
d60066b
Merge branch 'master' into ihbe/notification-settings-ux
corps Jul 13, 2023
6cea66c
Merge branch 'master' into ihbe/notification-settings-ux
corps Jul 15, 2023
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 src/sentry/api/endpoints/project_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ def get(self, request: Request) -> Response:
else:
queryset = queryset.filter(teams__organizationmember__user_id=request.user.id)

org_id_filter = request.GET.get("organization_id", None)
if org_id_filter:
queryset = queryset.filter(organization_id=org_id_filter)

query = request.GET.get("query")
if query:
tokens = tokenize_query(query)
Expand Down
53 changes: 28 additions & 25 deletions src/sentry/api/serializers/models/notification_setting.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
from sentry.models.notificationsetting import NotificationSetting
from sentry.models.team import Team
from sentry.models.user import User
from sentry.notifications.helpers import get_fallback_settings
from sentry.notifications.types import VALID_VALUES_FOR_KEY, NotificationSettingTypes
from sentry.services.hybrid_cloud.actor import RpcActor

# from sentry.notifications.helpers import get_fallback_settings
from sentry.notifications.types import NotificationSettingTypes # VALID_VALUES_FOR_KEY

# from sentry.services.hybrid_cloud.actor import RpcActor
from sentry.services.hybrid_cloud.organization import RpcTeam
from sentry.services.hybrid_cloud.user import RpcUser

Expand Down Expand Up @@ -65,15 +67,15 @@ def get_attrs(
f"NotificationSetting {notifications_setting.id} has neither team_id nor user_id"
)

for recipient in item_list:
# This works because both User and Team models implement `get_projects`.
results[recipient]["projects"] = recipient.get_projects()
# for recipient in item_list:
# # This works because both User and Team models implement `get_projects`.
# results[recipient]["projects"] = recipient.get_projects()

if isinstance(recipient, Team):
results[recipient]["organizations"] = {recipient.organization}
# if isinstance(recipient, Team):
# results[recipient]["organizations"] = {recipient.organization}

if isinstance(recipient, User):
results[recipient]["organizations"] = user.get_orgs()
# if isinstance(recipient, User):
# results[recipient]["organizations"] = user.get_orgs()

return results

Expand Down Expand Up @@ -110,25 +112,26 @@ def serialize(
:param kwargs: The same `kwargs` as `get_attrs`.
:returns A mapping. See example.
"""
type_option: Optional[NotificationSettingTypes] = kwargs.get("type")
types_to_serialize = {type_option} if type_option else set(VALID_VALUES_FOR_KEY.keys())

project_ids = {_.id for _ in attrs["projects"]}
organization_ids = {_.id for _ in attrs["organizations"]}

data = get_fallback_settings(
types_to_serialize,
project_ids,
organization_ids,
recipient=RpcActor.from_object(obj),
)
# type_option: Optional[NotificationSettingTypes] = kwargs.get("type")
# types_to_serialize = {type_option} if type_option else set(VALID_VALUES_FOR_KEY.keys())

# project_ids = {_.id for _ in attrs["projects"]}
# organization_ids = {_.id for _ in attrs["organizations"]}

data: MutableMapping[
str, MutableMapping[str, MutableMapping[int, MutableMapping[str, str]]]
] = defaultdict(lambda: defaultdict(lambda: defaultdict(dict)))
# data = get_fallback_settings(
# types_to_serialize,
# project_ids,
# organization_ids,
# recipient=RpcActor.from_object(obj),
# )

# Forgive the variable name, I wanted the following lines to be legible.
for n in attrs["settings"]:
# Filter out invalid notification settings.
if (n.scope_str == "project" and n.scope_identifier not in project_ids) or (
n.scope_str == "organization" and n.scope_identifier not in organization_ids
):
if n.scope_str not in ["project", "organization"]:
continue

# Override the notification settings.
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/models/notificationsetting.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def provider_str(self) -> str:
(NotificationSettingTypes.QUOTA, "quota"),
(NotificationSettingTypes.QUOTA_ERRORS, "quotaErrors"),
(NotificationSettingTypes.QUOTA_TRANSACTIONS, "quotaTransactions"),
(NotificationSettingTypes.QUOTA_ATTACHMENTS, "quotaAttacments"),
(NotificationSettingTypes.QUOTA_ATTACHMENTS, "quotaAttachments"),
(NotificationSettingTypes.QUOTA_REPLAYS, "quotaReplays"),
(NotificationSettingTypes.QUOTA_WARNINGS, "quotaWarnings"),
(NotificationSettingTypes.QUOTA_SPEND_ALLOCATIONS, "quotaSpendAllocations"),
Expand Down
61 changes: 44 additions & 17 deletions static/app/views/settings/account/accountNotificationFineTuning.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import {
ACCOUNT_NOTIFICATION_FIELDS,
FineTuneField,
} from 'sentry/views/settings/account/notifications/fields';
import NotificationSettingsByType from 'sentry/views/settings/account/notifications/notificationSettingsByType';
import NotificationSettingsByType, {
OrganizationSelectHeader,
} from 'sentry/views/settings/account/notifications/notificationSettingsByType';
import {
getNotificationTypeFromPathname,
groupByOrganization,
Expand Down Expand Up @@ -69,7 +71,6 @@ function AccountNotificationsByProject({projects, field}: ANBPProps) {
<Fragment>
{data.map(({name, projects: projectFields}) => (
<div key={name}>
<PanelHeader>{name}</PanelHeader>
{projectFields.map(f => (
<PanelBodyLineItem key={f.name}>
<SelectField
Expand Down Expand Up @@ -133,20 +134,33 @@ type State = AsyncView['state'] & {
emails: UserEmail[] | null;
fineTuneData: Record<string, any> | null;
notifications: Record<string, any> | null;
organizationId: string;
projects: Project[] | null;
};

class AccountNotificationFineTuning extends AsyncView<Props, State> {
getDefaultState() {
return {
...super.getDefaultState(),
emails: [],
fineTuneData: null,
notifications: [],
projects: [],
organizationId: this.props.organizations[0].id,
};
}

getEndpoints(): ReturnType<AsyncView['getEndpoints']> {
const {fineTuneType: pathnameType} = this.props.params;
const orgId = this.state?.organizationId || this.props.organizations[0].id;
const fineTuneType = getNotificationTypeFromPathname(pathnameType);
const endpoints = [
['notifications', '/users/me/notifications/'],
['fineTuneData', `/users/me/notifications/${fineTuneType}/`],
];

if (isGroupedByProject(fineTuneType)) {
endpoints.push(['projects', '/projects/']);
endpoints.push(['projects', `/projects/?organization_id=${orgId}`]);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
endpoints.push(['projects', `/projects/?organization_id=${orgId}`]);
endpoints.push(['projects', `/projects/?organizationId=${orgId}`]);

To go with the other casing change.

}

endpoints.push(['emails', '/users/me/emails/']);
Expand Down Expand Up @@ -176,6 +190,14 @@ class AccountNotificationFineTuning extends AsyncView<Props, State> {
);
}

handleOrgChange = (option: {label: string; value: string}) => {
this.setState({organizationId: option.value});
const self = this;
setTimeout(() => {
self.reloadData();
}, 0);
};

renderBody() {
const {params} = this.props;
const {fineTuneType: pathnameType} = params;
Expand All @@ -202,7 +224,6 @@ class AccountNotificationFineTuning extends AsyncView<Props, State> {
if (!notifications || !fineTuneData) {
return null;
}

return (
<div>
<SettingsPageHeader title={title} />
Expand All @@ -225,19 +246,25 @@ class AccountNotificationFineTuning extends AsyncView<Props, State> {
</Form>
)}
<Panel>
<PanelHeader hasButtons={isProject}>
{isProject ? (
<Fragment>
<OrganizationSelectHeader
organizations={this.props.organizations}
organizationId={this.state.organizationId}
handleOrgChange={this.handleOrgChange}
/>
{this.renderSearchInput({
placeholder: t('Search Projects'),
url,
stateKey,
})}
</Fragment>
) : (
<Heading>{t('Organizations')}</Heading>
)}
</PanelHeader>
<PanelBody>
<PanelHeader hasButtons={isProject}>
<Heading>{isProject ? t('Projects') : t('Organizations')}</Heading>
<div>
{isProject &&
this.renderSearchInput({
placeholder: t('Search Projects'),
url,
stateKey,
})}
</div>
</PanelHeader>

<Form
saveOnBlur
apiMethod="PUT"
Expand Down Expand Up @@ -269,4 +296,4 @@ const Heading = styled('div')`
flex: 1;
`;

export default AccountNotificationFineTuning;
export default withOrganizations(AccountNotificationFineTuning);
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import Form from 'sentry/components/forms/form';
import JsonForm from 'sentry/components/forms/jsonForm';
import {t} from 'sentry/locale';
import {OrganizationSummary} from 'sentry/types';
import withOrganizations from 'sentry/utils/withOrganizations';
import {
NotificationSettingsByProviderObject,
NotificationSettingsObject,
} from 'sentry/views/settings/account/notifications/constants';
import {StyledJsonForm} from 'sentry/views/settings/account/notifications/notificationSettingsByProjects';
import {
getParentData,
getParentField,
Expand Down Expand Up @@ -38,11 +38,16 @@ function NotificationSettingsByOrganization({
initialData={getParentData(notificationType, notificationSettings, organizations)}
onSubmitSuccess={onSubmitSuccess}
>
<JsonForm
<StyledJsonForm
title={t('Organizations')}
fields={organizations.map(organization =>
getParentField(notificationType, notificationSettings, organization, onChange)
)}
fields={organizations.map(organization => {
return getParentField(
notificationType,
notificationSettings,
organization,
onChange
);
})}
/>
</Form>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import {Project} from 'sentry/types';
import NotificationSettingsByProjects from 'sentry/views/settings/account/notifications/notificationSettingsByProjects';

const renderComponent = (projects: Project[]) => {
const {routerContext} = initializeOrg();
const {routerContext, organization} = initializeOrg();

MockApiClient.addMockResponse({
url: '/projects/',
url: `/projects/`,
method: 'GET',
body: projects,
});
Expand All @@ -28,6 +28,9 @@ const renderComponent = (projects: Project[]) => {
notificationSettings={notificationSettings}
onChange={jest.fn()}
onSubmitSuccess={jest.fn()}
organizationId={organization.id}
organizations={[organization]}
handleOrgChange={jest.fn()}
/>,
{context: routerContext}
);
Expand Down
Loading