8000 feat: turn off notification via email by joobisb · Pull Request #14520 · coder/coder · GitHub
[go: up one dir, main page]
Skip to content

feat: turn off notification via email #14520

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Sep 11, 2024
Merged

feat: turn off notification via email #14520

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8568812
feat: turn off notification via email
joobisb Sep 1, 2024
5309f0a
remove unused imports
joobisb Sep 1, 2024
254f4dc
addressed the comments
joobisb Sep 3, 2024
f9ac6d2
chore: refactored notifications page
joobisb Sep 4, 2024
18b3f9a
refactored and added tests
joobisb Sep 5, 2024
8538ec4
Merge remote-tracking branch 'origin/main' into email-notification-di…
joobisb Sep 11, 2024
025c633
Simplify effect
BrunoQuaresma Sep 11, 2024
8af93fd
Fix stories
BrunoQuaresma Sep 11, 2024
7f0a831
Fix fmt
BrunoQuaresma Sep 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: refactored notifications page
  • Loading branch information
@joobisb
joobisb committed Sep 4, 2024
commit f9ac6d20af67e7ea6e141f8d6dfb527e8ae25fb9
Binary file removed coderd/notifications/Archive.zip
View file
Open in desktop
Binary file not shown.
38 changes: 38 additions & 0 deletions site/src/api/queries/notifications.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {
UpdateNotificationTemplateMethod,
UpdateUserNotificationPreferences,
} from "api/typesGenerated";
import { displayError, displaySuccess } from "components/GlobalSnackbar/utils";
import type { QueryClient, UseMutationOptions } from "react-query";

export const userNotificationPreferencesKey = (userId: string) => [
Expand Down Expand Up @@ -136,3 +137,40 @@ export const updateNotificationTemplateMethod = (
UpdateNotificationTemplateMethod
>;
};

export const disableNotification = (
userId: string,
queryClient: QueryClient,
) => {
return {
mutationFn: async (templateId: string) => {
const result = await API.putUserNotificationPreferences(userId, {
template_disabled_map: {
[templateId]: true,
},
});

// Invalidate the user notification preferences query
queryClient.invalidateQueries(userNotificationPreferencesKey(userId));

return result;
},
onSuccess: (_, templateId) => {
const allTemplates = queryClient.getQueryData<NotificationTemplate[]>(
systemNotificationTemplatesKey,
);
const template = allTemplates?.find((t) => t.id === templateId);

if (template) {
displaySuccess(`${template.name} notification has been disabled`);
} else {
displaySuccess("Notification has been disabled");
}
},
onError: () => {
displayError(
"An error occurred when attempting to disable the requested notification",
);
},
} satisfies UseMutationOptions<NotificationPreference[], unknown, string>;
};
43 changes: 13 additions & 30 deletions site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.tsx
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

I need to remember to check for the following scenarios:

  1. When a valid template ID is disabled and the request is successful, check if the success message is displayed. Also, ensure that the UI is updated correctly. Verify if the request is being sent properly. Additionally, confirm if the error message is displayed when the request fails.

  2. When a not found template ID is disabled, check if an error message is displayed.

You can check how we do that using Storybook interaction tests on the NotificationsPage.stories.tsx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have verified these scnearios

Copy link
Contributor

Choose a reason for hiding this comment

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

@joobisb can you add a test please? Manual verification is good but it only helps us know if this works currently, and doesn't catch future degradations.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@joobisb we are close, we just need to automate these tests using the way I shared before. Thanks for your hard work! 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BrunoQuaresma the tests needed to be added to NotificationsPage.stories.tsx right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joobisb we are close, we just need to automate these tests using the way I shared before. Thanks for your hard work! 🙏

done, please have a look

Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import ListItemText, { listItemTextClasses } from "@mui/material/ListItemText";
import Switch from "@mui/material/Switch";
import Tooltip from "@mui/material/Tooltip";
import {
disableNotification,
notificationDispatchMethods,
selectTemplatesByGroup,
systemNotificationTemplates,
Expand Down Expand Up @@ -62,40 +63,22 @@ export const NotificationsPage: FC = () => {
const updatePreferences = useMutation(
updateUserNotificationPreferences(user.id, queryClient),
);
const disableMutation = useMutation(
disableNotification(user.id, queryClient),
);
const [searchParams] = useSearchParams();
const templateId = searchParams.get("disabled");
const disabledId = searchParams.get("disabled");

useEffect(() => {
if (templateId && templatesByGroup.isSuccess && templatesByGroup.data) {
disableTemplate(templateId);
}
}, [templateId, templatesByGroup.isSuccess, templatesByGroup.data]);

const disableTemplate = async (templateId: string) => {
try {
await updatePreferences.mutateAsync({
template_disabled_map: {
[templateId]: true,
},
});

const allTemplates = Object.values(templatesByGroup.data ?? {}).flat();
const template = allTemplates.find((t) => t.id === templateId);

if (!template) {
throw new Error(`Template with ID ${templateId} not found`);
}

displaySuccess(`${template.name} notification has been disabled`);

queryClient.invalidateQueries(
userNotificationPreferences(user.id).queryKey,
);
} catch (error) {
console.error(error);
displayError("Error on disabling notification");
if (disabledId && templatesByGroup.isSuccess && templatesByGroup.data) {
disableMutation.mutate(disabledId);
}
};
}, [
disabledId,
templatesByGroup.isSuccess,
templatesByGroup.data,
disableMutation,
]);

const ready =
disabledPreferences.data && templatesByGroup.data && dispatchMethods.data;
Expand Down
Loading
0