Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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.

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

Already on GitHub?Sign in to your account

Merged
BrunoQuaresma merged 9 commits intocoder:mainfromjoobisb:email-notification-disable
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from1 commit
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
PrevPrevious commit
NextNext commit
addressed the comments
  • Loading branch information
@joobisb
joobisb committedSep 3, 2024
commit254f4dce98fde018dbff5a7a53f40a571e9d5a00
Binary file addedcoderd/notifications/Archive.zip
View file
Open in desktop
Binary file not shown.
2 changes: 1 addition & 1 deletioncoderd/notifications/dispatch/smtp/html.gotmpl
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,7 +26,7 @@
<div style="border-top: 1px solid #e2e8f0; color: #475569; font-size: 12px; margin-top: 64px; padding-top: 24px; line-height: 1.6;">
<p>&copy;&nbsp;{{ current_year }}&nbsp;Coder. All rights reserved&nbsp;-&nbsp;<a href="{{ base_url }}" style="color: #2563eb; text-decoration: none;">{{ base_url }}</a></p>
<p><a href="{{ base_url }}/settings/notifications" style="color: #2563eb; text-decoration: none;">Click here to manage your notification settings</a></p>
<p><a href="{{ base_url }}/settings/notifications?unsubscribe={{ .NotificationTemplateID }}" style="color: #2563eb; text-decoration: none;">Stop receiving emails like this</a></p>
<p><a href="{{ base_url }}/settings/notifications?disabled={{ .NotificationTemplateID }}" style="color: #2563eb; text-decoration: none;">Stop receiving emails like this</a></p>
</div>
</div>
</body>
Expand Down
2 changes: 1 addition & 1 deletioncoderd/notifications/enqueuer.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -121,7 +121,7 @@ func (s *StoreEnqueuer) Enqueue(ctx context.Context, userID, templateID uuid.UUI
// actions which can be taken by the recipient.
func (s *StoreEnqueuer) buildPayload(metadata database.FetchNewMessageMetadataRow, labels map[string]string) (*types.MessagePayload, error) {
payload := types.MessagePayload{
Version: "1.0",
Version: "1.1",

NotificationName: metadata.NotificationName,
NotificationTemplateID: metadata.NotificationTemplateID.String(),
Expand Down
18 changes: 10 additions & 8 deletionscoderd/notifications/types/payload.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,12 +7,14 @@ package types
type MessagePayload struct {
Version string `json:"_version"`

NotificationName string `json:"notification_name"`
NotificationTemplateID string `json:"notification_template_id"`
UserID string `json:"user_id"`
UserEmail string `json:"user_email"`
UserName string `json:"user_name"`
UserUsername string `json:"user_username"`
Actions []TemplateAction `json:"actions"`
Labels map[string]string `json:"labels"`
NotificationName string `json:"notification_name"`
NotificationTemplateID string `json:"notification_template_id"`

UserID string `json:"user_id"`
UserEmail string `json:"user_email"`
UserName string `json:"user_name"`
UserUsername string `json:"user_username"`

Actions []TemplateAction `json:"actions"`
Labels map[string]string `json:"labels"`
}
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 usingStorybook interaction tests on theNotificationsPage.stories.tsx.

joobisb reacted with thumbs up emoji
Copy link
ContributorAuthor

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
ContributorAuthor

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 toNotificationsPage.stories.tsx right ?

Copy link
ContributorAuthor

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 numberDiff line numberDiff line change
Expand Up@@ -18,7 +18,7 @@ import type {
NotificationPreference,
NotificationTemplate,
} from "api/typesGenerated";
import { displaySuccess } from "components/GlobalSnackbar/utils";
import {displayError,displaySuccess } from "components/GlobalSnackbar/utils";
import { Loader } from "components/Loader/Loader";
import { Stack } from "components/Stack/Stack";
import { useAuthenticated } from "contexts/auth/RequireAuth";
Expand DownExpand Up@@ -63,24 +63,38 @@ export const NotificationsPage: FC = () => {
updateUserNotificationPreferences(user.id, queryClient),
);
const [searchParams] = useSearchParams();
constunsubscribeTemplateId = searchParams.get("unsubscribe");
consttemplateId = searchParams.get("disabled");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would rename this to something like 'disabledId' to make it clearer about its usage.

joobisb reacted with thumbs up emoji

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

const handleUnsubscribe = async (templateId: string) => {
await updatePreferences.mutateAsync({
template_disabled_map: {
[templateId]: true,
},
});
displaySuccess("Notification preferences updated");
queryClient.invalidateQueries(
userNotificationPreferences(user.id).queryKey,
);
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");
Copy link
Collaborator

@BrunoQuaresmaBrunoQuaresmaSep 3, 2024•
edited
Loading

Choose a reason for hiding this comment

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

I believe that moving thedisableTemplate mutation inside thequeries/notifications would improve the readability of the view. This approach aligns with the pattern we've been using in the UI, where all "server data" is handled in the queries module. Eg.:

constqueryClient=useQueryClient()constdisableMutation=useMutation(disableNotification(userId,queryClient))useEffect(()=>{try{disableMutation.mutateAsync().then(()=>{displaySuccess("...")}).catch(()=>{displayError("...")})}},[...])

joobisb reacted with thumbs up emoji
}
};

const ready =
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp