From fc282e7bb02b6f07e273e19d2be606707b34416b Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 29 Jul 2024 17:53:34 +0000 Subject: [PATCH 01/32] Add base notification settings page to deployment --- .../NotificationsPage/NotificationsPage.tsx | 314 ++++++++++++++++++ site/src/pages/DeploySettingsPage/Sidebar.tsx | 4 + .../pages/ManagementSettingsPage/Sidebar.tsx | 3 + site/src/router.tsx | 8 + 4 files changed, 329 insertions(+) create mode 100644 site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx new file mode 100644 index 0000000000000..3e08fa8d698d9 --- /dev/null +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -0,0 +1,314 @@ +import type { Interpolation, Theme } from "@emotion/react"; +import EmailIcon from "@mui/icons-material/EmailOutlined"; +import WebhookIcon from "@mui/icons-material/LanguageOutlined"; +import Card from "@mui/material/Card"; +import Divider from "@mui/material/Divider"; +import List from "@mui/material/List"; +import ListItem from "@mui/material/ListItem"; +import ListItemIcon from "@mui/material/ListItemIcon"; +import ListItemText, { listItemTextClasses } from "@mui/material/ListItemText"; +import Switch from "@mui/material/Switch"; +import TextField from "@mui/material/TextField"; +import ToggleButton from "@mui/material/ToggleButton"; +import ToggleButtonGroup from "@mui/material/ToggleButtonGroup"; +import type { FC } from "react"; +import { FormFields, FormSection, HorizontalForm } from "components/Form/Form"; +import { Section } from "pages/UserSettingsPage/Section"; +import Button from "@mui/material/Button"; +import { Stack } from "components/Stack/Stack"; + +export const NotificationsPage: FC = () => { + return ( +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ ); +}; + +export default NotificationsPage; + +const styles = { + listHeader: (theme) => ({ + background: theme.palette.background.paper, + borderBottom: `1px solid ${theme.palette.divider}`, + }), + listItemText: { + [`& .${listItemTextClasses.primary}`]: { + fontSize: 14, + fontWeight: 500, + }, + [`& .${listItemTextClasses.secondary}`]: { + fontSize: 14, + }, + }, + toggleGroup: (theme) => ({ + border: `1px solid ${theme.palette.divider}`, + borderRadius: 4, + }), + toggleButton: (theme) => ({ + border: 0, + borderRadius: 4, + fontSize: 16, + padding: "4px 8px", + color: theme.palette.text.disabled, + + "&:hover": { + color: theme.palette.text.primary, + }, + + "& svg": { + fontSize: "inherit", + }, + }), +} as Record>; diff --git a/site/src/pages/DeploySettingsPage/Sidebar.tsx b/site/src/pages/DeploySettingsPage/Sidebar.tsx index e473ab94ca510..6023947c4c0ae 100644 --- a/site/src/pages/DeploySettingsPage/Sidebar.tsx +++ b/site/src/pages/DeploySettingsPage/Sidebar.tsx @@ -3,6 +3,7 @@ import HubOutlinedIcon from "@mui/icons-material/HubOutlined"; import InsertChartIcon from "@mui/icons-material/InsertChart"; import LaunchOutlined from "@mui/icons-material/LaunchOutlined"; import LockRounded from "@mui/icons-material/LockOutlined"; +import NotificationsIcon from "@mui/icons-material/NotificationsNoneOutlined"; import Globe from "@mui/icons-material/PublicOutlined"; import ApprovalIcon from "@mui/icons-material/VerifiedUserOutlined"; import VpnKeyOutlined from "@mui/icons-material/VpnKeyOutlined"; @@ -47,6 +48,9 @@ export const Sidebar: FC = () => { Observability + + Notifications + ); }; diff --git a/site/src/pages/ManagementSettingsPage/Sidebar.tsx b/site/src/pages/ManagementSettingsPage/Sidebar.tsx index 5fb728117738c..5930d030d4737 100644 --- a/site/src/pages/ManagementSettingsPage/Sidebar.tsx +++ b/site/src/pages/ManagementSettingsPage/Sidebar.tsx @@ -110,6 +110,9 @@ const DeploymentSettingsNavigation: FC = ({ Auditing + + Notifications + )} diff --git a/site/src/router.tsx b/site/src/router.tsx index 615d12969e184..7ff8653c746d0 100644 --- a/site/src/router.tsx +++ b/site/src/router.tsx @@ -262,6 +262,13 @@ const WorkspaceProxyHealthPage = lazy( const ProvisionerDaemonsHealthPage = lazy( () => import("./pages/HealthPage/ProvisionerDaemonsPage"), ); +const UserNotificationsPage = lazy( + () => import("./pages/UserSettingsPage/NotificationsPage/NotificationsPage"), +); +const DeployNotificationsPage = lazy( + () => + import("./pages/DeploySettingsPage/NotificationsPage/NotificationsPage"), +); const RoutesWithSuspense = () => { return ( @@ -411,6 +418,7 @@ export const router = createBrowserRouter( } /> {groupsRouter()} } /> + } /> }> From 997e0d38feff9e5001ef538967f3683fc4fc02cf Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 29 Jul 2024 15:11:14 +0000 Subject: [PATCH 02/32] Add base notifications components --- .../NotificationsPage/NotificationsPage.tsx | 103 ++++++++++++++++++ site/src/pages/UserSettingsPage/Sidebar.tsx | 4 + site/src/router.tsx | 5 +- 3 files changed, 108 insertions(+), 4 deletions(-) create mode 100644 site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx new file mode 100644 index 0000000000000..4cbb0e83d30c1 --- /dev/null +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -0,0 +1,103 @@ +import type { Interpolation, Theme } from "@emotion/react"; +import EmailIcon from "@mui/icons-material/EmailOutlined"; +import Card from "@mui/material/Card"; +import Divider from "@mui/material/Divider"; +import List from "@mui/material/List"; +import ListItem from "@mui/material/ListItem"; +import ListItemIcon from "@mui/material/ListItemIcon"; +import ListItemText, { listItemTextClasses } from "@mui/material/ListItemText"; +import Switch from "@mui/material/Switch"; +import type { FC } from "react"; +import { Section } from "../Section"; + +export const NotificationsPage: FC = () => { + return ( +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ ); +}; + +export default NotificationsPage; + +const styles = { + listHeader: (theme) => ({ + background: theme.palette.background.paper, + borderBottom: `1px solid ${theme.palette.divider}`, + }), + listItemText: { + [`& .${listItemTextClasses.primary}`]: { + fontSize: 14, + fontWeight: 500, + }, + [`& .${listItemTextClasses.secondary}`]: { + fontSize: 14, + }, + }, + listItemEndIcon: (theme) => ({ + minWidth: 0, + fontSize: 20, + color: theme.palette.text.secondary, + + "& svg": { + fontSize: "inherit", + }, + }), +} as Record>; diff --git a/site/src/pages/UserSettingsPage/Sidebar.tsx b/site/src/pages/UserSettingsPage/Sidebar.tsx index 01b2ba8be88b6..19b9549d49788 100644 --- a/site/src/pages/UserSettingsPage/Sidebar.tsx +++ b/site/src/pages/UserSettingsPage/Sidebar.tsx @@ -2,6 +2,7 @@ import AppearanceIcon from "@mui/icons-material/Brush"; import ScheduleIcon from "@mui/icons-material/EditCalendarOutlined"; import FingerprintOutlinedIcon from "@mui/icons-material/FingerprintOutlined"; import SecurityIcon from "@mui/icons-material/LockOutlined"; +import NotificationsIcon from "@mui/icons-material/NotificationsNoneOutlined"; import AccountIcon from "@mui/icons-material/Person"; import VpnKeyOutlined from "@mui/icons-material/VpnKeyOutlined"; import type { FC } from "react"; @@ -56,6 +57,9 @@ export const Sidebar: FC = ({ user }) => { Tokens + + Notifications + ); }; diff --git a/site/src/router.tsx b/site/src/router.tsx index 7ff8653c746d0..b455093e39579 100644 --- a/site/src/router.tsx +++ b/site/src/router.tsx @@ -265,10 +265,6 @@ const ProvisionerDaemonsHealthPage = lazy( const UserNotificationsPage = lazy( () => import("./pages/UserSettingsPage/NotificationsPage/NotificationsPage"), ); -const DeployNotificationsPage = lazy( - () => - import("./pages/DeploySettingsPage/NotificationsPage/NotificationsPage"), -); const RoutesWithSuspense = () => { return ( @@ -439,6 +435,7 @@ export const router = createBrowserRouter( } /> } /> + } /> {/* In order for the 404 page to work properly the routes that start with From 73f847113cd7fb52c966e969015d50969f16f817 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 2 Aug 2024 12:51:56 +0000 Subject: [PATCH 03/32] Bind notifications into the user account notifications page --- ...235_update_notification_templates.down.sql | 0 ...00235_update_notification_templates.up.sql | 5 + coderd/notifications.go | 2 +- codersdk/notifications.go | 28 ++- pnpm-lock.yaml | 32 +-- site/src/api/api.ts | 25 ++ site/src/api/queries/notifications.ts | 55 ++++ site/src/api/typesGenerated.ts | 12 +- .../NotificationsPage/NotificationsPage.tsx | 236 ++++++++++++++---- site/src/pages/UserSettingsPage/Sidebar.tsx | 11 +- 10 files changed, 315 insertions(+), 91 deletions(-) create mode 100644 coderd/database/migrations/000235_update_notification_templates.down.sql create mode 100644 coderd/database/migrations/000235_update_notification_templates.up.sql create mode 100644 site/src/api/queries/notifications.ts diff --git a/coderd/database/migrations/000235_update_notification_templates.down.sql b/coderd/database/migrations/000235_update_notification_templates.down.sql new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/coderd/database/migrations/000235_update_notification_templates.up.sql b/coderd/database/migrations/000235_update_notification_templates.up.sql new file mode 100644 index 0000000000000..73b9e48c65fb4 --- /dev/null +++ b/coderd/database/migrations/000235_update_notification_templates.up.sql @@ -0,0 +1,5 @@ +UPDATE notification_templates +SET + "group" = 'User Events' +WHERE + id = '4e19c0ac-94e1-4532-9515-d1801aa283b2'; diff --git a/coderd/notifications.go b/coderd/notifications.go index bdf71f99cab98..97f88beddc3cf 100644 --- a/coderd/notifications.go +++ b/coderd/notifications.go @@ -277,7 +277,7 @@ func convertNotificationTemplates(in []database.NotificationTemplate) (out []cod BodyTemplate: tmpl.BodyTemplate, Actions: string(tmpl.Actions), Group: tmpl.Group.String, - Method: string(tmpl.Method.NotificationMethod), + Method: codersdk.NotificationTemplateMethod(tmpl.Method.NotificationMethod), Kind: string(tmpl.Kind), }) } diff --git a/codersdk/notifications.go b/codersdk/notifications.go index 92870b4dd2b95..03a430285920b 100644 --- a/codersdk/notifications.go +++ b/codersdk/notifications.go @@ -16,15 +16,23 @@ type NotificationsSettings struct { NotifierPaused bool `json:"notifier_paused"` } +type NotificationTemplateMethod string + +const ( + NotificationTemplateEmailMethod NotificationTemplateMethod = "email" + NotificationTemplateWebhookMethod NotificationTemplateMethod = "webhook" + NotificationTemplateNotDefinedMethod NotificationTemplateMethod = "" +) + type NotificationTemplate struct { - ID uuid.UUID `json:"id" format:"uuid"` - Name string `json:"name"` - TitleTemplate string `json:"title_template"` - BodyTemplate string `json:"body_template"` - Actions string `json:"actions" format:""` - Group string `json:"group"` - Method string `json:"method"` - Kind string `json:"kind"` + ID uuid.UUID `json:"id" format:"uuid"` + Name string `json:"name"` + TitleTemplate string `json:"title_template"` + BodyTemplate string `json:"body_template"` + Actions string `json:"actions" format:""` + Group string `json:"group"` + Method NotificationTemplateMethod `json:"method" enums:"email,webhook,''"` + Kind string `json:"kind"` } type NotificationMethodsResponse struct { @@ -73,7 +81,7 @@ func (c *Client) PutNotificationsSettings(ctx context.Context, settings Notifica // UpdateNotificationTemplateMethod modifies a notification template to use a specific notification method, overriding // the method set in the deployment configuration. -func (c *Client) UpdateNotificationTemplateMethod(ctx context.Context, notificationTemplateID uuid.UUID, method string) error { +func (c *Client) UpdateNotificationTemplateMethod(ctx context.Context, notificationTemplateID uuid.UUID, method NotificationTemplateMethod) error { res, err := c.Request(ctx, http.MethodPut, fmt.Sprintf("/api/v2/notifications/templates/%s/method", notificationTemplateID), UpdateNotificationTemplateMethod{Method: method}, @@ -193,7 +201,7 @@ func (c *Client) GetNotificationDispatchMethods(ctx context.Context) (Notificati } type UpdateNotificationTemplateMethod struct { - Method string `json:"method,omitempty" example:"webhook"` + Method NotificationTemplateMethod `json:"method,omitempty" enums:"email,webhook" example:"webhook"` } type UpdateUserNotificationPreferences struct { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d04d1cf7a21e5..e5e4d2584e40f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1,35 +1,29 @@ -lockfileVersion: '9.0' +lockfileVersion: '6.0' settings: autoInstallPeers: true excludeLinksFromLockfile: false -importers: +dependencies: + exec: + specifier: ^0.2.1 + version: 0.2.1 - .: - dependencies: - exec: - specifier: ^0.2.1 - version: 0.2.1 - devDependencies: - prettier: - specifier: 3.0.0 - version: 3.0.0 +devDependencies: + prettier: + specifier: 3.0.0 + version: 3.0.0 packages: - exec@0.2.1: + /exec@0.2.1: resolution: {integrity: sha512-lE5ZlJgRYh+rmwidatL2AqRA/U9IBoCpKlLriBmnfUIrV/Rj4oLjb63qZ57iBCHWi5j9IjLt5wOWkFYPiTfYAg==} engines: {node: '>= v0.9.1'} deprecated: deprecated in favor of builtin child_process.execFile + dev: false - prettier@3.0.0: + /prettier@3.0.0: resolution: {integrity: sha512-zBf5eHpwHOGPC47h0zrPyNn+eAEIdEzfywMoYn2XPi0P44Zp0tSq64rq0xAREh4auw2cJZHo9QUob+NqCQky4g==} engines: {node: '>=14'} hasBin: true - -snapshots: - - exec@0.2.1: {} - - prettier@3.0.0: {} + dev: true diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 7aeefe98a444c..aa5b139ee42ef 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -1995,6 +1995,31 @@ class ApiMethods { return response.data; }; + + getUserNotificationPreferences = async (userId: string) => { + const res = await this.axios.get( + `/api/v2/users/${userId}/notifications/preferences`, + ); + return res.data ?? []; + }; + + putUserNotificationPreferences = async ( + userId: string, + req: TypesGen.UpdateUserNotificationPreferences, + ) => { + const res = await this.axios.put( + `/api/v2/users/${userId}/notifications/preferences`, + req, + ); + return res.data; + }; + + getSystemNotificationTemplates = async () => { + const res = await this.axios.get( + `/api/v2/notifications/templates/system`, + ); + return res.data; + }; } // This is a hard coded CSRF token/cookie pair for local development. In prod, diff --git a/site/src/api/queries/notifications.ts b/site/src/api/queries/notifications.ts new file mode 100644 index 0000000000000..2c53d4e33a1d5 --- /dev/null +++ b/site/src/api/queries/notifications.ts @@ -0,0 +1,55 @@ +import type { QueryClient, UseMutationOptions } from "react-query"; +import { API } from "api/api"; +import type { + NotificationPreference, + UpdateUserNotificationPreferences, +} from "api/typesGenerated"; + +const userNotificationPreferencesKey = (userId: string) => [ + "users", + userId, + "notifications", + "preferences", +]; + +export const userNotificationPreferences = (userId: string) => { + return { + queryKey: userNotificationPreferencesKey(userId), + queryFn: () => API.getUserNotificationPreferences(userId), + }; +}; + +export const updateUserNotificationPreferences = ( + userId: string, + queryClient: QueryClient, +) => { + return { + mutationFn: (req) => { + return API.putUserNotificationPreferences(userId, req); + }, + onMutate: (data) => { + queryClient.setQueryData( + userNotificationPreferencesKey(userId), + Object.entries(data.template_disabled_map).map( + ([id, disabled]) => + ({ + id, + disabled, + updated_at: new Date().toISOString(), + }) as NotificationPreference, + ), + ); + }, + } satisfies UseMutationOptions< + NotificationPreference[], + unknown, + UpdateUserNotificationPreferences + >; +}; + +export const systemNotificationTemplatesByGroup = () => { + return { + queryKey: ["notifications", "templates", "system"], + queryFn: () => API.getSystemNotificationTemplates(), + }; +}; diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 5c2dc816fea1e..4e45ec64a8c4a 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -730,7 +730,7 @@ export interface NotificationTemplate { readonly body_template: string; readonly actions: string; readonly group: string; - readonly method: string; + readonly method: NotificationTemplateMethod; readonly kind: string; } @@ -1474,7 +1474,7 @@ export interface UpdateCheckResponse { // From codersdk/notifications.go export interface UpdateNotificationTemplateMethod { - readonly method?: string; + readonly method?: NotificationTemplateMethod; } // From codersdk/organizations.go @@ -2199,6 +2199,14 @@ export const LoginTypes: LoginType[] = [ "token", ]; +// From codersdk/notifications.go +export type NotificationTemplateMethod = "" | "email" | "webhook"; +export const NotificationTemplateMethods: NotificationTemplateMethod[] = [ + "", + "email", + "webhook", +]; + // From codersdk/oauth2.go export type OAuth2ProviderGrantType = "authorization_code" | "refresh_token"; export const OAuth2ProviderGrantTypes: OAuth2ProviderGrantType[] = [ diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx index 4cbb0e83d30c1..8f2ba1f68e3ac 100644 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -1,5 +1,7 @@ import type { Interpolation, Theme } from "@emotion/react"; import EmailIcon from "@mui/icons-material/EmailOutlined"; +import DeploymentIcon from "@mui/icons-material/LanguageOutlined"; +import WebhookIcon from "@mui/icons-material/WebhookOutlined"; import Card from "@mui/material/Card"; import Divider from "@mui/material/Divider"; import List from "@mui/material/List"; @@ -7,76 +9,199 @@ import ListItem from "@mui/material/ListItem"; import ListItemIcon from "@mui/material/ListItemIcon"; import ListItemText, { listItemTextClasses } from "@mui/material/ListItemText"; import Switch from "@mui/material/Switch"; -import type { FC } from "react"; +import Tooltip from "@mui/material/Tooltip"; +import { Fragment, type FC } from "react"; +import { useMutation, useQueries, useQueryClient } from "react-query"; +import { + systemNotificationTemplatesByGroup, + updateUserNotificationPreferences, + userNotificationPreferences, +} from "api/queries/notifications"; +import type { + NotificationPreference, + NotificationTemplate, +} from "api/typesGenerated"; +import { displaySuccess } from "components/GlobalSnackbar/utils"; +import { Loader } from "components/Loader/Loader"; +import { Stack } from "components/Stack/Stack"; +import { useAuthenticated } from "contexts/auth/RequireAuth"; import { Section } from "../Section"; +type PreferenceSwitchProps = { + id: string; + disabled: boolean; + onToggle: (checked: boolean) => Record; +}; + +const PreferenceSwitch: FC = ({ + id, + disabled, + onToggle, +}) => { + const { user } = useAuthenticated(); + const queryClient = useQueryClient(); + const updatePreferences = useMutation( + updateUserNotificationPreferences(user.id, queryClient), + ); + + return ( + { + await updatePreferences.mutateAsync({ + template_disabled_map: onToggle(checked), + }); + displaySuccess("Notification preference updated"); + }} + /> + ); +}; + export const NotificationsPage: FC = () => { + const { user } = useAuthenticated(); + const [disabledPreferences, templatesByGroup] = useQueries({ + queries: [ + { + ...userNotificationPreferences(user.id), + select: selectDisabledPreferences, + }, + { + ...systemNotificationTemplatesByGroup(), + select: selectTemplatesByGroup, + }, + ], + }); + return (
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + {templatesByGroup.data && disabledPreferences.data ? ( + + {Object.entries(templatesByGroup.data).map(([group, templates]) => { + const allDisabled = templates.some((tpl) => { + return disabledPreferences.data[tpl.id] === true; + }); + + return ( + + + + + { + const updated = { ...disabledPreferences.data }; + for (const tpl of templates) { + updated[tpl.id] = !checked; + } + return updated; + }} + /> + + + + {templates.map((tmpl) => { + return ( + + + + { + return { + ...disabledPreferences.data, + [tmpl.id]: !checked, + }; + }} + /> + + + + {tmpl.method === "email" && ( + + + + )} + {tmpl.method === "webhook" && ( + + + + )} + {tmpl.method === "" && ( + + + + )} + + + + + ); + })} + + + ); + })} + + ) : ( + + )}
); }; export default NotificationsPage; +function selectDisabledPreferences(data: NotificationPreference[]) { + return data.reduce( + (acc, pref) => { + acc[pref.id] = pref.disabled; + return acc; + }, + {} as Record, + ); +} + +function selectTemplatesByGroup( + data: NotificationTemplate[], +): Record { + return data.reduce( + (acc, tpl) => { + if (!acc[tpl.group]) { + acc[tpl.group] = []; + } + acc[tpl.group].push(tpl); + return acc; + }, + {} as Record, + ); +} + const styles = { listHeader: (theme) => ({ background: theme.palette.background.paper, @@ -86,6 +211,7 @@ const styles = { [`& .${listItemTextClasses.primary}`]: { fontSize: 14, fontWeight: 500, + textTransform: "capitalize", }, [`& .${listItemTextClasses.secondary}`]: { fontSize: 14, diff --git a/site/src/pages/UserSettingsPage/Sidebar.tsx b/site/src/pages/UserSettingsPage/Sidebar.tsx index 19b9549d49788..8b5190b6b16bf 100644 --- a/site/src/pages/UserSettingsPage/Sidebar.tsx +++ b/site/src/pages/UserSettingsPage/Sidebar.tsx @@ -21,9 +21,10 @@ interface SidebarProps { } export const Sidebar: FC = ({ user }) => { - const { entitlements } = useDashboard(); + const { entitlements, experiments } = useDashboard(); const showSchedulePage = entitlements.features.advanced_template_scheduling.enabled; + const showNotificationsPage = experiments.includes("notifications"); return ( @@ -57,9 +58,11 @@ export const Sidebar: FC = ({ user }) => { Tokens - - Notifications - + {showNotificationsPage && ( + + Notifications + + )} ); }; From 33c9ab07fcf653df906da619cbc7c8a64c1d588a Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 2 Aug 2024 13:00:13 +0000 Subject: [PATCH 04/32] Remove deployment notifications page --- site/src/router.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/site/src/router.tsx b/site/src/router.tsx index b455093e39579..766866297045f 100644 --- a/site/src/router.tsx +++ b/site/src/router.tsx @@ -414,7 +414,6 @@ export const router = createBrowserRouter( } /> {groupsRouter()} } /> - } /> }> From bbc8cbb00113cfa034f8459e379737bc3361d49b Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 2 Aug 2024 13:50:26 +0000 Subject: [PATCH 05/32] Add test for toggling notifications --- .../NotificationsPage.test.tsx | 152 ++++++++++++++++++ .../NotificationsPage/NotificationsPage.tsx | 2 +- 2 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.test.tsx diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.test.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.test.tsx new file mode 100644 index 0000000000000..a5d795f70ed28 --- /dev/null +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.test.tsx @@ -0,0 +1,152 @@ +import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { http, HttpResponse } from "msw"; +import type { + Experiments, + NotificationPreference, + NotificationTemplate, + UpdateUserNotificationPreferences, +} from "api/typesGenerated"; +import { renderWithAuth } from "testHelpers/renderHelpers"; +import { server } from "testHelpers/server"; +import NotificationsPage from "./NotificationsPage"; + +test("can enable and disable notifications", async () => { + server.use( + http.get("/api/v2/experiments", () => + HttpResponse.json(["notifications"] as Experiments), + ), + http.get("/api/v2/users/:userId/notifications/preferences", () => + HttpResponse.json(null), + ), + http.get("/api/v2/notifications/templates/system", () => + HttpResponse.json(notificationsTemplateSystemRes), + ), + http.put< + { userId: string }, + UpdateUserNotificationPreferences, + NotificationPreference[] + >( + "/api/v2/users/:userId/notifications/preferences", + async ({ request }) => { + const body = await request.json(); + const res: NotificationPreference[] = Object.entries(body).map( + ([id, disabled]) => ({ + disabled, + id, + updated_at: new Date().toISOString(), + }), + ); + return HttpResponse.json(res); + }, + ), + ); + renderWithAuth(); + const user = userEvent.setup(); + const workspaceGroupTemplates = notificationsTemplateSystemRes.filter( + (t) => t.group === "Workspace Events", + ); + + // Test notification groups + const workspaceGroupSwitch = await screen.findByLabelText("Workspace Events"); + await user.click(workspaceGroupSwitch); + await screen.findByText("Notification preferences updated"); + expect(workspaceGroupSwitch).not.toBeChecked(); + for (const template of workspaceGroupTemplates) { + const templateSwitch = screen.getByLabelText(template.name); + expect(templateSwitch).not.toBeChecked(); + } + + await user.click(workspaceGroupSwitch); + await screen.findByText("Notification preferences updated"); + expect(workspaceGroupSwitch).toBeChecked(); + for (const template of workspaceGroupTemplates) { + const templateSwitch = screen.getByLabelText(template.name); + expect(templateSwitch).toBeChecked(); + } + + // Test individual notifications + const workspaceDeletedSwitch = screen.getByLabelText("Workspace Deleted"); + await user.click(workspaceDeletedSwitch); + await screen.findByText("Notification preferences updated"); + expect(workspaceDeletedSwitch).not.toBeChecked(); + + await user.click(workspaceDeletedSwitch); + await screen.findByText("Notification preferences updated"); + expect(workspaceDeletedSwitch).toBeChecked(); +}); + +const notificationsTemplateSystemRes: NotificationTemplate[] = [ + { + id: "f517da0b-cdc9-410f-ab89-a86107c420ed", + name: "Workspace Deleted", + title_template: 'Workspace "{{.Labels.name}}" deleted', + body_template: + 'Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** was deleted.\nThe specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} ({{ .Labels.initiator }}){{end}}**".', + actions: + '[{"url": "{{ base_url }}/workspaces", "label": "View workspaces"}, {"url": "{{ base_url }}/templates", "label": "View templates"}]', + group: "Workspace Events", + method: "", + kind: "system", + }, + { + id: "381df2a9-c0c0-4749-420f-80a9280c66f9", + name: "Workspace Autobuild Failed", + title_template: 'Workspace "{{.Labels.name}}" autobuild failed', + body_template: + 'Hi {{.UserName}}\nAutomatic build of your workspace **{{.Labels.name}}** failed.\nThe specified reason was "**{{.Labels.reason}}**".', + actions: + '[{"url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}", "label": "View workspace"}]', + group: "Workspace Events", + method: "", + kind: "system", + }, + { + id: "c34a0c09-0704-4cac-bd1c-0c0146811c2b", + name: "Workspace updated automatically", + title_template: 'Workspace "{{.Labels.name}}" updated automatically', + body_template: + "Hi {{.UserName}}\nYour workspace **{{.Labels.name}}** has been updated automatically to the latest template version ({{.Labels.template_version_name}}).", + actions: + '[{"url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}", "label": "View workspace"}]', + group: "Workspace Events", + method: "", + kind: "system", + }, + { + id: "0ea69165-ec14-4314-91f1-69566ac3c5a0", + name: "Workspace Marked as Dormant", + title_template: 'Workspace "{{.Labels.name}}" marked as dormant', + body_template: + "Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** has been marked as [**dormant**](https://coder.com/docs/templates/schedule#dormancy-threshold-enterprise) because of {{.Labels.reason}}.\nDormant workspaces are [automatically deleted](https://coder.com/docs/templates/schedule#dormancy-auto-deletion-enterprise) after {{.Labels.timeTilDormant}} of inactivity.\nTo prevent deletion, use your workspace with the link below.", + actions: + '[{"url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}", "label": "View workspace"}]', + group: "Workspace Events", + method: "", + kind: "system", + }, + { + id: "51ce2fdf-c9ca-4be1-8d70-628674f9bc42", + name: "Workspace Marked for Deletion", + title_template: 'Workspace "{{.Labels.name}}" marked for deletion', + body_template: + "Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** has been marked for **deletion** after {{.Labels.timeTilDormant}} of [dormancy](https://coder.com/docs/templates/schedule#dormancy-auto-deletion-enterprise) because of {{.Labels.reason}}.\nTo prevent deletion, use your workspace with the link below.", + actions: + '[{"url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}", "label": "View workspace"}]', + group: "Workspace Events", + method: "", + kind: "system", + }, + { + id: "4e19c0ac-94e1-4532-9515-d1801aa283b2", + name: "User account created", + title_template: 'User account "{{.Labels.created_account_name}}" created', + body_template: + "Hi {{.UserName}},\nNew user account **{{.Labels.created_account_name}}** has been created.", + actions: + '[{"url": "{{ base_url }}/deployment/users?filter=status%3Aactive", "label": "View accounts"}]', + group: "User Events", + method: "", + kind: "system", + }, +]; diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx index 8f2ba1f68e3ac..9bb93e1f3d933 100644 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -53,7 +53,7 @@ const PreferenceSwitch: FC = ({ await updatePreferences.mutateAsync({ template_disabled_map: onToggle(checked), }); - displaySuccess("Notification preference updated"); + displaySuccess("Notification preferences updated"); }} /> ); From 7227a42204929dee54b1e7988baf214855bf9bac Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 5 Aug 2024 14:51:51 +0000 Subject: [PATCH 06/32] Update migration --- ...39_update_notification_templates.down.sql} | 0 ...0239_update_notification_templates.up.sql} | 0 pnpm-lock.yaml | 32 +++++++++++-------- 3 files changed, 19 insertions(+), 13 deletions(-) rename coderd/database/migrations/{000235_update_notification_templates.down.sql => 000239_update_notification_templates.down.sql} (100%) rename coderd/database/migrations/{000235_update_notification_templates.up.sql => 000239_update_notification_templates.up.sql} (100%) diff --git a/coderd/database/migrations/000235_update_notification_templates.down.sql b/coderd/database/migrations/000239_update_notification_templates.down.sql similarity index 100% rename from coderd/database/migrations/000235_update_notification_templates.down.sql rename to coderd/database/migrations/000239_update_notification_templates.down.sql diff --git a/coderd/database/migrations/000235_update_notification_templates.up.sql b/coderd/database/migrations/000239_update_notification_templates.up.sql similarity index 100% rename from coderd/database/migrations/000235_update_notification_templates.up.sql rename to coderd/database/migrations/000239_update_notification_templates.up.sql diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e5e4d2584e40f..d04d1cf7a21e5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1,29 +1,35 @@ -lockfileVersion: '6.0' +lockfileVersion: '9.0' settings: autoInstallPeers: true excludeLinksFromLockfile: false -dependencies: - exec: - specifier: ^0.2.1 - version: 0.2.1 +importers: -devDependencies: - prettier: - specifier: 3.0.0 - version: 3.0.0 + .: + dependencies: + exec: + specifier: ^0.2.1 + version: 0.2.1 + devDependencies: + prettier: + specifier: 3.0.0 + version: 3.0.0 packages: - /exec@0.2.1: + exec@0.2.1: resolution: {integrity: sha512-lE5ZlJgRYh+rmwidatL2AqRA/U9IBoCpKlLriBmnfUIrV/Rj4oLjb63qZ57iBCHWi5j9IjLt5wOWkFYPiTfYAg==} engines: {node: '>= v0.9.1'} deprecated: deprecated in favor of builtin child_process.execFile - dev: false - /prettier@3.0.0: + prettier@3.0.0: resolution: {integrity: sha512-zBf5eHpwHOGPC47h0zrPyNn+eAEIdEzfywMoYn2XPi0P44Zp0tSq64rq0xAREh4auw2cJZHo9QUob+NqCQky4g==} engines: {node: '>=14'} hasBin: true - dev: true + +snapshots: + + exec@0.2.1: {} + + prettier@3.0.0: {} From 788de881ac18e7028408acaa10582f6e6e4a537f Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 5 Aug 2024 19:00:52 +0000 Subject: [PATCH 07/32] Update template notification methods --- coderd/apidoc/docs.go | 24 +- coderd/apidoc/swagger.json | 16 +- docs/api/notifications.md | 32 +- docs/api/schemas.md | 46 ++- enterprise/coderd/notifications.go | 2 +- site/src/api/api.ts | 18 + site/src/api/queries/notifications.ts | 32 ++ site/src/modules/notifications/utils.tsx | 21 + .../NotificationsPage/NotificationsPage.tsx | 364 ++++++------------ .../NotificationsPage/NotificationsPage.tsx | 69 ++-- site/src/router.tsx | 8 + 11 files changed, 318 insertions(+), 314 deletions(-) create mode 100644 site/src/modules/notifications/utils.tsx diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 962fccae0a4ea..f3c9db86028b0 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -10426,7 +10426,16 @@ const docTemplate = `{ "type": "string" }, "method": { - "type": "string" + "enum": [ + "email", + "webhook", + "''" + ], + "allOf": [ + { + "$ref": "#/definitions/codersdk.NotificationTemplateMethod" + } + ] }, "name": { "type": "string" @@ -10436,6 +10445,19 @@ const docTemplate = `{ } } }, + "codersdk.NotificationTemplateMethod": { + "type": "string", + "enum": [ + "email", + "webhook", + "" + ], + "x-enum-varnames": [ + "NotificationTemplateEmailMethod", + "NotificationTemplateWebhookMethod", + "NotificationTemplateNotDefinedMethod" + ] + }, "codersdk.NotificationsConfig": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 35b8b82a21888..68cd297715081 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -9345,7 +9345,12 @@ "type": "string" }, "method": { - "type": "string" + "enum": ["email", "webhook", "''"], + "allOf": [ + { + "$ref": "#/definitions/codersdk.NotificationTemplateMethod" + } + ] }, "name": { "type": "string" @@ -9355,6 +9360,15 @@ } } }, + "codersdk.NotificationTemplateMethod": { + "type": "string", + "enum": ["email", "webhook", ""], + "x-enum-varnames": [ + "NotificationTemplateEmailMethod", + "NotificationTemplateWebhookMethod", + "NotificationTemplateNotDefinedMethod" + ] + }, "codersdk.NotificationsConfig": { "type": "object", "properties": { diff --git a/docs/api/notifications.md b/docs/api/notifications.md index 528153ebd103b..c4a5d29138549 100644 --- a/docs/api/notifications.md +++ b/docs/api/notifications.md @@ -147,7 +147,7 @@ curl -X GET http://coder-server:8080/api/v2/notifications/templates/system \ "group": "string", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "kind": "string", - "method": "string", + "method": "email", "name": "string", "title_template": "string" } @@ -164,17 +164,25 @@ curl -X GET http://coder-server:8080/api/v2/notifications/templates/system \ Status Code **200** -| Name | Type | Required | Restrictions | Description | -| ------------------ | ------------ | -------- | ------------ | ----------- | -| `[array item]` | array | false | | | -| `» actions` | string | false | | | -| `» body_template` | string | false | | | -| `» group` | string | false | | | -| `» id` | string(uuid) | false | | | -| `» kind` | string | false | | | -| `» method` | string | false | | | -| `» name` | string | false | | | -| `» title_template` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| ------------------ | ------------------------------------------------------------------------------------ | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» actions` | string | false | | | +| `» body_template` | string | false | | | +| `» group` | string | false | | | +| `» id` | string(uuid) | false | | | +| `» kind` | string | false | | | +| `» method` | [codersdk.NotificationTemplateMethod](schemas.md#codersdknotificationtemplatemethod) | false | | | +| `» name` | string | false | | | +| `» title_template` | string | false | | | + +#### Enumerated Values + +| Property | Value | +| -------- | --------- | +| `method` | `email` | +| `method` | `webhook` | +| `method` | `''` | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 7406d135112f1..9bab357f15c3c 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -3184,7 +3184,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "group": "string", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "kind": "string", - "method": "string", + "method": "email", "name": "string", "title_template": "string" } @@ -3192,16 +3192,40 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o ### Properties -| Name | Type | Required | Restrictions | Description | -| ---------------- | ------ | -------- | ------------ | ----------- | -| `actions` | string | false | | | -| `body_template` | string | false | | | -| `group` | string | false | | | -| `id` | string | false | | | -| `kind` | string | false | | | -| `method` | string | false | | | -| `name` | string | false | | | -| `title_template` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| ---------------- | -------------------------------------------------------------------------- | -------- | ------------ | ----------- | +| `actions` | string | false | | | +| `body_template` | string | false | | | +| `group` | string | false | | | +| `id` | string | false | | | +| `kind` | string | false | | | +| `method` | [codersdk.NotificationTemplateMethod](#codersdknotificationtemplatemethod) | false | | | +| `name` | string | false | | | +| `title_template` | string | false | | | + +#### Enumerated Values + +| Property | Value | +| -------- | --------- | +| `method` | `email` | +| `method` | `webhook` | +| `method` | `''` | + +## codersdk.NotificationTemplateMethod + +```json +"email" +``` + +### Properties + +#### Enumerated Values + +| Value | +| --------- | +| `email` | +| `webhook` | +| `` | ## codersdk.NotificationsConfig diff --git a/enterprise/coderd/notifications.go b/enterprise/coderd/notifications.go index 3f3ea2b911026..ff8ed4b9e728c 100644 --- a/enterprise/coderd/notifications.go +++ b/enterprise/coderd/notifications.go @@ -42,7 +42,7 @@ func (api *API) updateNotificationTemplateMethod(rw http.ResponseWriter, r *http } var nm database.NullNotificationMethod - if err := nm.Scan(req.Method); err != nil || !nm.Valid || !nm.NotificationMethod.Valid() { + if err := nm.Scan(string(req.Method)); err != nil || !nm.Valid || !nm.NotificationMethod.Valid() { vals := database.AllNotificationMethodValues() acceptable := make([]string, len(vals)) for i, v := range vals { diff --git a/site/src/api/api.ts b/site/src/api/api.ts index aa5b139ee42ef..ddc781e0e1994 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -2020,6 +2020,24 @@ class ApiMethods { ); return res.data; }; + + getNotificationDispatchMethods = async () => { + const res = await this.axios.get( + `/api/v2/notifications/dispatch-methods`, + ); + return res.data; + }; + + updateNotificationTemplateMethod = async ( + templateId: string, + req: TypesGen.UpdateNotificationTemplateMethod, + ) => { + const res = await this.axios.put( + `/api/v2/notifications/templates/${templateId}/method`, + req, + ); + return res.data; + }; } // This is a hard coded CSRF token/cookie pair for local development. In prod, diff --git a/site/src/api/queries/notifications.ts b/site/src/api/queries/notifications.ts index 2c53d4e33a1d5..ba4292dfdacf6 100644 --- a/site/src/api/queries/notifications.ts +++ b/site/src/api/queries/notifications.ts @@ -2,6 +2,8 @@ import type { QueryClient, UseMutationOptions } from "react-query"; import { API } from "api/api"; import type { NotificationPreference, + NotificationTemplate, + UpdateNotificationTemplateMethod, UpdateUserNotificationPreferences, } from "api/typesGenerated"; @@ -53,3 +55,33 @@ export const systemNotificationTemplatesByGroup = () => { queryFn: () => API.getSystemNotificationTemplates(), }; }; + +export function selectTemplatesByGroup( + data: NotificationTemplate[], +): Record { + return data.reduce( + (acc, tpl) => { + if (!acc[tpl.group]) { + acc[tpl.group] = []; + } + acc[tpl.group].push(tpl); + return acc; + }, + {} as Record, + ); +} + +export const notificationDispatchMethods = () => { + return { + staleTime: Infinity, + queryKey: ["notifications", "dispatchMethods"], + queryFn: () => API.getNotificationDispatchMethods(), + }; +}; + +export const updateNotificationTemplateMethod = (templateId: string) => { + return { + mutationFn: (req: UpdateNotificationTemplateMethod) => + API.updateNotificationTemplateMethod(templateId, req), + }; +}; diff --git a/site/src/modules/notifications/utils.tsx b/site/src/modules/notifications/utils.tsx new file mode 100644 index 0000000000000..0ea048e5ea441 --- /dev/null +++ b/site/src/modules/notifications/utils.tsx @@ -0,0 +1,21 @@ +import EmailIcon from "@mui/icons-material/EmailOutlined"; +import DeploymentIcon from "@mui/icons-material/LanguageOutlined"; +import WebhookIcon from "@mui/icons-material/WebhookOutlined"; + +export const methodIcons: Record = { + "": DeploymentIcon, + smtp: EmailIcon, + webhook: WebhookIcon, +}; + +const methodLabels: Record = { + "": "Default", + smtp: "SMTP", + webhook: "Webhook", +}; + +export const methodLabel = (method: string, defaultMethod?: string) => { + return method === "" && defaultMethod + ? `${methodLabels[method]} - ${methodLabels[defaultMethod]}` + : methodLabels[method]; +}; diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index 3e08fa8d698d9..59773aef860f2 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -1,277 +1,142 @@ import type { Interpolation, Theme } from "@emotion/react"; -import EmailIcon from "@mui/icons-material/EmailOutlined"; -import WebhookIcon from "@mui/icons-material/LanguageOutlined"; import Card from "@mui/material/Card"; import Divider from "@mui/material/Divider"; import List from "@mui/material/List"; import ListItem from "@mui/material/ListItem"; -import ListItemIcon from "@mui/material/ListItemIcon"; import ListItemText, { listItemTextClasses } from "@mui/material/ListItemText"; -import Switch from "@mui/material/Switch"; -import TextField from "@mui/material/TextField"; import ToggleButton from "@mui/material/ToggleButton"; import ToggleButtonGroup from "@mui/material/ToggleButtonGroup"; -import type { FC } from "react"; -import { FormFields, FormSection, HorizontalForm } from "components/Form/Form"; -import { Section } from "pages/UserSettingsPage/Section"; -import Button from "@mui/material/Button"; +import Tooltip from "@mui/material/Tooltip"; +import { Fragment, type FC } from "react"; +import { useMutation, useQueries } from "react-query"; +import { + notificationDispatchMethods, + selectTemplatesByGroup, + systemNotificationTemplatesByGroup, + updateNotificationTemplateMethod, +} from "api/queries/notifications"; +import { Alert } from "components/Alert/Alert"; +import { displaySuccess } from "components/GlobalSnackbar/utils"; +import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; +import { methodIcons, methodLabel } from "modules/notifications/utils"; +import { Section } from "pages/UserSettingsPage/Section"; +import { useDeploySettings } from "../DeploySettingsLayout"; + +type MethodToggleGroupProps = { + templateId: string; + value: string; + available: readonly string[]; + defaultMethod: string; +}; + +const MethodToggleGroup: FC = ({ + value, + available, + templateId, + defaultMethod, +}) => { + const updateMethodMutation = useMutation( + updateNotificationTemplateMethod(templateId), + ); + const options = ["", ...available]; + + return ( + { + await updateMethodMutation.mutateAsync({ + method, + }); + displaySuccess("Notification method updated"); + }} + > + {options.map((method) => { + const Icon = methodIcons[method]; + const label = methodLabel(method, defaultMethod); + return ( + + + + + + ); + })} + + ); +}; export const NotificationsPage: FC = () => { + const { deploymentValues } = useDeploySettings(); + const [templatesByGroup, dispatchMethods] = useQueries({ + queries: [ + { + ...systemNotificationTemplatesByGroup(), + select: selectTemplatesByGroup, + }, + notificationDispatchMethods(), + ], + }); + const ready = templatesByGroup.data && dispatchMethods.data; + + const shouldDisplayWebhookWarning = + deploymentValues.config.notifications?.webhook.endpoint === "" && + dispatchMethods.data?.available.includes("webhook"); + return (
- - - - - - - - - - - - - + {ready ? ( + + {shouldDisplayWebhookWarning && ( + + Webhook method is enabled, but the endpoint is not configured. + + )} + {Object.entries(templatesByGroup.data).map(([group, templates]) => ( - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + {templates.map((tpl) => { + return ( + + + + + + + + ); + })} - - - + ))} + + ) : ( + + )}
); }; @@ -311,4 +176,9 @@ const styles = { fontSize: "inherit", }, }), + divider: { + "&:last-child": { + display: "none", + }, + }, } as Record>; diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx index 9bb93e1f3d933..88e6ac5ec20cf 100644 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -1,7 +1,4 @@ import type { Interpolation, Theme } from "@emotion/react"; -import EmailIcon from "@mui/icons-material/EmailOutlined"; -import DeploymentIcon from "@mui/icons-material/LanguageOutlined"; -import WebhookIcon from "@mui/icons-material/WebhookOutlined"; import Card from "@mui/material/Card"; import Divider from "@mui/material/Divider"; import List from "@mui/material/List"; @@ -13,18 +10,18 @@ import Tooltip from "@mui/material/Tooltip"; import { Fragment, type FC } from "react"; import { useMutation, useQueries, useQueryClient } from "react-query"; import { + notificationDispatchMethods, + selectTemplatesByGroup, systemNotificationTemplatesByGroup, updateUserNotificationPreferences, userNotificationPreferences, } from "api/queries/notifications"; -import type { - NotificationPreference, - NotificationTemplate, -} from "api/typesGenerated"; +import type { NotificationPreference } from "api/typesGenerated"; import { displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; import { useAuthenticated } from "contexts/auth/RequireAuth"; +import { methodIcons, methodLabel } from "modules/notifications/utils"; import { Section } from "../Section"; type PreferenceSwitchProps = { @@ -61,7 +58,7 @@ const PreferenceSwitch: FC = ({ export const NotificationsPage: FC = () => { const { user } = useAuthenticated(); - const [disabledPreferences, templatesByGroup] = useQueries({ + const [disabledPreferences, templatesByGroup, dispatchMethods] = useQueries({ queries: [ { ...userNotificationPreferences(user.id), @@ -71,8 +68,11 @@ export const NotificationsPage: FC = () => { ...systemNotificationTemplatesByGroup(), select: selectTemplatesByGroup, }, + notificationDispatchMethods(), ], }); + const ready = + disabledPreferences.data && templatesByGroup.data && dispatchMethods.data; return (
{ description="Configure notifications. Some may be disabled by the deployment administrator." layout="fluid" > - {templatesByGroup.data && disabledPreferences.data ? ( + {ready ? ( {Object.entries(templatesByGroup.data).map(([group, templates]) => { const allDisabled = templates.some((tpl) => { @@ -118,6 +118,12 @@ export const NotificationsPage: FC = () => { /> {templates.map((tmpl) => { + const Icon = methodIcons[tmpl.method]; + const label = methodLabel( + tmpl.method, + dispatchMethods.data.default, + ); + return ( @@ -141,25 +147,16 @@ export const NotificationsPage: FC = () => { css={styles.listItemText} primary={tmpl.name} /> - - {tmpl.method === "email" && ( - - - - )} - {tmpl.method === "webhook" && ( - - - - )} - {tmpl.method === "" && ( - - - - )} + + + + - + ); })} @@ -187,21 +184,6 @@ function selectDisabledPreferences(data: NotificationPreference[]) { ); } -function selectTemplatesByGroup( - data: NotificationTemplate[], -): Record { - return data.reduce( - (acc, tpl) => { - if (!acc[tpl.group]) { - acc[tpl.group] = []; - } - acc[tpl.group].push(tpl); - return acc; - }, - {} as Record, - ); -} - const styles = { listHeader: (theme) => ({ background: theme.palette.background.paper, @@ -226,4 +208,9 @@ const styles = { fontSize: "inherit", }, }), + divider: { + "&:last-child": { + display: "none", + }, + }, } as Record>; diff --git a/site/src/router.tsx b/site/src/router.tsx index 766866297045f..f1ee22d017311 100644 --- a/site/src/router.tsx +++ b/site/src/router.tsx @@ -265,6 +265,10 @@ const ProvisionerDaemonsHealthPage = lazy( const UserNotificationsPage = lazy( () => import("./pages/UserSettingsPage/NotificationsPage/NotificationsPage"), ); +const DeploymentNotificationsPage = lazy( + () => + import("./pages/DeploySettingsPage/NotificationsPage/NotificationsPage"), +); const RoutesWithSuspense = () => { return ( @@ -414,6 +418,10 @@ export const router = createBrowserRouter( } /> {groupsRouter()} } /> + } + /> }> From 1262f7b3ad76b6c0d36b31bd3002982fd4714476 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 5 Aug 2024 19:12:41 +0000 Subject: [PATCH 08/32] Fix types --- coderd/apidoc/docs.go | 21 +++++------------- coderd/apidoc/swagger.json | 17 +++++--------- coderd/notifications.go | 6 ++--- codersdk/notifications.go | 14 ++++++------ docs/api/notifications.md | 28 +++++++++++++++--------- docs/api/schemas.md | 26 ++++++++-------------- enterprise/coderd/notifications_test.go | 7 +++--- site/src/api/typesGenerated.ts | 8 +++---- site/src/modules/notifications/utils.tsx | 19 ++++++++++------ 9 files changed, 68 insertions(+), 78 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index f3c9db86028b0..f69d46b16e8b8 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -10382,11 +10382,11 @@ const docTemplate = `{ "available": { "type": "array", "items": { - "type": "string" + "$ref": "#/definitions/codersdk.NotificationTemplateMethod" } }, "default": { - "type": "string" + "$ref": "#/definitions/codersdk.NotificationTemplateMethod" } } }, @@ -10426,16 +10426,7 @@ const docTemplate = `{ "type": "string" }, "method": { - "enum": [ - "email", - "webhook", - "''" - ], - "allOf": [ - { - "$ref": "#/definitions/codersdk.NotificationTemplateMethod" - } - ] + "$ref": "#/definitions/codersdk.NotificationTemplateMethod" }, "name": { "type": "string" @@ -10448,14 +10439,14 @@ const docTemplate = `{ "codersdk.NotificationTemplateMethod": { "type": "string", "enum": [ - "email", + "smtp", "webhook", "" ], "x-enum-varnames": [ - "NotificationTemplateEmailMethod", + "NotificationTemplateSMTPMethod", "NotificationTemplateWebhookMethod", - "NotificationTemplateNotDefinedMethod" + "NotificationTemplateDefaultMethod" ] }, "codersdk.NotificationsConfig": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 68cd297715081..be1f969c87a13 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -9301,11 +9301,11 @@ "available": { "type": "array", "items": { - "type": "string" + "$ref": "#/definitions/codersdk.NotificationTemplateMethod" } }, "default": { - "type": "string" + "$ref": "#/definitions/codersdk.NotificationTemplateMethod" } } }, @@ -9345,12 +9345,7 @@ "type": "string" }, "method": { - "enum": ["email", "webhook", "''"], - "allOf": [ - { - "$ref": "#/definitions/codersdk.NotificationTemplateMethod" - } - ] + "$ref": "#/definitions/codersdk.NotificationTemplateMethod" }, "name": { "type": "string" @@ -9362,11 +9357,11 @@ }, "codersdk.NotificationTemplateMethod": { "type": "string", - "enum": ["email", "webhook", ""], + "enum": ["smtp", "webhook", ""], "x-enum-varnames": [ - "NotificationTemplateEmailMethod", + "NotificationTemplateSMTPMethod", "NotificationTemplateWebhookMethod", - "NotificationTemplateNotDefinedMethod" + "NotificationTemplateDefaultMethod" ] }, "codersdk.NotificationsConfig": { diff --git a/coderd/notifications.go b/coderd/notifications.go index 97f88beddc3cf..c7d28efc3c396 100644 --- a/coderd/notifications.go +++ b/coderd/notifications.go @@ -152,14 +152,14 @@ func (api *API) systemNotificationTemplates(rw http.ResponseWriter, r *http.Requ // @Success 200 {array} codersdk.NotificationMethodsResponse // @Router /notifications/dispatch-methods [get] func (api *API) notificationDispatchMethods(rw http.ResponseWriter, r *http.Request) { - var methods []string + var methods []codersdk.NotificationTemplateMethod for _, nm := range database.AllNotificationMethodValues() { - methods = append(methods, string(nm)) + methods = append(methods, codersdk.NotificationTemplateMethod(nm)) } httpapi.Write(r.Context(), rw, http.StatusOK, codersdk.NotificationMethodsResponse{ AvailableNotificationMethods: methods, - DefaultNotificationMethod: api.DeploymentValues.Notifications.Method.Value(), + DefaultNotificationMethod: codersdk.NotificationTemplateMethod(api.DeploymentValues.Notifications.Method.Value()), }) } diff --git a/codersdk/notifications.go b/codersdk/notifications.go index 03a430285920b..2bf63d86ee000 100644 --- a/codersdk/notifications.go +++ b/codersdk/notifications.go @@ -19,9 +19,9 @@ type NotificationsSettings struct { type NotificationTemplateMethod string const ( - NotificationTemplateEmailMethod NotificationTemplateMethod = "email" - NotificationTemplateWebhookMethod NotificationTemplateMethod = "webhook" - NotificationTemplateNotDefinedMethod NotificationTemplateMethod = "" + NotificationTemplateSMTPMethod NotificationTemplateMethod = "smtp" + NotificationTemplateWebhookMethod NotificationTemplateMethod = "webhook" + NotificationTemplateDefaultMethod NotificationTemplateMethod = "" ) type NotificationTemplate struct { @@ -31,13 +31,13 @@ type NotificationTemplate struct { BodyTemplate string `json:"body_template"` Actions string `json:"actions" format:""` Group string `json:"group"` - Method NotificationTemplateMethod `json:"method" enums:"email,webhook,''"` + Method NotificationTemplateMethod `json:"method"` Kind string `json:"kind"` } type NotificationMethodsResponse struct { - AvailableNotificationMethods []string `json:"available"` - DefaultNotificationMethod string `json:"default"` + AvailableNotificationMethods []NotificationTemplateMethod `json:"available"` + DefaultNotificationMethod NotificationTemplateMethod `json:"default"` } type NotificationPreference struct { @@ -201,7 +201,7 @@ func (c *Client) GetNotificationDispatchMethods(ctx context.Context) (Notificati } type UpdateNotificationTemplateMethod struct { - Method NotificationTemplateMethod `json:"method,omitempty" enums:"email,webhook" example:"webhook"` + Method NotificationTemplateMethod `json:"method,omitempty" example:"webhook"` } type UpdateUserNotificationPreferences struct { diff --git a/docs/api/notifications.md b/docs/api/notifications.md index c4a5d29138549..72eb7d0fc8cdd 100644 --- a/docs/api/notifications.md +++ b/docs/api/notifications.md @@ -20,8 +20,8 @@ curl -X GET http://coder-server:8080/api/v2/notifications/dispatch-methods \ ```json [ { - "available": ["string"], - "default": "string" + "available": ["smtp"], + "default": "smtp" } ] ``` @@ -36,11 +36,19 @@ curl -X GET http://coder-server:8080/api/v2/notifications/dispatch-methods \ Status Code **200** -| Name | Type | Required | Restrictions | Description | -| -------------- | ------ | -------- | ------------ | ----------- | -| `[array item]` | array | false | | | -| `» available` | array | false | | | -| `» default` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| -------------- | ------------------------------------------------------------------------------------ | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» available` | array | false | | | +| `» default` | [codersdk.NotificationTemplateMethod](schemas.md#codersdknotificationtemplatemethod) | false | | | + +#### Enumerated Values + +| Property | Value | +| --------- | --------- | +| `default` | `smtp` | +| `default` | `webhook` | +| `default` | `` | To perform this operation, you must be authenticated. [Learn more](authentication.md). @@ -147,7 +155,7 @@ curl -X GET http://coder-server:8080/api/v2/notifications/templates/system \ "group": "string", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "kind": "string", - "method": "email", + "method": "smtp", "name": "string", "title_template": "string" } @@ -180,9 +188,9 @@ Status Code **200** | Property | Value | | -------- | --------- | -| `method` | `email` | +| `method` | `smtp` | | `method` | `webhook` | -| `method` | `''` | +| `method` | `` | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 9bab357f15c3c..699e2a927dece 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -3145,17 +3145,17 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o ```json { - "available": ["string"], - "default": "string" + "available": ["smtp"], + "default": "smtp" } ``` ### Properties -| Name | Type | Required | Restrictions | Description | -| ----------- | --------------- | -------- | ------------ | ----------- | -| `available` | array of string | false | | | -| `default` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| ----------- | ----------------------------------------------------------------------------------- | -------- | ------------ | ----------- | +| `available` | array of [codersdk.NotificationTemplateMethod](#codersdknotificationtemplatemethod) | false | | | +| `default` | [codersdk.NotificationTemplateMethod](#codersdknotificationtemplatemethod) | false | | | ## codersdk.NotificationPreference @@ -3184,7 +3184,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "group": "string", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "kind": "string", - "method": "email", + "method": "smtp", "name": "string", "title_template": "string" } @@ -3203,18 +3203,10 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `name` | string | false | | | | `title_template` | string | false | | | -#### Enumerated Values - -| Property | Value | -| -------- | --------- | -| `method` | `email` | -| `method` | `webhook` | -| `method` | `''` | - ## codersdk.NotificationTemplateMethod ```json -"email" +"smtp" ``` ### Properties @@ -3223,7 +3215,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | Value | | --------- | -| `email` | +| `smtp` | | `webhook` | | `` | diff --git a/enterprise/coderd/notifications_test.go b/enterprise/coderd/notifications_test.go index 5546bec1dcb79..fe0509045bc6f 100644 --- a/enterprise/coderd/notifications_test.go +++ b/enterprise/coderd/notifications_test.go @@ -11,7 +11,6 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/coderdtest" - "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/codersdk" @@ -45,7 +44,7 @@ func TestUpdateNotificationTemplateMethod(t *testing.T) { api, _ := coderdenttest.New(t, createOpts(t)) var ( - method = string(database.NotificationMethodSmtp) + method = codersdk.NotificationTemplateSMTPMethod templateID = notifications.TemplateWorkspaceDeleted ) @@ -79,7 +78,7 @@ func TestUpdateNotificationTemplateMethod(t *testing.T) { anotherClient, _ := coderdtest.CreateAnotherUser(t, api, firstUser.OrganizationID) // When: calling the API as an unprivileged user. - err := anotherClient.UpdateNotificationTemplateMethod(ctx, notifications.TemplateWorkspaceDeleted, string(database.NotificationMethodWebhook)) + err := anotherClient.UpdateNotificationTemplateMethod(ctx, notifications.TemplateWorkspaceDeleted, codersdk.NotificationTemplateWebhookMethod) // Then: the request is denied because of insufficient permissions. var sdkError *codersdk.Error @@ -129,7 +128,7 @@ func TestUpdateNotificationTemplateMethod(t *testing.T) { api, _ := coderdenttest.New(t, createOpts(t)) var ( - method = string(database.NotificationMethodSmtp) + method = codersdk.NotificationTemplateSMTPMethod templateID = notifications.TemplateWorkspaceDeleted ) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 4e45ec64a8c4a..39ee0270af618 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -711,8 +711,8 @@ export interface MinimalUser { // From codersdk/notifications.go export interface NotificationMethodsResponse { - readonly available: readonly string[]; - readonly default: string; + readonly available: readonly NotificationTemplateMethod[]; + readonly default: NotificationTemplateMethod; } // From codersdk/notifications.go @@ -2200,10 +2200,10 @@ export const LoginTypes: LoginType[] = [ ]; // From codersdk/notifications.go -export type NotificationTemplateMethod = "" | "email" | "webhook"; +export type NotificationTemplateMethod = "" | "smtp" | "webhook"; export const NotificationTemplateMethods: NotificationTemplateMethod[] = [ "", - "email", + "smtp", "webhook", ]; diff --git a/site/src/modules/notifications/utils.tsx b/site/src/modules/notifications/utils.tsx index 0ea048e5ea441..f6d283f28fac5 100644 --- a/site/src/modules/notifications/utils.tsx +++ b/site/src/modules/notifications/utils.tsx @@ -1,20 +1,25 @@ import EmailIcon from "@mui/icons-material/EmailOutlined"; import DeploymentIcon from "@mui/icons-material/LanguageOutlined"; import WebhookIcon from "@mui/icons-material/WebhookOutlined"; +import type { NotificationTemplateMethod } from "api/typesGenerated"; -export const methodIcons: Record = { - "": DeploymentIcon, - smtp: EmailIcon, - webhook: WebhookIcon, -}; +export const methodIcons: Record = + { + "": DeploymentIcon, + smtp: EmailIcon, + webhook: WebhookIcon, + }; -const methodLabels: Record = { +const methodLabels: Record = { "": "Default", smtp: "SMTP", webhook: "Webhook", }; -export const methodLabel = (method: string, defaultMethod?: string) => { +export const methodLabel = ( + method: NotificationTemplateMethod, + defaultMethod?: NotificationTemplateMethod, +) => { return method === "" && defaultMethod ? `${methodLabels[method]} - ${methodLabels[defaultMethod]}` : methodLabels[method]; From e449e3dd44f10d12c859ef09103f191f7ed2a274 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 5 Aug 2024 19:21:13 +0000 Subject: [PATCH 09/32] Fix remaining type issues --- .../NotificationsPage/NotificationsPage.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index 59773aef860f2..7d2475338f94c 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -15,6 +15,7 @@ import { systemNotificationTemplatesByGroup, updateNotificationTemplateMethod, } from "api/queries/notifications"; +import type { NotificationTemplateMethod } from "api/typesGenerated"; import { Alert } from "components/Alert/Alert"; import { displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; @@ -26,8 +27,8 @@ import { useDeploySettings } from "../DeploySettingsLayout"; type MethodToggleGroupProps = { templateId: string; value: string; - available: readonly string[]; - defaultMethod: string; + available: readonly NotificationTemplateMethod[]; + defaultMethod: NotificationTemplateMethod; }; const MethodToggleGroup: FC = ({ @@ -39,7 +40,7 @@ const MethodToggleGroup: FC = ({ const updateMethodMutation = useMutation( updateNotificationTemplateMethod(templateId), ); - const options = ["", ...available]; + const options: NotificationTemplateMethod[] = ["", ...available]; return ( = ({ aria-label="Notification method" css={styles.toggleGroup} onChange={async (_, method) => { + // Retain the value if the user clicks the same button, ensuring + // at least one value remains selected. + if (method === value) { + return; + } + await updateMethodMutation.mutateAsync({ method, }); From 0154b943dde881d13aac6be7e5e115daf42cb857 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 5 Aug 2024 19:31:44 +0000 Subject: [PATCH 10/32] Experience improvements --- coderd/database/queries.sql.go | 1 + coderd/database/queries/notifications.sql | 3 +- site/src/api/queries/notifications.ts | 36 ++++++++++++++++--- .../NotificationsPage/NotificationsPage.tsx | 29 +++++++++------ .../NotificationsPage/NotificationsPage.tsx | 4 +-- 5 files changed, 55 insertions(+), 18 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index d8a6e3a1abb03..97d9158659c07 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3659,6 +3659,7 @@ const getNotificationTemplatesByKind = `-- name: GetNotificationTemplatesByKind SELECT id, name, title_template, body_template, actions, "group", method, kind FROM notification_templates WHERE kind = $1::notification_template_kind +ORDER BY notification_templates.name ASC ` func (q *sqlQuerier) GetNotificationTemplatesByKind(ctx context.Context, kind NotificationTemplateKind) ([]NotificationTemplate, error) { diff --git a/coderd/database/queries/notifications.sql b/coderd/database/queries/notifications.sql index f5b8601871ccc..a1636156f31a9 100644 --- a/coderd/database/queries/notifications.sql +++ b/coderd/database/queries/notifications.sql @@ -170,4 +170,5 @@ WHERE id = @id::uuid; -- name: GetNotificationTemplatesByKind :many SELECT * FROM notification_templates -WHERE kind = @kind::notification_template_kind; +WHERE kind = @kind::notification_template_kind +ORDER BY notification_templates.name ASC; diff --git a/site/src/api/queries/notifications.ts b/site/src/api/queries/notifications.ts index ba4292dfdacf6..ad8e6d8c6cf0c 100644 --- a/site/src/api/queries/notifications.ts +++ b/site/src/api/queries/notifications.ts @@ -49,9 +49,11 @@ export const updateUserNotificationPreferences = ( >; }; -export const systemNotificationTemplatesByGroup = () => { +const systemNotificationTemplatesKey = ["notifications", "templates", "system"]; + +export const systemNotificationTemplates = () => { return { - queryKey: ["notifications", "templates", "system"], + queryKey: systemNotificationTemplatesKey, queryFn: () => API.getSystemNotificationTemplates(), }; }; @@ -79,9 +81,35 @@ export const notificationDispatchMethods = () => { }; }; -export const updateNotificationTemplateMethod = (templateId: string) => { +export const updateNotificationTemplateMethod = ( + templateId: string, + queryClient: QueryClient, +) => { return { mutationFn: (req: UpdateNotificationTemplateMethod) => API.updateNotificationTemplateMethod(templateId, req), - }; + onMutate: (data) => { + const prevData = queryClient.getQueryData( + systemNotificationTemplatesKey, + ); + if (!prevData) { + return; + } + queryClient.setQueryData( + systemNotificationTemplatesKey, + prevData.map((tpl) => + tpl.id === templateId + ? { + ...tpl, + method: data.method, + } + : tpl, + ), + ); + }, + } satisfies UseMutationOptions< + void, + unknown, + UpdateNotificationTemplateMethod + >; }; diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index 7d2475338f94c..ce271e1eaf070 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -8,11 +8,11 @@ import ToggleButton from "@mui/material/ToggleButton"; import ToggleButtonGroup from "@mui/material/ToggleButtonGroup"; import Tooltip from "@mui/material/Tooltip"; import { Fragment, type FC } from "react"; -import { useMutation, useQueries } from "react-query"; +import { useMutation, useQueries, useQueryClient } from "react-query"; import { notificationDispatchMethods, selectTemplatesByGroup, - systemNotificationTemplatesByGroup, + systemNotificationTemplates, updateNotificationTemplateMethod, } from "api/queries/notifications"; import type { NotificationTemplateMethod } from "api/typesGenerated"; @@ -37,8 +37,9 @@ const MethodToggleGroup: FC = ({ templateId, defaultMethod, }) => { + const queryClient = useQueryClient(); const updateMethodMutation = useMutation( - updateNotificationTemplateMethod(templateId), + updateNotificationTemplateMethod(templateId, queryClient), ); const options: NotificationTemplateMethod[] = ["", ...available]; @@ -50,12 +51,6 @@ const MethodToggleGroup: FC = ({ aria-label="Notification method" css={styles.toggleGroup} onChange={async (_, method) => { - // Retain the value if the user clicks the same button, ensuring - // at least one value remains selected. - if (method === value) { - return; - } - await updateMethodMutation.mutateAsync({ method, }); @@ -67,7 +62,19 @@ const MethodToggleGroup: FC = ({ const label = methodLabel(method, defaultMethod); return ( - + { + // Retain the value if the user clicks the same button, ensuring + // at least one value remains selected. + if (method === value) { + e.preventDefault(); + e.stopPropagation(); + return; + } + }} + > @@ -82,7 +89,7 @@ export const NotificationsPage: FC = () => { const [templatesByGroup, dispatchMethods] = useQueries({ queries: [ { - ...systemNotificationTemplatesByGroup(), + ...systemNotificationTemplates(), select: selectTemplatesByGroup, }, notificationDispatchMethods(), diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx index 88e6ac5ec20cf..c3364dcdbfa7a 100644 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -12,7 +12,7 @@ import { useMutation, useQueries, useQueryClient } from "react-query"; import { notificationDispatchMethods, selectTemplatesByGroup, - systemNotificationTemplatesByGroup, + systemNotificationTemplates, updateUserNotificationPreferences, userNotificationPreferences, } from "api/queries/notifications"; @@ -65,7 +65,7 @@ export const NotificationsPage: FC = () => { select: selectDisabledPreferences, }, { - ...systemNotificationTemplatesByGroup(), + ...systemNotificationTemplates(), select: selectTemplatesByGroup, }, notificationDispatchMethods(), From aedf15908f9cae3fd4270888a5902cc6a4b8c7ca Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 5 Aug 2024 19:47:11 +0000 Subject: [PATCH 11/32] Fix validation --- codersdk/notifications.go | 13 +++++++++++ enterprise/coderd/notifications.go | 17 ++++++++++---- enterprise/coderd/notifications_test.go | 31 +++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/codersdk/notifications.go b/codersdk/notifications.go index 2bf63d86ee000..daa69b9195e4b 100644 --- a/codersdk/notifications.go +++ b/codersdk/notifications.go @@ -24,6 +24,19 @@ const ( NotificationTemplateDefaultMethod NotificationTemplateMethod = "" ) +func (m NotificationTemplateMethod) Validate() error { + switch m { + case NotificationTemplateSMTPMethod: + return nil + case NotificationTemplateWebhookMethod: + return nil + case NotificationTemplateDefaultMethod: + return nil + default: + return xerrors.Errorf("unknown notification template method: %q", m) + } +} + type NotificationTemplate struct { ID uuid.UUID `json:"id" format:"uuid"` Name string `json:"name"` diff --git a/enterprise/coderd/notifications.go b/enterprise/coderd/notifications.go index ff8ed4b9e728c..cf770961411ae 100644 --- a/enterprise/coderd/notifications.go +++ b/enterprise/coderd/notifications.go @@ -41,8 +41,7 @@ func (api *API) updateNotificationTemplateMethod(rw http.ResponseWriter, r *http return } - var nm database.NullNotificationMethod - if err := nm.Scan(string(req.Method)); err != nil || !nm.Valid || !nm.NotificationMethod.Valid() { + if err := req.Method.Validate(); err != nil { vals := database.AllNotificationMethodValues() acceptable := make([]string, len(vals)) for i, v := range vals { @@ -63,7 +62,12 @@ func (api *API) updateNotificationTemplateMethod(rw http.ResponseWriter, r *http return } - if template.Method == nm { + var prevMethod string + if err := template.Method.Scan(&prevMethod); err != nil { + httpapi.InternalServerError(rw, err) + return + } + if codersdk.NotificationTemplateMethod(prevMethod) == req.Method { httpapi.Write(ctx, rw, http.StatusNotModified, codersdk.Response{ Message: "Notification template method unchanged.", }) @@ -76,8 +80,11 @@ func (api *API) updateNotificationTemplateMethod(rw http.ResponseWriter, r *http err := api.Database.InTx(func(tx database.Store) error { var err error template, err = api.Database.UpdateNotificationTemplateMethodByID(r.Context(), database.UpdateNotificationTemplateMethodByIDParams{ - ID: template.ID, - Method: nm, + ID: template.ID, + Method: database.NullNotificationMethod{ + Valid: true, + NotificationMethod: database.NotificationMethod(req.Method), + }, }) if err != nil { return xerrors.Errorf("failed to update notification template ID: %w", err) diff --git a/enterprise/coderd/notifications_test.go b/enterprise/coderd/notifications_test.go index fe0509045bc6f..0323decb9bd48 100644 --- a/enterprise/coderd/notifications_test.go +++ b/enterprise/coderd/notifications_test.go @@ -64,6 +64,37 @@ func TestUpdateNotificationTemplateMethod(t *testing.T) { require.Equal(t, method, template.Method) }) + t.Run("Default deployment-wide permission", func(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres; it relies on read from and writing to the notification_templates table") + } + + ctx := testutil.Context(t, testutil.WaitSuperLong) + api, _ := coderdenttest.New(t, createOpts(t)) + + var ( + method = codersdk.NotificationTemplateDefaultMethod + templateID = notifications.TemplateWorkspaceDeleted + ) + + // Given: a template whose method is initially empty (i.e. deferring to the global method value). + template, err := getTemplateByID(t, ctx, api, templateID) + require.NoError(t, err) + require.NotNil(t, template) + require.Empty(t, template.Method) + + // When: calling the API to update the method. + require.NoError(t, api.UpdateNotificationTemplateMethod(ctx, notifications.TemplateWorkspaceDeleted, method), "initial request to set the method failed") + + // Then: the method should be set. + template, err = getTemplateByID(t, ctx, api, templateID) + require.NoError(t, err) + require.NotNil(t, template) + require.Equal(t, method, template.Method) + }) + t.Run("Insufficient permissions", func(t *testing.T) { t.Parallel() From 1a2efaed685159ebc36f1d62d8d22dcc6f757631 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 6 Aug 2024 11:24:40 +0000 Subject: [PATCH 12/32] Remove BE changes --- coderd/apidoc/docs.go | 19 ++------ coderd/apidoc/swagger.json | 15 ++----- coderd/database/queries.sql.go | 1 - coderd/database/queries/notifications.sql | 3 +- coderd/notifications.go | 8 ++-- codersdk/notifications.go | 45 +++++-------------- docs/api/notifications.md | 54 ++++++++--------------- docs/api/schemas.md | 50 +++++++-------------- enterprise/coderd/notifications.go | 17 +++---- enterprise/coderd/notifications_test.go | 38 ++-------------- 10 files changed, 68 insertions(+), 182 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index f69d46b16e8b8..962fccae0a4ea 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -10382,11 +10382,11 @@ const docTemplate = `{ "available": { "type": "array", "items": { - "$ref": "#/definitions/codersdk.NotificationTemplateMethod" + "type": "string" } }, "default": { - "$ref": "#/definitions/codersdk.NotificationTemplateMethod" + "type": "string" } } }, @@ -10426,7 +10426,7 @@ const docTemplate = `{ "type": "string" }, "method": { - "$ref": "#/definitions/codersdk.NotificationTemplateMethod" + "type": "string" }, "name": { "type": "string" @@ -10436,19 +10436,6 @@ const docTemplate = `{ } } }, - "codersdk.NotificationTemplateMethod": { - "type": "string", - "enum": [ - "smtp", - "webhook", - "" - ], - "x-enum-varnames": [ - "NotificationTemplateSMTPMethod", - "NotificationTemplateWebhookMethod", - "NotificationTemplateDefaultMethod" - ] - }, "codersdk.NotificationsConfig": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index be1f969c87a13..35b8b82a21888 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -9301,11 +9301,11 @@ "available": { "type": "array", "items": { - "$ref": "#/definitions/codersdk.NotificationTemplateMethod" + "type": "string" } }, "default": { - "$ref": "#/definitions/codersdk.NotificationTemplateMethod" + "type": "string" } } }, @@ -9345,7 +9345,7 @@ "type": "string" }, "method": { - "$ref": "#/definitions/codersdk.NotificationTemplateMethod" + "type": "string" }, "name": { "type": "string" @@ -9355,15 +9355,6 @@ } } }, - "codersdk.NotificationTemplateMethod": { - "type": "string", - "enum": ["smtp", "webhook", ""], - "x-enum-varnames": [ - "NotificationTemplateSMTPMethod", - "NotificationTemplateWebhookMethod", - "NotificationTemplateDefaultMethod" - ] - }, "codersdk.NotificationsConfig": { "type": "object", "properties": { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 97d9158659c07..d8a6e3a1abb03 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3659,7 +3659,6 @@ const getNotificationTemplatesByKind = `-- name: GetNotificationTemplatesByKind SELECT id, name, title_template, body_template, actions, "group", method, kind FROM notification_templates WHERE kind = $1::notification_template_kind -ORDER BY notification_templates.name ASC ` func (q *sqlQuerier) GetNotificationTemplatesByKind(ctx context.Context, kind NotificationTemplateKind) ([]NotificationTemplate, error) { diff --git a/coderd/database/queries/notifications.sql b/coderd/database/queries/notifications.sql index a1636156f31a9..f5b8601871ccc 100644 --- a/coderd/database/queries/notifications.sql +++ b/coderd/database/queries/notifications.sql @@ -170,5 +170,4 @@ WHERE id = @id::uuid; -- name: GetNotificationTemplatesByKind :many SELECT * FROM notification_templates -WHERE kind = @kind::notification_template_kind -ORDER BY notification_templates.name ASC; +WHERE kind = @kind::notification_template_kind; diff --git a/coderd/notifications.go b/coderd/notifications.go index c7d28efc3c396..bdf71f99cab98 100644 --- a/coderd/notifications.go +++ b/coderd/notifications.go @@ -152,14 +152,14 @@ func (api *API) systemNotificationTemplates(rw http.ResponseWriter, r *http.Requ // @Success 200 {array} codersdk.NotificationMethodsResponse // @Router /notifications/dispatch-methods [get] func (api *API) notificationDispatchMethods(rw http.ResponseWriter, r *http.Request) { - var methods []codersdk.NotificationTemplateMethod + var methods []string for _, nm := range database.AllNotificationMethodValues() { - methods = append(methods, codersdk.NotificationTemplateMethod(nm)) + methods = append(methods, string(nm)) } httpapi.Write(r.Context(), rw, http.StatusOK, codersdk.NotificationMethodsResponse{ AvailableNotificationMethods: methods, - DefaultNotificationMethod: codersdk.NotificationTemplateMethod(api.DeploymentValues.Notifications.Method.Value()), + DefaultNotificationMethod: api.DeploymentValues.Notifications.Method.Value(), }) } @@ -277,7 +277,7 @@ func convertNotificationTemplates(in []database.NotificationTemplate) (out []cod BodyTemplate: tmpl.BodyTemplate, Actions: string(tmpl.Actions), Group: tmpl.Group.String, - Method: codersdk.NotificationTemplateMethod(tmpl.Method.NotificationMethod), + Method: string(tmpl.Method.NotificationMethod), Kind: string(tmpl.Kind), }) } diff --git a/codersdk/notifications.go b/codersdk/notifications.go index daa69b9195e4b..92870b4dd2b95 100644 --- a/codersdk/notifications.go +++ b/codersdk/notifications.go @@ -16,41 +16,20 @@ type NotificationsSettings struct { NotifierPaused bool `json:"notifier_paused"` } -type NotificationTemplateMethod string - -const ( - NotificationTemplateSMTPMethod NotificationTemplateMethod = "smtp" - NotificationTemplateWebhookMethod NotificationTemplateMethod = "webhook" - NotificationTemplateDefaultMethod NotificationTemplateMethod = "" -) - -func (m NotificationTemplateMethod) Validate() error { - switch m { - case NotificationTemplateSMTPMethod: - return nil - case NotificationTemplateWebhookMethod: - return nil - case NotificationTemplateDefaultMethod: - return nil - default: - return xerrors.Errorf("unknown notification template method: %q", m) - } -} - type NotificationTemplate struct { - ID uuid.UUID `json:"id" format:"uuid"` - Name string `json:"name"` - TitleTemplate string `json:"title_template"` - BodyTemplate string `json:"body_template"` - Actions string `json:"actions" format:""` - Group string `json:"group"` - Method NotificationTemplateMethod `json:"method"` - Kind string `json:"kind"` + ID uuid.UUID `json:"id" format:"uuid"` + Name string `json:"name"` + TitleTemplate string `json:"title_template"` + BodyTemplate string `json:"body_template"` + Actions string `json:"actions" format:""` + Group string `json:"group"` + Method string `json:"method"` + Kind string `json:"kind"` } type NotificationMethodsResponse struct { - AvailableNotificationMethods []NotificationTemplateMethod `json:"available"` - DefaultNotificationMethod NotificationTemplateMethod `json:"default"` + AvailableNotificationMethods []string `json:"available"` + DefaultNotificationMethod string `json:"default"` } type NotificationPreference struct { @@ -94,7 +73,7 @@ func (c *Client) PutNotificationsSettings(ctx context.Context, settings Notifica // UpdateNotificationTemplateMethod modifies a notification template to use a specific notification method, overriding // the method set in the deployment configuration. -func (c *Client) UpdateNotificationTemplateMethod(ctx context.Context, notificationTemplateID uuid.UUID, method NotificationTemplateMethod) error { +func (c *Client) UpdateNotificationTemplateMethod(ctx context.Context, notificationTemplateID uuid.UUID, method string) error { res, err := c.Request(ctx, http.MethodPut, fmt.Sprintf("/api/v2/notifications/templates/%s/method", notificationTemplateID), UpdateNotificationTemplateMethod{Method: method}, @@ -214,7 +193,7 @@ func (c *Client) GetNotificationDispatchMethods(ctx context.Context) (Notificati } type UpdateNotificationTemplateMethod struct { - Method NotificationTemplateMethod `json:"method,omitempty" example:"webhook"` + Method string `json:"method,omitempty" example:"webhook"` } type UpdateUserNotificationPreferences struct { diff --git a/docs/api/notifications.md b/docs/api/notifications.md index 72eb7d0fc8cdd..528153ebd103b 100644 --- a/docs/api/notifications.md +++ b/docs/api/notifications.md @@ -20,8 +20,8 @@ curl -X GET http://coder-server:8080/api/v2/notifications/dispatch-methods \ ```json [ { - "available": ["smtp"], - "default": "smtp" + "available": ["string"], + "default": "string" } ] ``` @@ -36,19 +36,11 @@ curl -X GET http://coder-server:8080/api/v2/notifications/dispatch-methods \ Status Code **200** -| Name | Type | Required | Restrictions | Description | -| -------------- | ------------------------------------------------------------------------------------ | -------- | ------------ | ----------- | -| `[array item]` | array | false | | | -| `» available` | array | false | | | -| `» default` | [codersdk.NotificationTemplateMethod](schemas.md#codersdknotificationtemplatemethod) | false | | | - -#### Enumerated Values - -| Property | Value | -| --------- | --------- | -| `default` | `smtp` | -| `default` | `webhook` | -| `default` | `` | +| Name | Type | Required | Restrictions | Description | +| -------------- | ------ | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» available` | array | false | | | +| `» default` | string | false | | | To perform this operation, you must be authenticated. [Learn more](authentication.md). @@ -155,7 +147,7 @@ curl -X GET http://coder-server:8080/api/v2/notifications/templates/system \ "group": "string", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "kind": "string", - "method": "smtp", + "method": "string", "name": "string", "title_template": "string" } @@ -172,25 +164,17 @@ curl -X GET http://coder-server:8080/api/v2/notifications/templates/system \ Status Code **200** -| Name | Type | Required | Restrictions | Description | -| ------------------ | ------------------------------------------------------------------------------------ | -------- | ------------ | ----------- | -| `[array item]` | array | false | | | -| `» actions` | string | false | | | -| `» body_template` | string | false | | | -| `» group` | string | false | | | -| `» id` | string(uuid) | false | | | -| `» kind` | string | false | | | -| `» method` | [codersdk.NotificationTemplateMethod](schemas.md#codersdknotificationtemplatemethod) | false | | | -| `» name` | string | false | | | -| `» title_template` | string | false | | | - -#### Enumerated Values - -| Property | Value | -| -------- | --------- | -| `method` | `smtp` | -| `method` | `webhook` | -| `method` | `` | +| Name | Type | Required | Restrictions | Description | +| ------------------ | ------------ | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» actions` | string | false | | | +| `» body_template` | string | false | | | +| `» group` | string | false | | | +| `» id` | string(uuid) | false | | | +| `» kind` | string | false | | | +| `» method` | string | false | | | +| `» name` | string | false | | | +| `» title_template` | string | false | | | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 699e2a927dece..7406d135112f1 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -3145,17 +3145,17 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o ```json { - "available": ["smtp"], - "default": "smtp" + "available": ["string"], + "default": "string" } ``` ### Properties -| Name | Type | Required | Restrictions | Description | -| ----------- | ----------------------------------------------------------------------------------- | -------- | ------------ | ----------- | -| `available` | array of [codersdk.NotificationTemplateMethod](#codersdknotificationtemplatemethod) | false | | | -| `default` | [codersdk.NotificationTemplateMethod](#codersdknotificationtemplatemethod) | false | | | +| Name | Type | Required | Restrictions | Description | +| ----------- | --------------- | -------- | ------------ | ----------- | +| `available` | array of string | false | | | +| `default` | string | false | | | ## codersdk.NotificationPreference @@ -3184,7 +3184,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "group": "string", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "kind": "string", - "method": "smtp", + "method": "string", "name": "string", "title_template": "string" } @@ -3192,32 +3192,16 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o ### Properties -| Name | Type | Required | Restrictions | Description | -| ---------------- | -------------------------------------------------------------------------- | -------- | ------------ | ----------- | -| `actions` | string | false | | | -| `body_template` | string | false | | | -| `group` | string | false | | | -| `id` | string | false | | | -| `kind` | string | false | | | -| `method` | [codersdk.NotificationTemplateMethod](#codersdknotificationtemplatemethod) | false | | | -| `name` | string | false | | | -| `title_template` | string | false | | | - -## codersdk.NotificationTemplateMethod - -```json -"smtp" -``` - -### Properties - -#### Enumerated Values - -| Value | -| --------- | -| `smtp` | -| `webhook` | -| `` | +| Name | Type | Required | Restrictions | Description | +| ---------------- | ------ | -------- | ------------ | ----------- | +| `actions` | string | false | | | +| `body_template` | string | false | | | +| `group` | string | false | | | +| `id` | string | false | | | +| `kind` | string | false | | | +| `method` | string | false | | | +| `name` | string | false | | | +| `title_template` | string | false | | | ## codersdk.NotificationsConfig diff --git a/enterprise/coderd/notifications.go b/enterprise/coderd/notifications.go index cf770961411ae..3f3ea2b911026 100644 --- a/enterprise/coderd/notifications.go +++ b/enterprise/coderd/notifications.go @@ -41,7 +41,8 @@ func (api *API) updateNotificationTemplateMethod(rw http.ResponseWriter, r *http return } - if err := req.Method.Validate(); err != nil { + var nm database.NullNotificationMethod + if err := nm.Scan(req.Method); err != nil || !nm.Valid || !nm.NotificationMethod.Valid() { vals := database.AllNotificationMethodValues() acceptable := make([]string, len(vals)) for i, v := range vals { @@ -62,12 +63,7 @@ func (api *API) updateNotificationTemplateMethod(rw http.ResponseWriter, r *http return } - var prevMethod string - if err := template.Method.Scan(&prevMethod); err != nil { - httpapi.InternalServerError(rw, err) - return - } - if codersdk.NotificationTemplateMethod(prevMethod) == req.Method { + if template.Method == nm { httpapi.Write(ctx, rw, http.StatusNotModified, codersdk.Response{ Message: "Notification template method unchanged.", }) @@ -80,11 +76,8 @@ func (api *API) updateNotificationTemplateMethod(rw http.ResponseWriter, r *http err := api.Database.InTx(func(tx database.Store) error { var err error template, err = api.Database.UpdateNotificationTemplateMethodByID(r.Context(), database.UpdateNotificationTemplateMethodByIDParams{ - ID: template.ID, - Method: database.NullNotificationMethod{ - Valid: true, - NotificationMethod: database.NotificationMethod(req.Method), - }, + ID: template.ID, + Method: nm, }) if err != nil { return xerrors.Errorf("failed to update notification template ID: %w", err) diff --git a/enterprise/coderd/notifications_test.go b/enterprise/coderd/notifications_test.go index 0323decb9bd48..5546bec1dcb79 100644 --- a/enterprise/coderd/notifications_test.go +++ b/enterprise/coderd/notifications_test.go @@ -11,6 +11,7 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/codersdk" @@ -44,38 +45,7 @@ func TestUpdateNotificationTemplateMethod(t *testing.T) { api, _ := coderdenttest.New(t, createOpts(t)) var ( - method = codersdk.NotificationTemplateSMTPMethod - templateID = notifications.TemplateWorkspaceDeleted - ) - - // Given: a template whose method is initially empty (i.e. deferring to the global method value). - template, err := getTemplateByID(t, ctx, api, templateID) - require.NoError(t, err) - require.NotNil(t, template) - require.Empty(t, template.Method) - - // When: calling the API to update the method. - require.NoError(t, api.UpdateNotificationTemplateMethod(ctx, notifications.TemplateWorkspaceDeleted, method), "initial request to set the method failed") - - // Then: the method should be set. - template, err = getTemplateByID(t, ctx, api, templateID) - require.NoError(t, err) - require.NotNil(t, template) - require.Equal(t, method, template.Method) - }) - - t.Run("Default deployment-wide permission", func(t *testing.T) { - t.Parallel() - - if !dbtestutil.WillUsePostgres() { - t.Skip("This test requires postgres; it relies on read from and writing to the notification_templates table") - } - - ctx := testutil.Context(t, testutil.WaitSuperLong) - api, _ := coderdenttest.New(t, createOpts(t)) - - var ( - method = codersdk.NotificationTemplateDefaultMethod + method = string(database.NotificationMethodSmtp) templateID = notifications.TemplateWorkspaceDeleted ) @@ -109,7 +79,7 @@ func TestUpdateNotificationTemplateMethod(t *testing.T) { anotherClient, _ := coderdtest.CreateAnotherUser(t, api, firstUser.OrganizationID) // When: calling the API as an unprivileged user. - err := anotherClient.UpdateNotificationTemplateMethod(ctx, notifications.TemplateWorkspaceDeleted, codersdk.NotificationTemplateWebhookMethod) + err := anotherClient.UpdateNotificationTemplateMethod(ctx, notifications.TemplateWorkspaceDeleted, string(database.NotificationMethodWebhook)) // Then: the request is denied because of insufficient permissions. var sdkError *codersdk.Error @@ -159,7 +129,7 @@ func TestUpdateNotificationTemplateMethod(t *testing.T) { api, _ := coderdenttest.New(t, createOpts(t)) var ( - method = codersdk.NotificationTemplateSMTPMethod + method = string(database.NotificationMethodSmtp) templateID = notifications.TemplateWorkspaceDeleted ) From a1f363ccce0313b1bf1bcb7f91f56c1b2d20bdee Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 6 Aug 2024 11:26:35 +0000 Subject: [PATCH 13/32] Fix FE types --- site/src/api/typesGenerated.ts | 16 ++++------------ site/src/modules/notifications/utils.tsx | 19 +++++++------------ .../NotificationsPage/NotificationsPage.tsx | 7 +++---- 3 files changed, 14 insertions(+), 28 deletions(-) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 39ee0270af618..5c2dc816fea1e 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -711,8 +711,8 @@ export interface MinimalUser { // From codersdk/notifications.go export interface NotificationMethodsResponse { - readonly available: readonly NotificationTemplateMethod[]; - readonly default: NotificationTemplateMethod; + readonly available: readonly string[]; + readonly default: string; } // From codersdk/notifications.go @@ -730,7 +730,7 @@ export interface NotificationTemplate { readonly body_template: string; readonly actions: string; readonly group: string; - readonly method: NotificationTemplateMethod; + readonly method: string; readonly kind: string; } @@ -1474,7 +1474,7 @@ export interface UpdateCheckResponse { // From codersdk/notifications.go export interface UpdateNotificationTemplateMethod { - readonly method?: NotificationTemplateMethod; + readonly method?: string; } // From codersdk/organizations.go @@ -2199,14 +2199,6 @@ export const LoginTypes: LoginType[] = [ "token", ]; -// From codersdk/notifications.go -export type NotificationTemplateMethod = "" | "smtp" | "webhook"; -export const NotificationTemplateMethods: NotificationTemplateMethod[] = [ - "", - "smtp", - "webhook", -]; - // From codersdk/oauth2.go export type OAuth2ProviderGrantType = "authorization_code" | "refresh_token"; export const OAuth2ProviderGrantTypes: OAuth2ProviderGrantType[] = [ diff --git a/site/src/modules/notifications/utils.tsx b/site/src/modules/notifications/utils.tsx index f6d283f28fac5..0ea048e5ea441 100644 --- a/site/src/modules/notifications/utils.tsx +++ b/site/src/modules/notifications/utils.tsx @@ -1,25 +1,20 @@ import EmailIcon from "@mui/icons-material/EmailOutlined"; import DeploymentIcon from "@mui/icons-material/LanguageOutlined"; import WebhookIcon from "@mui/icons-material/WebhookOutlined"; -import type { NotificationTemplateMethod } from "api/typesGenerated"; -export const methodIcons: Record = - { - "": DeploymentIcon, - smtp: EmailIcon, - webhook: WebhookIcon, - }; +export const methodIcons: Record = { + "": DeploymentIcon, + smtp: EmailIcon, + webhook: WebhookIcon, +}; -const methodLabels: Record = { +const methodLabels: Record = { "": "Default", smtp: "SMTP", webhook: "Webhook", }; -export const methodLabel = ( - method: NotificationTemplateMethod, - defaultMethod?: NotificationTemplateMethod, -) => { +export const methodLabel = (method: string, defaultMethod?: string) => { return method === "" && defaultMethod ? `${methodLabels[method]} - ${methodLabels[defaultMethod]}` : methodLabels[method]; diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index ce271e1eaf070..21d39fe9e34e1 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -15,7 +15,6 @@ import { systemNotificationTemplates, updateNotificationTemplateMethod, } from "api/queries/notifications"; -import type { NotificationTemplateMethod } from "api/typesGenerated"; import { Alert } from "components/Alert/Alert"; import { displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; @@ -27,8 +26,8 @@ import { useDeploySettings } from "../DeploySettingsLayout"; type MethodToggleGroupProps = { templateId: string; value: string; - available: readonly NotificationTemplateMethod[]; - defaultMethod: NotificationTemplateMethod; + available: readonly string[]; + defaultMethod: string; }; const MethodToggleGroup: FC = ({ @@ -41,7 +40,7 @@ const MethodToggleGroup: FC = ({ const updateMethodMutation = useMutation( updateNotificationTemplateMethod(templateId, queryClient), ); - const options: NotificationTemplateMethod[] = ["", ...available]; + const options = ["", ...available]; return ( Date: Tue, 6 Aug 2024 11:32:54 +0000 Subject: [PATCH 14/32] Fix notifications permissions --- .../NotificationsPage/NotificationsPage.tsx | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx index c3364dcdbfa7a..82693401f5410 100644 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -16,7 +16,10 @@ import { updateUserNotificationPreferences, userNotificationPreferences, } from "api/queries/notifications"; -import type { NotificationPreference } from "api/typesGenerated"; +import type { + NotificationPreference, + NotificationTemplate, +} from "api/typesGenerated"; import { displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; @@ -57,7 +60,7 @@ const PreferenceSwitch: FC = ({ }; export const NotificationsPage: FC = () => { - const { user } = useAuthenticated(); + const { user, permissions } = useAuthenticated(); const [disabledPreferences, templatesByGroup, dispatchMethods] = useQueries({ queries: [ { @@ -66,7 +69,15 @@ export const NotificationsPage: FC = () => { }, { ...systemNotificationTemplates(), - select: selectTemplatesByGroup, + select: (data: NotificationTemplate[]) => { + const groups = selectTemplatesByGroup(data); + return permissions.viewDeploymentValues + ? groups + : { + // Members only have access to the "Workspace Notifications" group + ["Workspace Notifications"]: groups["Workspace Notifications"], + }; + }, }, notificationDispatchMethods(), ], From 1ff09738fe6a46063d3f7648b303f91e3a488bf5 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 6 Aug 2024 11:49:19 +0000 Subject: [PATCH 15/32] Display webhook info --- .../NotificationsPage/NotificationsPage.tsx | 50 +++++++++++++++---- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index 21d39fe9e34e1..344af79d46d6a 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -1,4 +1,6 @@ import type { Interpolation, Theme } from "@emotion/react"; +import AlertTitle from "@mui/material/AlertTitle"; +import Button from "@mui/material/Button"; import Card from "@mui/material/Card"; import Divider from "@mui/material/Divider"; import List from "@mui/material/List"; @@ -15,10 +17,11 @@ import { systemNotificationTemplates, updateNotificationTemplateMethod, } from "api/queries/notifications"; -import { Alert } from "components/Alert/Alert"; +import { Alert, AlertDetail } from "components/Alert/Alert"; import { displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; +import { useClipboard } from "hooks"; import { methodIcons, methodLabel } from "modules/notifications/utils"; import { Section } from "pages/UserSettingsPage/Section"; import { useDeploySettings } from "../DeploySettingsLayout"; @@ -96,9 +99,9 @@ export const NotificationsPage: FC = () => { }); const ready = templatesByGroup.data && dispatchMethods.data; - const shouldDisplayWebhookWarning = - deploymentValues.config.notifications?.webhook.endpoint === "" && - dispatchMethods.data?.available.includes("webhook"); + const isUsingWebhook = dispatchMethods.data?.available.includes("webhook"); + const webhookEndpoint = + deploymentValues.config.notifications?.webhook.endpoint; return (
{ > {ready ? ( - {shouldDisplayWebhookWarning && ( - - Webhook method is enabled, but the endpoint is not configured. - - )} + {isUsingWebhook && + (webhookEndpoint ? ( + + ) : ( + + Webhook method is enabled, but the endpoint is not configured. + + ))} {Object.entries(templatesByGroup.data).map(([group, templates]) => ( { export default NotificationsPage; +type WebhookInfoProps = { + endpoint: string; +}; + +const WebhookInfo = ({ endpoint }: WebhookInfoProps) => { + const clipboard = useClipboard({ textToCopy: endpoint }); + + return ( + + {clipboard.showCopiedSuccess ? "Copied!" : "Copy"} + + } + > + Webhook Endpoint + {endpoint} + + ); +}; + const styles = { listHeader: (theme) => ({ background: theme.palette.background.paper, From 7cc7bdb64b38e75a3da2c5b3859b6ac94a3e0479 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 6 Aug 2024 13:32:12 +0000 Subject: [PATCH 16/32] Add tests to the notifications page --- site/src/@types/storybook.d.ts | 13 +- site/src/api/queries/notifications.ts | 15 +- site/src/api/queries/users.ts | 4 +- .../DeploySettingsLayout.tsx | 2 +- .../NotificationsPage.stories.tsx | 69 ++++++++ .../NotificationsPage/NotificationsPage.tsx | 2 +- .../NotificationsPage.stories.tsx | 78 +++++++++ .../NotificationsPage.test.tsx | 152 ------------------ .../NotificationsPage/NotificationsPage.tsx | 2 +- site/src/testHelpers/entities.ts | 128 +++++++++++++++ site/src/testHelpers/storybook.tsx | 49 ++++++ 11 files changed, 354 insertions(+), 160 deletions(-) create mode 100644 site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx create mode 100644 site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.stories.tsx delete mode 100644 site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.test.tsx diff --git a/site/src/@types/storybook.d.ts b/site/src/@types/storybook.d.ts index 778bf53d0a0b1..31ab2f32fed6b 100644 --- a/site/src/@types/storybook.d.ts +++ b/site/src/@types/storybook.d.ts @@ -1,6 +1,13 @@ import * as _storybook_types from "@storybook/react"; import type { QueryKey } from "react-query"; -import type { Experiments, FeatureName } from "api/typesGenerated"; +import type { + Experiments, + FeatureName, + SerpentOption, + User, + DeploymentValues, +} from "api/typesGenerated"; +import type { Permissions } from "contexts/auth/permissions"; declare module "@storybook/react" { type WebSocketEvent = @@ -11,5 +18,9 @@ declare module "@storybook/react" { experiments?: Experiments; queries?: { key: QueryKey; data: unknown }[]; webSocket?: WebSocketEvent[]; + user?: User; + permissions?: Partial; + deploymentValues?: DeploymentValues; + deploymentOptions?: SerpentOption[]; } } diff --git a/site/src/api/queries/notifications.ts b/site/src/api/queries/notifications.ts index ad8e6d8c6cf0c..11218292e8521 100644 --- a/site/src/api/queries/notifications.ts +++ b/site/src/api/queries/notifications.ts @@ -7,7 +7,7 @@ import type { UpdateUserNotificationPreferences, } from "api/typesGenerated"; -const userNotificationPreferencesKey = (userId: string) => [ +export const userNotificationPreferencesKey = (userId: string) => [ "users", userId, "notifications", @@ -49,7 +49,11 @@ export const updateUserNotificationPreferences = ( >; }; -const systemNotificationTemplatesKey = ["notifications", "templates", "system"]; +export const systemNotificationTemplatesKey = [ + "notifications", + "templates", + "system", +]; export const systemNotificationTemplates = () => { return { @@ -73,10 +77,15 @@ export function selectTemplatesByGroup( ); } +export const notificationDispatchMethodsKey = [ + "notifications", + "dispatchMethods", +]; + export const notificationDispatchMethods = () => { return { staleTime: Infinity, - queryKey: ["notifications", "dispatchMethods"], + queryKey: notificationDispatchMethodsKey, queryFn: () => API.getNotificationDispatchMethods(), }; }; diff --git a/site/src/api/queries/users.ts b/site/src/api/queries/users.ts index 8417dade576c8..700449b41ff7c 100644 --- a/site/src/api/queries/users.ts +++ b/site/src/api/queries/users.ts @@ -141,10 +141,12 @@ export function apiKey(): UseQueryOptions { }; } +export const hasFirstUserKey = ["hasFirstUser"]; + export const hasFirstUser = (userMetadata: MetadataState) => { return cachedQuery({ metadata: userMetadata, - queryKey: ["hasFirstUser"], + queryKey: hasFirstUserKey, queryFn: API.hasFirstUser, }); }; diff --git a/site/src/pages/DeploySettingsPage/DeploySettingsLayout.tsx b/site/src/pages/DeploySettingsPage/DeploySettingsLayout.tsx index 14b77ff550ca1..1504bce7c0606 100644 --- a/site/src/pages/DeploySettingsPage/DeploySettingsLayout.tsx +++ b/site/src/pages/DeploySettingsPage/DeploySettingsLayout.tsx @@ -25,7 +25,7 @@ export const useDeploySettings = (): DeploySettingsContextValue => { const context = useContext(DeploySettingsContext); if (!context) { throw new Error( - "useDeploySettings should be used inside of DeploySettingsLayout", + "useDeploySettings should be used inside of DeploySettingsContext or DeploySettingsLayout", ); } return context; diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx new file mode 100644 index 0000000000000..3c49510b4e584 --- /dev/null +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx @@ -0,0 +1,69 @@ +import type { Meta, StoryObj } from "@storybook/react"; +import { spyOn, userEvent, within } from "@storybook/test"; +import { API } from "api/api"; +import { + notificationDispatchMethodsKey, + systemNotificationTemplatesKey, +} from "api/queries/notifications"; +import type { DeploymentValues } from "api/typesGenerated"; +import { + MockNotificationMethodsResponse, + MockNotificationTemplates, + MockUser, +} from "testHelpers/entities"; +import { + withAuthProvider, + withDashboardProvider, + withDeploySettings, + withGlobalSnackbar, +} from "testHelpers/storybook"; +import { NotificationsPage } from "./NotificationsPage"; + +const meta: Meta = { + title: "pages/DeploymentSettings/NotificationsPage", + component: NotificationsPage, + parameters: { + experiments: ["notifications"], + queries: [ + { key: systemNotificationTemplatesKey, data: MockNotificationTemplates }, + { + key: notificationDispatchMethodsKey, + data: MockNotificationMethodsResponse, + }, + ], + user: MockUser, + permissions: { viewDeploymentValues: true }, + deploymentValues: { + notifications: { + webhook: { + endpoint: "https://example.com", + }, + }, + } as DeploymentValues, + }, + decorators: [ + withGlobalSnackbar, + withAuthProvider, + withDashboardProvider, + withDeploySettings, + ], +}; + +export default meta; + +type Story = StoryObj; + +export const Default: Story = {}; + +export const Toggle: Story = { + play: async ({ canvasElement }) => { + spyOn(API, "updateNotificationTemplateMethod").mockResolvedValue(); + const user = userEvent.setup(); + const canvas = within(canvasElement); + const option = await canvas.findByText("Workspace Marked as Dormant"); + const toggleButton = within(option.closest("li")!).getByRole("button", { + name: "Webhook", + }); + await user.click(toggleButton); + }, +}; diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index 344af79d46d6a..c30df2d66087c 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -101,7 +101,7 @@ export const NotificationsPage: FC = () => { const isUsingWebhook = dispatchMethods.data?.available.includes("webhook"); const webhookEndpoint = - deploymentValues.config.notifications?.webhook.endpoint; + deploymentValues?.config.notifications?.webhook.endpoint; return (
= { + title: "pages/UserSettingsPage/NotificationsPage", + component: NotificationsPage, + parameters: { + experiments: ["notifications"], + queries: [ + { + key: userNotificationPreferencesKey(MockUser.id), + data: MockNotificationPreferences, + }, + { + key: systemNotificationTemplatesKey, + data: MockNotificationTemplates, + }, + { + key: notificationDispatchMethodsKey, + data: MockNotificationMethodsResponse, + }, + ], + user: MockUser, + permissions: { viewDeploymentValues: true }, + }, + decorators: [withGlobalSnackbar, withAuthProvider, withDashboardProvider], +}; + +export default meta; +type Story = StoryObj; + +export const Default: Story = {}; + +export const ToggleGroup: Story = { + play: async ({ canvasElement }) => { + spyOn(API, "putUserNotificationPreferences").mockResolvedValue([]); + const user = userEvent.setup(); + const canvas = within(canvasElement); + const groupLabel = await canvas.findByLabelText("Workspace Events"); + await user.click(groupLabel); + }, +}; + +export const ToggleNotification: Story = { + play: async ({ canvasElement }) => { + spyOn(API, "putUserNotificationPreferences").mockResolvedValue([]); + const user = userEvent.setup(); + const canvas = within(canvasElement); + const notificationLabel = await canvas.findByLabelText( + "Workspace Marked as Dormant", + ); + await user.click(notificationLabel); + }, +}; + +export const NonAdmin: Story = { + parameters: { + permissions: { viewDeploymentValues: false }, + }, +}; diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.test.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.test.tsx deleted file mode 100644 index a5d795f70ed28..0000000000000 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.test.tsx +++ /dev/null @@ -1,152 +0,0 @@ -import { screen } from "@testing-library/react"; -import userEvent from "@testing-library/user-event"; -import { http, HttpResponse } from "msw"; -import type { - Experiments, - NotificationPreference, - NotificationTemplate, - UpdateUserNotificationPreferences, -} from "api/typesGenerated"; -import { renderWithAuth } from "testHelpers/renderHelpers"; -import { server } from "testHelpers/server"; -import NotificationsPage from "./NotificationsPage"; - -test("can enable and disable notifications", async () => { - server.use( - http.get("/api/v2/experiments", () => - HttpResponse.json(["notifications"] as Experiments), - ), - http.get("/api/v2/users/:userId/notifications/preferences", () => - HttpResponse.json(null), - ), - http.get("/api/v2/notifications/templates/system", () => - HttpResponse.json(notificationsTemplateSystemRes), - ), - http.put< - { userId: string }, - UpdateUserNotificationPreferences, - NotificationPreference[] - >( - "/api/v2/users/:userId/notifications/preferences", - async ({ request }) => { - const body = await request.json(); - const res: NotificationPreference[] = Object.entries(body).map( - ([id, disabled]) => ({ - disabled, - id, - updated_at: new Date().toISOString(), - }), - ); - return HttpResponse.json(res); - }, - ), - ); - renderWithAuth(); - const user = userEvent.setup(); - const workspaceGroupTemplates = notificationsTemplateSystemRes.filter( - (t) => t.group === "Workspace Events", - ); - - // Test notification groups - const workspaceGroupSwitch = await screen.findByLabelText("Workspace Events"); - await user.click(workspaceGroupSwitch); - await screen.findByText("Notification preferences updated"); - expect(workspaceGroupSwitch).not.toBeChecked(); - for (const template of workspaceGroupTemplates) { - const templateSwitch = screen.getByLabelText(template.name); - expect(templateSwitch).not.toBeChecked(); - } - - await user.click(workspaceGroupSwitch); - await screen.findByText("Notification preferences updated"); - expect(workspaceGroupSwitch).toBeChecked(); - for (const template of workspaceGroupTemplates) { - const templateSwitch = screen.getByLabelText(template.name); - expect(templateSwitch).toBeChecked(); - } - - // Test individual notifications - const workspaceDeletedSwitch = screen.getByLabelText("Workspace Deleted"); - await user.click(workspaceDeletedSwitch); - await screen.findByText("Notification preferences updated"); - expect(workspaceDeletedSwitch).not.toBeChecked(); - - await user.click(workspaceDeletedSwitch); - await screen.findByText("Notification preferences updated"); - expect(workspaceDeletedSwitch).toBeChecked(); -}); - -const notificationsTemplateSystemRes: NotificationTemplate[] = [ - { - id: "f517da0b-cdc9-410f-ab89-a86107c420ed", - name: "Workspace Deleted", - title_template: 'Workspace "{{.Labels.name}}" deleted', - body_template: - 'Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** was deleted.\nThe specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} ({{ .Labels.initiator }}){{end}}**".', - actions: - '[{"url": "{{ base_url }}/workspaces", "label": "View workspaces"}, {"url": "{{ base_url }}/templates", "label": "View templates"}]', - group: "Workspace Events", - method: "", - kind: "system", - }, - { - id: "381df2a9-c0c0-4749-420f-80a9280c66f9", - name: "Workspace Autobuild Failed", - title_template: 'Workspace "{{.Labels.name}}" autobuild failed', - body_template: - 'Hi {{.UserName}}\nAutomatic build of your workspace **{{.Labels.name}}** failed.\nThe specified reason was "**{{.Labels.reason}}**".', - actions: - '[{"url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}", "label": "View workspace"}]', - group: "Workspace Events", - method: "", - kind: "system", - }, - { - id: "c34a0c09-0704-4cac-bd1c-0c0146811c2b", - name: "Workspace updated automatically", - title_template: 'Workspace "{{.Labels.name}}" updated automatically', - body_template: - "Hi {{.UserName}}\nYour workspace **{{.Labels.name}}** has been updated automatically to the latest template version ({{.Labels.template_version_name}}).", - actions: - '[{"url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}", "label": "View workspace"}]', - group: "Workspace Events", - method: "", - kind: "system", - }, - { - id: "0ea69165-ec14-4314-91f1-69566ac3c5a0", - name: "Workspace Marked as Dormant", - title_template: 'Workspace "{{.Labels.name}}" marked as dormant', - body_template: - "Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** has been marked as [**dormant**](https://coder.com/docs/templates/schedule#dormancy-threshold-enterprise) because of {{.Labels.reason}}.\nDormant workspaces are [automatically deleted](https://coder.com/docs/templates/schedule#dormancy-auto-deletion-enterprise) after {{.Labels.timeTilDormant}} of inactivity.\nTo prevent deletion, use your workspace with the link below.", - actions: - '[{"url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}", "label": "View workspace"}]', - group: "Workspace Events", - method: "", - kind: "system", - }, - { - id: "51ce2fdf-c9ca-4be1-8d70-628674f9bc42", - name: "Workspace Marked for Deletion", - title_template: 'Workspace "{{.Labels.name}}" marked for deletion', - body_template: - "Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** has been marked for **deletion** after {{.Labels.timeTilDormant}} of [dormancy](https://coder.com/docs/templates/schedule#dormancy-auto-deletion-enterprise) because of {{.Labels.reason}}.\nTo prevent deletion, use your workspace with the link below.", - actions: - '[{"url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}", "label": "View workspace"}]', - group: "Workspace Events", - method: "", - kind: "system", - }, - { - id: "4e19c0ac-94e1-4532-9515-d1801aa283b2", - name: "User account created", - title_template: 'User account "{{.Labels.created_account_name}}" created', - body_template: - "Hi {{.UserName}},\nNew user account **{{.Labels.created_account_name}}** has been created.", - actions: - '[{"url": "{{ base_url }}/deployment/users?filter=status%3Aactive", "label": "View accounts"}]', - group: "User Events", - method: "", - kind: "system", - }, -]; diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx index 82693401f5410..95fee4da9abd8 100644 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -75,7 +75,7 @@ export const NotificationsPage: FC = () => { ? groups : { // Members only have access to the "Workspace Notifications" group - ["Workspace Notifications"]: groups["Workspace Notifications"], + ["Workspace Events"]: groups["Workspace Events"], }; }, }, diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 1c12784a3c84f..e4671fe3860d2 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -3655,3 +3655,131 @@ export const MockOAuth2ProviderAppSecrets: TypesGen.OAuth2ProviderAppSecret[] = client_secret_truncated: "foo", }, ]; + +export const MockNotificationPreferences: TypesGen.NotificationPreference[] = [ + { + id: "f44d9314-ad03-4bc8-95d0-5cad491da6b6", + disabled: false, + updated_at: "2024-08-06T11:58:37.755053Z", + }, + { + id: "381df2a9-c0c0-4749-420f-80a9280c66f9", + disabled: true, + updated_at: "2024-08-06T11:58:37.755053Z", + }, + { + id: "f517da0b-cdc9-410f-ab89-a86107c420ed", + disabled: false, + updated_at: "2024-08-06T11:58:37.755053Z", + }, + { + id: "c34a0c09-0704-4cac-bd1c-0c0146811c2b", + disabled: false, + updated_at: "2024-08-06T11:58:37.755053Z", + }, + { + id: "0ea69165-ec14-4314-91f1-69566ac3c5a0", + disabled: false, + updated_at: "2024-08-06T11:58:37.755053Z", + }, + { + id: "51ce2fdf-c9ca-4be1-8d70-628674f9bc42", + disabled: false, + updated_at: "2024-08-06T11:58:37.755053Z", + }, + { + id: "4e19c0ac-94e1-4532-9515-d1801aa283b2", + disabled: true, + updated_at: "2024-08-06T11:58:37.755053Z", + }, +]; + +export const MockNotificationTemplates: TypesGen.NotificationTemplate[] = [ + { + id: "381df2a9-c0c0-4749-420f-80a9280c66f9", + name: "Workspace Autobuild Failed", + title_template: 'Workspace "{{.Labels.name}}" autobuild failed', + body_template: + 'Hi {{.UserName}}\nAutomatic build of your workspace **{{.Labels.name}}** failed.\nThe specified reason was "**{{.Labels.reason}}**".', + actions: + '[{"url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}", "label": "View workspace"}]', + group: "Workspace Events", + method: "webhook", + kind: "system", + }, + { + id: "f517da0b-cdc9-410f-ab89-a86107c420ed", + name: "Workspace Deleted", + title_template: 'Workspace "{{.Labels.name}}" deleted', + body_template: + 'Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** was deleted.\nThe specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} ({{ .Labels.initiator }}){{end}}**".', + actions: + '[{"url": "{{ base_url }}/workspaces", "label": "View workspaces"}, {"url": "{{ base_url }}/templates", "label": "View templates"}]', + group: "Workspace Events", + method: "smtp", + kind: "system", + }, + { + id: "f44d9314-ad03-4bc8-95d0-5cad491da6b6", + name: "User account deleted", + title_template: 'User account "{{.Labels.deleted_account_name}}" deleted', + body_template: + "Hi {{.UserName}},\n\nUser account **{{.Labels.deleted_account_name}}** has been deleted.", + actions: + '[{"url": "{{ base_url }}/deployment/users?filter=status%3Aactive", "label": "View accounts"}]', + group: "User Events", + method: "", + kind: "system", + }, + { + id: "4e19c0ac-94e1-4532-9515-d1801aa283b2", + name: "User account created", + title_template: 'User account "{{.Labels.created_account_name}}" created', + body_template: + "Hi {{.UserName}},\n\nNew user account **{{.Labels.created_account_name}}** has been created.", + actions: + '[{"url": "{{ base_url }}/deployment/users?filter=status%3Aactive", "label": "View accounts"}]', + group: "User Events", + method: "", + kind: "system", + }, + { + id: "0ea69165-ec14-4314-91f1-69566ac3c5a0", + name: "Workspace Marked as Dormant", + title_template: 'Workspace "{{.Labels.name}}" marked as dormant', + body_template: + "Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** has been marked as [**dormant**](https://coder.com/docs/templates/schedule#dormancy-threshold-enterprise) because of {{.Labels.reason}}.\nDormant workspaces are [automatically deleted](https://coder.com/docs/templates/schedule#dormancy-auto-deletion-enterprise) after {{.Labels.timeTilDormant}} of inactivity.\nTo prevent deletion, use your workspace with the link below.", + actions: + '[{"url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}", "label": "View workspace"}]', + group: "Workspace Events", + method: "smtp", + kind: "system", + }, + { + id: "c34a0c09-0704-4cac-bd1c-0c0146811c2b", + name: "Workspace updated automatically", + title_template: 'Workspace "{{.Labels.name}}" updated automatically', + body_template: + "Hi {{.UserName}}\nYour workspace **{{.Labels.name}}** has been updated automatically to the latest template version ({{.Labels.template_version_name}}).", + actions: + '[{"url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}", "label": "View workspace"}]', + group: "Workspace Events", + method: "smtp", + kind: "system", + }, + { + id: "51ce2fdf-c9ca-4be1-8d70-628674f9bc42", + name: "Workspace Marked for Deletion", + title_template: 'Workspace "{{.Labels.name}}" marked for deletion', + body_template: + "Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** has been marked for **deletion** after {{.Labels.timeTilDormant}} of [dormancy](https://coder.com/docs/templates/schedule#dormancy-auto-deletion-enterprise) because of {{.Labels.reason}}.\nTo prevent deletion, use your workspace with the link below.", + actions: + '[{"url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}", "label": "View workspace"}]', + group: "Workspace Events", + method: "webhook", + kind: "system", + }, +]; + +export const MockNotificationMethodsResponse: TypesGen.NotificationMethodsResponse = + { available: ["smtp", "webhook"], default: "smtp" }; diff --git a/site/src/testHelpers/storybook.tsx b/site/src/testHelpers/storybook.tsx index 30fbc28d0fccc..31f7523356f6f 100644 --- a/site/src/testHelpers/storybook.tsx +++ b/site/src/testHelpers/storybook.tsx @@ -1,8 +1,15 @@ import type { StoryContext } from "@storybook/react"; import type { FC } from "react"; +import { useQueryClient } from "react-query"; import { withDefaultFeatures } from "api/api"; +import { getAuthorizationKey } from "api/queries/authCheck"; +import { hasFirstUserKey, meKey } from "api/queries/users"; import type { Entitlements } from "api/typesGenerated"; +import { GlobalSnackbar } from "components/GlobalSnackbar/GlobalSnackbar"; +import { AuthProvider } from "contexts/auth/AuthProvider"; +import { permissionsToCheck } from "contexts/auth/permissions"; import { DashboardContext } from "modules/dashboard/DashboardProvider"; +import { DeploySettingsContext } from "pages/DeploySettingsPage/DeploySettingsLayout"; import { MockAppearanceConfig, MockDefaultOrganization, @@ -87,3 +94,45 @@ export const withDesktopViewport = (Story: FC) => ( ); + +export const withAuthProvider = (Story: FC, { parameters }: StoryContext) => { + if (!parameters.user) { + console.warn("You forgot to add `parameters.user` to your story"); + } + // eslint-disable-next-line react-hooks/rules-of-hooks -- decorators are components + const queryClient = useQueryClient(); + queryClient.setQueryData(meKey, parameters.user); + queryClient.setQueryData(hasFirstUserKey, true); + queryClient.setQueryData( + getAuthorizationKey({ checks: permissionsToCheck }), + parameters.permissions ?? {}, + ); + + return ( + + + + ); +}; + +export const withGlobalSnackbar = (Story: FC) => ( + <> + + + +); + +export const withDeploySettings = (Story: FC, { parameters }: StoryContext) => { + return ( + + + + ); +}; From 4956409ac1fc0eec15c395c948c57159ca229dd3 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 6 Aug 2024 16:01:26 +0000 Subject: [PATCH 17/32] Remove unecessary migration --- .../migrations/000239_update_notification_templates.down.sql | 0 .../migrations/000239_update_notification_templates.up.sql | 5 ----- 2 files changed, 5 deletions(-) delete mode 100644 coderd/database/migrations/000239_update_notification_templates.down.sql delete mode 100644 coderd/database/migrations/000239_update_notification_templates.up.sql diff --git a/coderd/database/migrations/000239_update_notification_templates.down.sql b/coderd/database/migrations/000239_update_notification_templates.down.sql deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/coderd/database/migrations/000239_update_notification_templates.up.sql b/coderd/database/migrations/000239_update_notification_templates.up.sql deleted file mode 100644 index 73b9e48c65fb4..0000000000000 --- a/coderd/database/migrations/000239_update_notification_templates.up.sql +++ /dev/null @@ -1,5 +0,0 @@ -UPDATE notification_templates -SET - "group" = 'User Events' -WHERE - id = '4e19c0ac-94e1-4532-9515-d1801aa283b2'; From 1c622424e2bfba735629d118dfc04fcd39388ca4 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 6 Aug 2024 16:19:38 +0000 Subject: [PATCH 18/32] Don't show deployment wide method --- site/src/modules/notifications/utils.tsx | 26 +++++++++------ .../NotificationsPage/NotificationsPage.tsx | 32 ++++++++++++------- .../NotificationsPage/NotificationsPage.tsx | 14 +++++--- 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/site/src/modules/notifications/utils.tsx b/site/src/modules/notifications/utils.tsx index 0ea048e5ea441..1511d40aa8400 100644 --- a/site/src/modules/notifications/utils.tsx +++ b/site/src/modules/notifications/utils.tsx @@ -1,21 +1,29 @@ import EmailIcon from "@mui/icons-material/EmailOutlined"; -import DeploymentIcon from "@mui/icons-material/LanguageOutlined"; import WebhookIcon from "@mui/icons-material/WebhookOutlined"; -export const methodIcons: Record = { - "": DeploymentIcon, +// TODO: This should be provided by the auto generated types from codersdk +const notificationMethods = ["smtp", "webhook"] as const; + +export type NotificationMethod = (typeof notificationMethods)[number]; + +export const methodIcons: Record = { smtp: EmailIcon, webhook: WebhookIcon, }; -const methodLabels: Record = { - "": "Default", +export const methodLabels: Record = { smtp: "SMTP", webhook: "Webhook", }; -export const methodLabel = (method: string, defaultMethod?: string) => { - return method === "" && defaultMethod - ? `${methodLabels[method]} - ${methodLabels[defaultMethod]}` - : methodLabels[method]; +export const castNotificationMethod = (value: string) => { + if (notificationMethods.includes(value as NotificationMethod)) { + return value as NotificationMethod; + } + + throw new Error( + `Invalid notification method: ${value}. Accepted values: ${notificationMethods.join( + ", ", + )}`, + ); }; diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index c30df2d66087c..b504b3aeb0d02 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -22,28 +22,30 @@ import { displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; import { useClipboard } from "hooks"; -import { methodIcons, methodLabel } from "modules/notifications/utils"; +import { + castNotificationMethod, + methodIcons, + methodLabels, + type NotificationMethod, +} from "modules/notifications/utils"; import { Section } from "pages/UserSettingsPage/Section"; import { useDeploySettings } from "../DeploySettingsLayout"; type MethodToggleGroupProps = { templateId: string; - value: string; - available: readonly string[]; - defaultMethod: string; + options: NotificationMethod[]; + value: NotificationMethod; }; const MethodToggleGroup: FC = ({ value, - available, + options, templateId, - defaultMethod, }) => { const queryClient = useQueryClient(); const updateMethodMutation = useMutation( updateNotificationTemplateMethod(templateId, queryClient), ); - const options = ["", ...available]; return ( = ({ > {options.map((method) => { const Icon = methodIcons[method]; - const label = methodLabel(method, defaultMethod); + const label = methodLabels[method]; return ( { return (
@@ -131,6 +133,13 @@ export const NotificationsPage: FC = () => { {templates.map((tpl) => { + const value = castNotificationMethod( + tpl.method || dispatchMethods.data.default, + ); + const options = dispatchMethods.data.available.map( + castNotificationMethod, + ); + return ( @@ -139,10 +148,9 @@ export const NotificationsPage: FC = () => { primary={tpl.name} /> diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx index 95fee4da9abd8..db0919bca9192 100644 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -24,7 +24,11 @@ import { displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; import { useAuthenticated } from "contexts/auth/RequireAuth"; -import { methodIcons, methodLabel } from "modules/notifications/utils"; +import { + castNotificationMethod, + methodIcons, + methodLabels, +} from "modules/notifications/utils"; import { Section } from "../Section"; type PreferenceSwitchProps = { @@ -129,11 +133,11 @@ export const NotificationsPage: FC = () => { /> {templates.map((tmpl) => { - const Icon = methodIcons[tmpl.method]; - const label = methodLabel( - tmpl.method, - dispatchMethods.data.default, + const method = castNotificationMethod( + tmpl.method || dispatchMethods.data.default, ); + const Icon = methodIcons[method]; + const label = methodLabels[method]; return ( From a020619aeb476b4853b0b9163c7aa9d3a83a1c82 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 6 Aug 2024 16:21:03 +0000 Subject: [PATCH 19/32] Fix templates sorting --- coderd/database/queries.sql.go | 1 + coderd/database/queries/notifications.sql | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index d8a6e3a1abb03..d06eca07f3e4e 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3659,6 +3659,7 @@ const getNotificationTemplatesByKind = `-- name: GetNotificationTemplatesByKind SELECT id, name, title_template, body_template, actions, "group", method, kind FROM notification_templates WHERE kind = $1::notification_template_kind +ORDER BY name ASC ` func (q *sqlQuerier) GetNotificationTemplatesByKind(ctx context.Context, kind NotificationTemplateKind) ([]NotificationTemplate, error) { diff --git a/coderd/database/queries/notifications.sql b/coderd/database/queries/notifications.sql index f5b8601871ccc..3500a9c413068 100644 --- a/coderd/database/queries/notifications.sql +++ b/coderd/database/queries/notifications.sql @@ -170,4 +170,5 @@ WHERE id = @id::uuid; -- name: GetNotificationTemplatesByKind :many SELECT * FROM notification_templates -WHERE kind = @kind::notification_template_kind; +WHERE kind = @kind::notification_template_kind +ORDER BY name ASC; From 0efc40d753cb1cf1202bff25ae2d3d01985dbb61 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 6 Aug 2024 16:47:26 +0000 Subject: [PATCH 20/32] Add nav tabs --- .../NotificationsPage/NotificationsPage.tsx | 181 ++++++++++-------- site/src/pages/UserSettingsPage/Section.tsx | 1 + 2 files changed, 101 insertions(+), 81 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index b504b3aeb0d02..36b8346a2a4b9 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -1,6 +1,4 @@ import type { Interpolation, Theme } from "@emotion/react"; -import AlertTitle from "@mui/material/AlertTitle"; -import Button from "@mui/material/Button"; import Card from "@mui/material/Card"; import Divider from "@mui/material/Divider"; import List from "@mui/material/List"; @@ -11,17 +9,19 @@ import ToggleButtonGroup from "@mui/material/ToggleButtonGroup"; import Tooltip from "@mui/material/Tooltip"; import { Fragment, type FC } from "react"; import { useMutation, useQueries, useQueryClient } from "react-query"; +import { useSearchParams } from "react-router-dom"; import { notificationDispatchMethods, selectTemplatesByGroup, systemNotificationTemplates, updateNotificationTemplateMethod, } from "api/queries/notifications"; -import { Alert, AlertDetail } from "components/Alert/Alert"; +import type { NotificationsConfig } from "api/typesGenerated"; +import { Alert } from "components/Alert/Alert"; import { displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; -import { useClipboard } from "hooks"; +import { TabLink, Tabs, TabsList } from "components/Tabs/Tabs"; import { castNotificationMethod, methodIcons, @@ -29,7 +29,9 @@ import { type NotificationMethod, } from "modules/notifications/utils"; import { Section } from "pages/UserSettingsPage/Section"; +import { deploymentGroupHasParent } from "utils/deployOptions"; import { useDeploySettings } from "../DeploySettingsLayout"; +import OptionsTable from "../OptionsTable"; type MethodToggleGroupProps = { templateId: string; @@ -89,6 +91,7 @@ const MethodToggleGroup: FC = ({ }; export const NotificationsPage: FC = () => { + const [searchParams] = useSearchParams(); const { deploymentValues } = useDeploySettings(); const [templatesByGroup, dispatchMethods] = useQueries({ queries: [ @@ -99,11 +102,9 @@ export const NotificationsPage: FC = () => { notificationDispatchMethods(), ], }); - const ready = templatesByGroup.data && dispatchMethods.data; - - const isUsingWebhook = dispatchMethods.data?.available.includes("webhook"); - const webhookEndpoint = - deploymentValues?.config.notifications?.webhook.endpoint; + const ready = + templatesByGroup.data && dispatchMethods.data && deploymentValues; + const tab = searchParams.get("tab") || "events"; return (
{ description="Control delivery methods for notifications. Settings applied to this deployment." layout="fluid" > - {ready ? ( - - {isUsingWebhook && - (webhookEndpoint ? ( - - ) : ( - - Webhook method is enabled, but the endpoint is not configured. - - ))} - {Object.entries(templatesByGroup.data).map(([group, templates]) => ( - - - - - + + + + Events + + + Settings + + + - {templates.map((tpl) => { - const value = castNotificationMethod( - tpl.method || dispatchMethods.data.default, - ); - const options = dispatchMethods.data.available.map( - castNotificationMethod, - ); - - return ( - - - - - - - - ); - })} - - - ))} - - ) : ( - - )} +
+ {ready ? ( + tab === "events" ? ( + + ) : ( + + deploymentGroupHasParent(o.group, "Notifications"), + )} + /> + ) + ) : ( + + )} +
); }; -export default NotificationsPage; - -type WebhookInfoProps = { - endpoint: string; +type EventsViewProps = { + defaultMethod: NotificationMethod; + availableMethods: NotificationMethod[]; + notificationsConfig?: NotificationsConfig; + templatesByGroup: ReturnType; }; -const WebhookInfo = ({ endpoint }: WebhookInfoProps) => { - const clipboard = useClipboard({ textToCopy: endpoint }); +const EventsView: FC = ({ + defaultMethod, + availableMethods, + notificationsConfig, + templatesByGroup, +}) => { + const isUsingWebhook = availableMethods.includes("webhook"); + const webhookEndpoint = notificationsConfig?.webhook.endpoint; return ( - + {isUsingWebhook && !webhookEndpoint && ( + + Webhook method is enabled, but the endpoint is not configured. + + )} + {Object.entries(templatesByGroup).map(([group, templates]) => ( + - {clipboard.showCopiedSuccess ? "Copied!" : "Copy"} - - } - > - Webhook Endpoint - {endpoint} - + + + + + + {templates.map((tpl) => { + const value = castNotificationMethod(tpl.method || defaultMethod); + + return ( + + + + + + + + ); + })} + + + ))} + ); }; +export default NotificationsPage; + const styles = { + content: { paddingTop: 24 }, listHeader: (theme) => ({ background: theme.palette.background.paper, borderBottom: `1px solid ${theme.palette.divider}`, diff --git a/site/src/pages/UserSettingsPage/Section.tsx b/site/src/pages/UserSettingsPage/Section.tsx index 64fa369ef68ce..0ce892540b5c5 100644 --- a/site/src/pages/UserSettingsPage/Section.tsx +++ b/site/src/pages/UserSettingsPage/Section.tsx @@ -70,6 +70,7 @@ const styles = { description: (theme) => ({ color: theme.palette.text.secondary, fontSize: 16, + margin: 0, marginTop: 4, lineHeight: "140%", }), From 1aedb92c353ecf0d5e9e2a2a86e4c3d9afc54769 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 6 Aug 2024 16:49:12 +0000 Subject: [PATCH 21/32] Update titles --- .../NotificationsPage/NotificationsPage.tsx | 85 ++++---- .../NotificationsPage/NotificationsPage.tsx | 183 +++++++++--------- 2 files changed, 141 insertions(+), 127 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index 36b8346a2a4b9..7a3df7d97737a 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -8,6 +8,7 @@ import ToggleButton from "@mui/material/ToggleButton"; import ToggleButtonGroup from "@mui/material/ToggleButtonGroup"; import Tooltip from "@mui/material/Tooltip"; import { Fragment, type FC } from "react"; +import { Helmet } from "react-helmet-async"; import { useMutation, useQueries, useQueryClient } from "react-query"; import { useSearchParams } from "react-router-dom"; import { @@ -30,6 +31,7 @@ import { } from "modules/notifications/utils"; import { Section } from "pages/UserSettingsPage/Section"; import { deploymentGroupHasParent } from "utils/deployOptions"; +import { pageTitle } from "utils/page"; import { useDeploySettings } from "../DeploySettingsLayout"; import OptionsTable from "../OptionsTable"; @@ -107,47 +109,52 @@ export const NotificationsPage: FC = () => { const tab = searchParams.get("tab") || "events"; return ( -
- - - - Events - - - Settings - - - + <> + + {pageTitle("Notifications Settings")} + +
+ + + + Events + + + Settings + + + -
- {ready ? ( - tab === "events" ? ( - +
+ {ready ? ( + tab === "events" ? ( + + ) : ( + + deploymentGroupHasParent(o.group, "Notifications"), + )} + /> + ) ) : ( - - deploymentGroupHasParent(o.group, "Notifications"), - )} - /> - ) - ) : ( - - )} -
-
+ + )} + +
+ ); }; diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx index db0919bca9192..15d54da39231d 100644 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -8,6 +8,7 @@ import ListItemText, { listItemTextClasses } from "@mui/material/ListItemText"; import Switch from "@mui/material/Switch"; import Tooltip from "@mui/material/Tooltip"; import { Fragment, type FC } from "react"; +import { Helmet } from "react-helmet-async"; import { useMutation, useQueries, useQueryClient } from "react-query"; import { notificationDispatchMethods, @@ -29,6 +30,7 @@ import { methodIcons, methodLabels, } from "modules/notifications/utils"; +import { pageTitle } from "utils/page"; import { Section } from "../Section"; type PreferenceSwitchProps = { @@ -90,100 +92,105 @@ export const NotificationsPage: FC = () => { disabledPreferences.data && templatesByGroup.data && dispatchMethods.data; return ( -
- {ready ? ( - - {Object.entries(templatesByGroup.data).map(([group, templates]) => { - const allDisabled = templates.some((tpl) => { - return disabledPreferences.data[tpl.id] === true; - }); + <> + + {pageTitle("Notifications Settings")} + +
+ {ready ? ( + + {Object.entries(templatesByGroup.data).map(([group, templates]) => { + const allDisabled = templates.some((tpl) => { + return disabledPreferences.data[tpl.id] === true; + }); - return ( - - - - - { - const updated = { ...disabledPreferences.data }; - for (const tpl of templates) { - updated[tpl.id] = !checked; - } - return updated; + return ( + + + + + { + const updated = { ...disabledPreferences.data }; + for (const tpl of templates) { + updated[tpl.id] = !checked; + } + return updated; + }} + /> + + - - - - {templates.map((tmpl) => { - const method = castNotificationMethod( - tmpl.method || dispatchMethods.data.default, - ); - const Icon = methodIcons[method]; - const label = methodLabels[method]; + + {templates.map((tmpl) => { + const method = castNotificationMethod( + tmpl.method || dispatchMethods.data.default, + ); + const Icon = methodIcons[method]; + const label = methodLabels[method]; - return ( - - - - { - return { - ...disabledPreferences.data, - [tmpl.id]: !checked, - }; + return ( + + + + { + return { + ...disabledPreferences.data, + [tmpl.id]: !checked, + }; + }} + /> + + - - - - - - - - - - - ); - })} - - - ); - })} - - ) : ( - - )} -
+ + + + + + + + + ); + })} + + + ); + })} + + ) : ( + + )} +
+ ); }; From 786b005b3b8e614bf91f5dea69af3f17744013a9 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 6 Aug 2024 17:02:45 +0000 Subject: [PATCH 22/32] Add tests --- .../NotificationsPage.stories.tsx | 191 +++++++++++++++++- 1 file changed, 190 insertions(+), 1 deletion(-) diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx index 3c49510b4e584..c35144d3eccf4 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx @@ -5,7 +5,7 @@ import { notificationDispatchMethodsKey, systemNotificationTemplatesKey, } from "api/queries/notifications"; -import type { DeploymentValues } from "api/typesGenerated"; +import type { DeploymentValues, SerpentOption } from "api/typesGenerated"; import { MockNotificationMethodsResponse, MockNotificationTemplates, @@ -33,6 +33,7 @@ const meta: Meta = { ], user: MockUser, permissions: { viewDeploymentValues: true }, + deploymentOptions: mockNotificationOptions(), deploymentValues: { notifications: { webhook: { @@ -55,6 +56,18 @@ type Story = StoryObj; export const Default: Story = {}; +export const NoWebhookEndpoint: Story = { + parameters: { + deploymentValues: { + notifications: { + webhook: { + endpoint: "", + }, + }, + } as DeploymentValues, + }, +}; + export const Toggle: Story = { play: async ({ canvasElement }) => { spyOn(API, "updateNotificationTemplateMethod").mockResolvedValue(); @@ -67,3 +80,179 @@ export const Toggle: Story = { await user.click(toggleButton); }, }; + +export const Settings: Story = { + play: async ({ canvasElement }) => { + const user = userEvent.setup(); + const canvas = within(canvasElement); + const settingsTab = await canvas.findByText("Settings"); + await user.click(settingsTab); + }, +}; + +function mockNotificationOptions(): SerpentOption[] { + return [ + { + name: "Notifications: Dispatch Timeout", + description: + "How long to wait while a notification is being sent before giving up.", + flag: "notifications-dispatch-timeout", + env: "CODER_NOTIFICATIONS_DISPATCH_TIMEOUT", + yaml: "dispatchTimeout", + default: "1m0s", + value: 60000000000, + annotations: { + format_duration: "true", + }, + group: { + name: "Notifications", + yaml: "notifications", + description: "Configure how notifications are processed and delivered.", + }, + value_source: "default", + }, + { + name: "Notifications: Fetch Interval", + description: "How often to query the database for queued notifications.", + flag: "notifications-fetch-interval", + env: "CODER_NOTIFICATIONS_FETCH_INTERVAL", + yaml: "fetchInterval", + default: "15s", + value: 15000000000, + annotations: { + format_duration: "true", + }, + group: { + name: "Notifications", + yaml: "notifications", + description: "Configure how notifications are processed and delivered.", + }, + hidden: true, + value_source: "default", + }, + { + name: "Notifications: Lease Count", + description: + "How many notifications a notifier should lease per fetch interval.", + flag: "notifications-lease-count", + env: "CODER_NOTIFICATIONS_LEASE_COUNT", + yaml: "leaseCount", + default: "20", + value: 20, + group: { + name: "Notifications", + yaml: "notifications", + description: "Configure how notifications are processed and delivered.", + }, + hidden: true, + value_source: "default", + }, + { + name: "Notifications: Lease Period", + description: + "How long a notifier should lease a message. This is effectively how long a notification is 'owned' by a notifier, and once this period expires it will be available for lease by another notifier. Leasing is important in order for multiple running notifiers to not pick the same messages to deliver concurrently. This lease period will only expire if a notifier shuts down ungracefully; a dispatch of the notification releases the lease.", + flag: "notifications-lease-period", + env: "CODER_NOTIFICATIONS_LEASE_PERIOD", + yaml: "leasePeriod", + default: "2m0s", + value: 120000000000, + annotations: { + format_duration: "true", + }, + group: { + name: "Notifications", + yaml: "notifications", + description: "Configure how notifications are processed and delivered.", + }, + hidden: true, + value_source: "default", + }, + { + name: "Notifications: Max Send Attempts", + description: "The upper limit of attempts to send a notification.", + flag: "notifications-max-send-attempts", + env: "CODER_NOTIFICATIONS_MAX_SEND_ATTEMPTS", + yaml: "maxSendAttempts", + default: "5", + value: 5, + group: { + name: "Notifications", + yaml: "notifications", + description: "Configure how notifications are processed and delivered.", + }, + value_source: "default", + }, + { + name: "Notifications: Method", + description: + "Which delivery method to use (available options: 'smtp', 'webhook').", + flag: "notifications-method", + env: "CODER_NOTIFICATIONS_METHOD", + yaml: "method", + default: "smtp", + value: "smtp", + group: { + name: "Notifications", + yaml: "notifications", + description: "Configure how notifications are processed and delivered.", + }, + value_source: "env", + }, + { + name: "Notifications: Retry Interval", + description: "The minimum time between retries.", + flag: "notifications-retry-interval", + env: "CODER_NOTIFICATIONS_RETRY_INTERVAL", + yaml: "retryInterval", + default: "5m0s", + value: 300000000000, + annotations: { + format_duration: "true", + }, + group: { + name: "Notifications", + yaml: "notifications", + description: "Configure how notifications are processed and delivered.", + }, + hidden: true, + value_source: "default", + }, + { + name: "Notifications: Store Sync Buffer Size", + description: + "The notifications system buffers message updates in memory to ease pressure on the database. This option controls how many updates are kept in memory. The lower this value the lower the change of state inconsistency in a non-graceful shutdown - but it also increases load on the database. It is recommended to keep this option at its default value.", + flag: "notifications-store-sync-buffer-size", + env: "CODER_NOTIFICATIONS_STORE_SYNC_BUFFER_SIZE", + yaml: "storeSyncBufferSize", + default: "50", + value: 50, + group: { + name: "Notifications", + yaml: "notifications", + description: "Configure how notifications are processed and delivered.", + }, + hidden: true, + value_source: "default", + }, + { + name: "Notifications: Store Sync Interval", + description: + "The notifications system buffers message updates in memory to ease pressure on the database. This option controls how often it synchronizes its state with the database. The shorter this value the lower the change of state inconsistency in a non-graceful shutdown - but it also increases load on the database. It is recommended to keep this option at its default value.", + flag: "notifications-store-sync-interval", + env: "CODER_NOTIFICATIONS_STORE_SYNC_INTERVAL", + yaml: "storeSyncInterval", + default: "2s", + value: 2000000000, + annotations: { + format_duration: "true", + }, + group: { + name: "Notifications", + yaml: "notifications", + description: "Configure how notifications are processed and delivered.", + }, + hidden: true, + value_source: "default", + }, + ]; +} From 2ecbe5ffc0a3b71864c2129b4f0ac9209b6101c3 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 7 Aug 2024 12:10:48 +0000 Subject: [PATCH 23/32] Improve product copy --- .../NotificationsPage/NotificationsPage.tsx | 10 ++++++++-- .../NotificationsPage/NotificationsPage.tsx | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index 7a3df7d97737a..25245da782b9a 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -10,7 +10,7 @@ import Tooltip from "@mui/material/Tooltip"; import { Fragment, type FC } from "react"; import { Helmet } from "react-helmet-async"; import { useMutation, useQueries, useQueryClient } from "react-query"; -import { useSearchParams } from "react-router-dom"; +import { Link, useSearchParams } from "react-router-dom"; import { notificationDispatchMethods, selectTemplatesByGroup, @@ -115,7 +115,13 @@ export const NotificationsPage: FC = () => {
+ Control delivery methods for notifications on this deployment. + Notifications may be disabled in your{" "} + profile settings. + + } layout="fluid" > diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx index 15d54da39231d..b73cb4c63ee88 100644 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -98,7 +98,7 @@ export const NotificationsPage: FC = () => {
{ready ? ( From ec7ab409dc8174288cd0cb18d08eece886e5d0c5 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 7 Aug 2024 12:20:38 +0000 Subject: [PATCH 24/32] Fix notifications visibility --- site/src/pages/DeploySettingsPage/Sidebar.tsx | 11 ++++++++--- site/src/pages/UserSettingsPage/Sidebar.tsx | 3 +-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/Sidebar.tsx b/site/src/pages/DeploySettingsPage/Sidebar.tsx index 6023947c4c0ae..c12149b298cd7 100644 --- a/site/src/pages/DeploySettingsPage/Sidebar.tsx +++ b/site/src/pages/DeploySettingsPage/Sidebar.tsx @@ -13,8 +13,11 @@ import { Sidebar as BaseSidebar, SidebarNavItem, } from "components/Sidebar/Sidebar"; +import { useDashboard } from "modules/dashboard/useDashboard"; export const Sidebar: FC = () => { + const { experiments } = useDashboard(); + return ( @@ -48,9 +51,11 @@ export const Sidebar: FC = () => { Observability - - Notifications - + {experiments.includes("notifications") && ( + + Notifications + + )} ); }; diff --git a/site/src/pages/UserSettingsPage/Sidebar.tsx b/site/src/pages/UserSettingsPage/Sidebar.tsx index 8b5190b6b16bf..e05ca300381fd 100644 --- a/site/src/pages/UserSettingsPage/Sidebar.tsx +++ b/site/src/pages/UserSettingsPage/Sidebar.tsx @@ -24,7 +24,6 @@ export const Sidebar: FC = ({ user }) => { const { entitlements, experiments } = useDashboard(); const showSchedulePage = entitlements.features.advanced_template_scheduling.enabled; - const showNotificationsPage = experiments.includes("notifications"); return ( @@ -58,7 +57,7 @@ export const Sidebar: FC = ({ user }) => { Tokens - {showNotificationsPage && ( + {experiments.includes("notifications") && ( Notifications From c986e51d997b2d6ea2e09fd4b70a9cadecba3d2f Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 7 Aug 2024 13:30:58 +0000 Subject: [PATCH 25/32] Minor improvements --- .../NotificationsPage/NotificationsPage.tsx | 23 ++++++++++++++++++- site/src/testHelpers/storybook.tsx | 2 +- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index 25245da782b9a..a99027242f1d0 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -119,7 +119,19 @@ export const NotificationsPage: FC = () => { <> Control delivery methods for notifications on this deployment. Notifications may be disabled in your{" "} - profile settings. + ({ + color: theme.roles.active.fill.outline, + textDecoration: "none", + "&: hover": { + textDecoration: "underline", + }, + })} + > + profile settings + + . } layout="fluid" @@ -178,7 +190,9 @@ const EventsView: FC = ({ templatesByGroup, }) => { const isUsingWebhook = availableMethods.includes("webhook"); + const isUsingSmpt = availableMethods.includes("smtp"); const webhookEndpoint = notificationsConfig?.webhook.endpoint; + const smtpConfig = notificationsConfig?.email; return ( @@ -187,6 +201,13 @@ const EventsView: FC = ({ Webhook method is enabled, but the endpoint is not configured. )} + + {isUsingSmpt && !smtpConfig && ( + + SMTP method is enabled, but is not configured. + + )} + {Object.entries(templatesByGroup).map(([group, templates]) => ( ( export const withAuthProvider = (Story: FC, { parameters }: StoryContext) => { if (!parameters.user) { - console.warn("You forgot to add `parameters.user` to your story"); + throw new Error("You forgot to add `parameters.user` to your story"); } // eslint-disable-next-line react-hooks/rules-of-hooks -- decorators are components const queryClient = useQueryClient(); From e037423c515d0a6c0c5ce173d5382a1770abb0c7 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 8 Aug 2024 13:56:14 +0000 Subject: [PATCH 26/32] Remove alerts --- .../NotificationsPage.stories.tsx | 12 ---------- .../NotificationsPage/NotificationsPage.tsx | 22 ------------------- 2 files changed, 34 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx index c35144d3eccf4..7315595883efd 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx @@ -56,18 +56,6 @@ type Story = StoryObj; export const Default: Story = {}; -export const NoWebhookEndpoint: Story = { - parameters: { - deploymentValues: { - notifications: { - webhook: { - endpoint: "", - }, - }, - } as DeploymentValues, - }, -}; - export const Toggle: Story = { play: async ({ canvasElement }) => { spyOn(API, "updateNotificationTemplateMethod").mockResolvedValue(); diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index a99027242f1d0..8a8713d876ec4 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -17,8 +17,6 @@ import { systemNotificationTemplates, updateNotificationTemplateMethod, } from "api/queries/notifications"; -import type { NotificationsConfig } from "api/typesGenerated"; -import { Alert } from "components/Alert/Alert"; import { displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; @@ -157,7 +155,6 @@ export const NotificationsPage: FC = () => { availableMethods={dispatchMethods.data.available.map( castNotificationMethod, )} - notificationsConfig={deploymentValues.config.notifications} templatesByGroup={templatesByGroup.data} /> ) : ( @@ -179,35 +176,16 @@ export const NotificationsPage: FC = () => { type EventsViewProps = { defaultMethod: NotificationMethod; availableMethods: NotificationMethod[]; - notificationsConfig?: NotificationsConfig; templatesByGroup: ReturnType; }; const EventsView: FC = ({ defaultMethod, availableMethods, - notificationsConfig, templatesByGroup, }) => { - const isUsingWebhook = availableMethods.includes("webhook"); - const isUsingSmpt = availableMethods.includes("smtp"); - const webhookEndpoint = notificationsConfig?.webhook.endpoint; - const smtpConfig = notificationsConfig?.email; - return ( - {isUsingWebhook && !webhookEndpoint && ( - - Webhook method is enabled, but the endpoint is not configured. - - )} - - {isUsingSmpt && !smtpConfig && ( - - SMTP method is enabled, but is not configured. - - )} - {Object.entries(templatesByGroup).map(([group, templates]) => ( Date: Thu, 8 Aug 2024 16:54:34 +0000 Subject: [PATCH 27/32] Add alerts when SMTP or Webhook config are enabled but not set --- .../NotificationsPage.stories.tsx | 33 +++++++++++++++++++ .../NotificationsPage/NotificationsPage.tsx | 23 ++++++++++++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx index 7315595883efd..e902d3d5cd940 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.stories.tsx @@ -39,6 +39,9 @@ const meta: Meta = { webhook: { endpoint: "https://example.com", }, + email: { + smarthost: "smtp.example.com", + }, }, } as DeploymentValues, }, @@ -56,6 +59,36 @@ type Story = StoryObj; export const Default: Story = {}; +export const NoEmailSmarthost: Story = { + parameters: { + deploymentValues: { + notifications: { + webhook: { + endpoint: "https://example.com", + }, + email: { + smarthost: "", + }, + }, + } as DeploymentValues, + }, +}; + +export const NoWebhookEndpoint: Story = { + parameters: { + deploymentValues: { + notifications: { + webhook: { + endpoint: "", + }, + email: { + smarthost: "smtp.example.com", + }, + }, + } as DeploymentValues, + }, +}; + export const Toggle: Story = { play: async ({ canvasElement }) => { spyOn(API, "updateNotificationTemplateMethod").mockResolvedValue(); diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index 8a8713d876ec4..b31ca4bcc2970 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -17,6 +17,8 @@ import { systemNotificationTemplates, updateNotificationTemplateMethod, } from "api/queries/notifications"; +import type { DeploymentValues } from "api/typesGenerated"; +import { Alert } from "components/Alert/Alert"; import { displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; @@ -149,13 +151,14 @@ export const NotificationsPage: FC = () => { {ready ? ( tab === "events" ? ( ) : ( ; + deploymentValues: DeploymentValues; }; const EventsView: FC = ({ defaultMethod, availableMethods, templatesByGroup, + deploymentValues, }) => { return ( + {availableMethods.includes("smtp") && + deploymentValues.notifications?.webhook.endpoint === "" && ( + + Webhook notifications are enabled, but no endpoint has been + configured. + + )} + + {availableMethods.includes("smtp") && + deploymentValues.notifications?.email.smarthost === "" && ( + + SMTP notifications are enabled, but no smart host has been + configured. + + )} + {Object.entries(templatesByGroup).map(([group, templates]) => ( Date: Thu, 8 Aug 2024 17:26:09 +0000 Subject: [PATCH 28/32] Apply a few Michaels suggestions --- site/src/api/queries/notifications.ts | 18 ++++++++++++++++-- .../NotificationsPage/NotificationsPage.tsx | 12 ++++-------- .../NotificationsPage/NotificationsPage.tsx | 12 ++++-------- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/site/src/api/queries/notifications.ts b/site/src/api/queries/notifications.ts index 11218292e8521..7c6f9c4f6e804 100644 --- a/site/src/api/queries/notifications.ts +++ b/site/src/api/queries/notifications.ts @@ -38,7 +38,7 @@ export const updateUserNotificationPreferences = ( id, disabled, updated_at: new Date().toISOString(), - }) as NotificationPreference, + }) satisfies NotificationPreference, ), ); }, @@ -65,7 +65,7 @@ export const systemNotificationTemplates = () => { export function selectTemplatesByGroup( data: NotificationTemplate[], ): Record { - return data.reduce( + const grouped = data.reduce( (acc, tpl) => { if (!acc[tpl.group]) { acc[tpl.group] = []; @@ -75,6 +75,20 @@ export function selectTemplatesByGroup( }, {} as Record, ); + + // Sort templates within each group + for (const group in grouped) { + grouped[group].sort((a, b) => a.name.localeCompare(b.name)); + } + + // Sort groups by name + const sortedGroups = Object.keys(grouped).sort((a, b) => a.localeCompare(b)); + const sortedGrouped: Record = {}; + for (const group of sortedGroups) { + sortedGrouped[group] = grouped[group]; + } + + return sortedGrouped; } export const notificationDispatchMethodsKey = [ diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index b31ca4bcc2970..6ba14ed82dab0 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -190,7 +190,7 @@ const EventsView: FC = ({ deploymentValues, }) => { return ( - + {availableMethods.includes("smtp") && deploymentValues.notifications?.webhook.endpoint === "" && ( @@ -218,8 +218,9 @@ const EventsView: FC = ({ - {templates.map((tpl) => { + {templates.map((tpl, i) => { const value = castNotificationMethod(tpl.method || defaultMethod); + const isLastItem = i === templates.length - 1; return ( @@ -234,7 +235,7 @@ const EventsView: FC = ({ value={value} /> - + {!isLastItem && } ); })} @@ -281,9 +282,4 @@ const styles = { fontSize: "inherit", }, }), - divider: { - "&:last-child": { - display: "none", - }, - }, } as Record>; diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx index b73cb4c63ee88..0e12a1d7d63d9 100644 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -102,7 +102,7 @@ export const NotificationsPage: FC = () => { layout="fluid" > {ready ? ( - + {Object.entries(templatesByGroup.data).map(([group, templates]) => { const allDisabled = templates.some((tpl) => { return disabledPreferences.data[tpl.id] === true; @@ -138,12 +138,13 @@ export const NotificationsPage: FC = () => { }} /> - {templates.map((tmpl) => { + {templates.map((tmpl, i) => { const method = castNotificationMethod( tmpl.method || dispatchMethods.data.default, ); const Icon = methodIcons[method]; const label = methodLabels[method]; + const isLastItem = i === templates.length - 1; return ( @@ -177,7 +178,7 @@ export const NotificationsPage: FC = () => { - + {!isLastItem && } ); })} @@ -230,9 +231,4 @@ const styles = { fontSize: "inherit", }, }), - divider: { - "&:last-child": { - display: "none", - }, - }, } as Record>; From d403eed382480f34ba731770749cc60c406f7610 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 9 Aug 2024 14:26:43 +0000 Subject: [PATCH 29/32] Simplify state logic for the switch component --- .../NotificationsPage/NotificationsPage.tsx | 66 +++++++------------ 1 file changed, 23 insertions(+), 43 deletions(-) diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx index 0e12a1d7d63d9..b7c399ca35acd 100644 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -33,38 +33,6 @@ import { import { pageTitle } from "utils/page"; import { Section } from "../Section"; -type PreferenceSwitchProps = { - id: string; - disabled: boolean; - onToggle: (checked: boolean) => Record; -}; - -const PreferenceSwitch: FC = ({ - id, - disabled, - onToggle, -}) => { - const { user } = useAuthenticated(); - const queryClient = useQueryClient(); - const updatePreferences = useMutation( - updateUserNotificationPreferences(user.id, queryClient), - ); - - return ( - { - await updatePreferences.mutateAsync({ - template_disabled_map: onToggle(checked), - }); - displaySuccess("Notification preferences updated"); - }} - /> - ); -}; - export const NotificationsPage: FC = () => { const { user, permissions } = useAuthenticated(); const [disabledPreferences, templatesByGroup, dispatchMethods] = useQueries({ @@ -88,6 +56,10 @@ export const NotificationsPage: FC = () => { notificationDispatchMethods(), ], }); + const queryClient = useQueryClient(); + const updatePreferences = useMutation( + updateUserNotificationPreferences(user.id, queryClient), + ); const ready = disabledPreferences.data && templatesByGroup.data && dispatchMethods.data; @@ -117,15 +89,18 @@ export const NotificationsPage: FC = () => { - { + checked={!allDisabled} + onChange={async (_, checked) => { const updated = { ...disabledPreferences.data }; for (const tpl of templates) { updated[tpl.id] = !checked; } - return updated; + await updatePreferences.mutateAsync({ + template_disabled_map: updated, + }); + displaySuccess("Notification preferences updated"); }} /> @@ -150,14 +125,19 @@ export const NotificationsPage: FC = () => { - { - return { - ...disabledPreferences.data, - [tmpl.id]: !checked, - }; + checked={!disabledPreferences.data[tmpl.id]} + onChange={async (_, checked) => { + await updatePreferences.mutateAsync({ + template_disabled_map: { + ...disabledPreferences.data, + [tmpl.id]: !checked, + }, + }); + displaySuccess( + "Notification preferences updated", + ); }} /> From fb02aec23e64e76fbc1b3803c99e6010347a950d Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 9 Aug 2024 14:38:56 +0000 Subject: [PATCH 30/32] Update copy --- .../NotificationsPage/NotificationsPage.tsx | 22 ++----------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index 6ba14ed82dab0..232aed5bcb093 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -10,7 +10,7 @@ import Tooltip from "@mui/material/Tooltip"; import { Fragment, type FC } from "react"; import { Helmet } from "react-helmet-async"; import { useMutation, useQueries, useQueryClient } from "react-query"; -import { Link, useSearchParams } from "react-router-dom"; +import { useSearchParams } from "react-router-dom"; import { notificationDispatchMethods, selectTemplatesByGroup, @@ -115,25 +115,7 @@ export const NotificationsPage: FC = () => {
- Control delivery methods for notifications on this deployment. - Notifications may be disabled in your{" "} - ({ - color: theme.roles.active.fill.outline, - textDecoration: "none", - "&: hover": { - textDecoration: "underline", - }, - })} - > - profile settings - - . - - } + description="Control delivery methods for notifications on this deployment." layout="fluid" > From 013ccfffc8df77effb0e96c8f66f4a97163c8e89 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 9 Aug 2024 16:06:58 +0000 Subject: [PATCH 31/32] Apply PR comments --- .../NotificationsPage/NotificationsPage.tsx | 2 +- .../pages/ManagementSettingsPage/Sidebar.tsx | 3 +++ .../SidebarView.stories.tsx | 1 + .../ManagementSettingsPage/SidebarView.tsx | 21 +++++++++++++++---- 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index 232aed5bcb093..99c858a7b8767 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -184,7 +184,7 @@ const EventsView: FC = ({ {availableMethods.includes("smtp") && deploymentValues.notifications?.email.smarthost === "" && ( - SMTP notifications are enabled, but no smart host has been + SMTP notifications are enabled, but no smarthost has been configured. )} diff --git a/site/src/pages/ManagementSettingsPage/Sidebar.tsx b/site/src/pages/ManagementSettingsPage/Sidebar.tsx index 6ac55c59b999f..44ee6021c8d6f 100644 --- a/site/src/pages/ManagementSettingsPage/Sidebar.tsx +++ b/site/src/pages/ManagementSettingsPage/Sidebar.tsx @@ -3,6 +3,7 @@ import { useQuery } from "react-query"; import { useLocation, useParams } from "react-router-dom"; import { organizationsPermissions } from "api/queries/organizations"; import { useAuthenticated } from "contexts/auth/RequireAuth"; +import { useDashboard } from "modules/dashboard/useDashboard"; import { canEditOrganization, useOrganizationSettings, @@ -19,6 +20,7 @@ import { type OrganizationWithPermissions, SidebarView } from "./SidebarView"; export const Sidebar: FC = () => { const location = useLocation(); const { permissions } = useAuthenticated(); + const { experiments } = useDashboard(); const { organizations } = useOrganizationSettings(); const { organization: organizationName } = useParams() as { organization?: string; @@ -54,6 +56,7 @@ export const Sidebar: FC = () => { activeOrganizationName={organizationName} organizations={editableOrgs} permissions={permissions} + experiments={experiments} /> ); }; diff --git a/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx b/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx index 44cb0de7fc97a..adb0688844c85 100644 --- a/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx +++ b/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx @@ -35,6 +35,7 @@ const meta: Meta = { }, ], permissions: MockPermissions, + experiments: ["notifications"], }, }; diff --git a/site/src/pages/ManagementSettingsPage/SidebarView.tsx b/site/src/pages/ManagementSettingsPage/SidebarView.tsx index 21ece25bcf60a..0571f17c8eaf3 100644 --- a/site/src/pages/ManagementSettingsPage/SidebarView.tsx +++ b/site/src/pages/ManagementSettingsPage/SidebarView.tsx @@ -4,7 +4,11 @@ import AddIcon from "@mui/icons-material/Add"; import SettingsIcon from "@mui/icons-material/Settings"; import type { FC, ReactNode } from "react"; import { Link, NavLink } from "react-router-dom"; -import type { AuthorizationResponse, Organization } from "api/typesGenerated"; +import type { + AuthorizationResponse, + Experiments, + Organization, +} from "api/typesGenerated"; import { Loader } from "components/Loader/Loader"; import { Sidebar as BaseSidebar } from "components/Sidebar/Sidebar"; import { Stack } from "components/Stack/Stack"; @@ -26,6 +30,8 @@ interface SidebarProps { organizations: OrganizationWithPermissions[] | undefined; /** Site-wide permissions. */ permissions: AuthorizationResponse; + /** Active experiments */ + experiments: Experiments; } /** @@ -36,6 +42,7 @@ export const SidebarView: FC = ({ activeOrganizationName, organizations, permissions, + experiments, }) => { // TODO: Do something nice to scroll to the active org. return ( @@ -43,6 +50,7 @@ export const SidebarView: FC = ({
Deployment
= ({ active, permissions, + experiments, }) => { return (
@@ -133,9 +144,11 @@ const DeploymentSettingsNavigation: FC = ({ Users )} - - Notifications - + {experiments.includes("notifications") && ( + + Notifications + + )} )}
From 7858a5a76e5207097284cb6388d3fcd31683a141 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 9 Aug 2024 16:23:41 +0000 Subject: [PATCH 32/32] Add docs --- .../NotificationsPage/NotificationsPage.tsx | 34 +++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx index 99c858a7b8767..a76b9e08d9274 100644 --- a/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploySettingsPage/NotificationsPage/NotificationsPage.tsx @@ -1,4 +1,5 @@ import type { Interpolation, Theme } from "@emotion/react"; +import Button from "@mui/material/Button"; import Card from "@mui/material/Card"; import Divider from "@mui/material/Divider"; import List from "@mui/material/List"; @@ -31,6 +32,7 @@ import { } from "modules/notifications/utils"; import { Section } from "pages/UserSettingsPage/Section"; import { deploymentGroupHasParent } from "utils/deployOptions"; +import { docs } from "utils/docs"; import { pageTitle } from "utils/page"; import { useDeploySettings } from "../DeploySettingsLayout"; import OptionsTable from "../OptionsTable"; @@ -175,7 +177,21 @@ const EventsView: FC = ({ {availableMethods.includes("smtp") && deploymentValues.notifications?.webhook.endpoint === "" && ( - + + Read the docs + + } + > Webhook notifications are enabled, but no endpoint has been configured. @@ -183,7 +199,21 @@ const EventsView: FC = ({ {availableMethods.includes("smtp") && deploymentValues.notifications?.email.smarthost === "" && ( - + + Read the docs + + } + > SMTP notifications are enabled, but no smarthost has been configured.