Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

お知らせ通知を高速化する #7438

Merged
merged 2 commits into from
Mar 9, 2024

Conversation

naokinaokiboo
Copy link
Contributor

@naokinaokiboo naokinaokiboo commented Feb 26, 2024

Issue

概要

お知らせ通知を投稿したとき、現状の実装ではユーザー1人につき2つのJob(ActiveJobのJob)を使って通知を送っている。(メール通知で1つ、サイト内通知で1つ)

そのため、通知する人数の2倍のJobが作成されてしまう。

リソース節約のために、1つのJobで対象ユーザー全員への通知を行うようにする。

変更確認方法

  1. feature/optimize-post-announcementをローカルに取り込む
  2. 任意のユーザーでログインをし、お知らせ作成ページ(http://localhost:3000/announcements/new)にアクセスする
  3. タイトル、内容を入力、通知ターゲットは全員(退会者を除く)を選択し、「作成」ボタンをクリック
  4. 以下の内容を確認する(個々の詳細な確認方法については後述)
    • お知らせ通知作成時、1つのJobだけがキューに入れられていること
    • メール通知、サイト内通知が正しく行われていること

お知らせ通知作成時、1つのJobだけがキューに入れられていることを確認

log/development.logに、キューにJobを追加するログが1つしか出ていないことを確認する

以下、ログのサンプル

[ActiveJob] [PostAnnouncementJob] [b06939d2-4604-4856-94a2-ce648647c7e1] Performing PostAnnouncementJob (Job ID: b06939d2-4604-4856-94a2-ce648647c7e1) from GoodJob(default) enqueued at 2024-02-26T03:05:37Z with arguments:

メール通知、サイト内通知が正しく行われていることを確認

メール通知

  • http://localhost:3000/letter_openerにアクセスして、送信されたメールの数が正しいことを確認する
    • メール通知が行われる数 = 全ユーザー数 - (メール通知がOFFまたは退会している人の数) - (発信者:1)(※ 発信者はメール通知ONである前提の式です)
  • お知らせを作成した人へのメールは存在しないことを確認する

参考:全ユーザー数

select count(*) from users;

参考:メール通知がOFFまたは退会者となっている人の数

select count(*) from users where mail_notification = false or retired_on is not null;

サイト内通知

  • notificationsテーブルで対象の通知のレコード数が正しいことを確認する
    • サイト内通知が行われる数 = 全ユーザー数 - (退会している人の数) - (発信者:1)
  • お知らせを作成した人へはサイト内通知が作成されていないことを確認する
    • 右上の通知に、作成したお知らせが表示されていなければOKです。
      image

参考:退会している人の数

select count(*) from users where retired_on is not null;

参考:対象のお知らせのサイト内通知の数

-- xxxxxxxxxxの部分は、対象のお知らせのidに合わせる
select count(*) from notifications where link = '/announcements/xxxxxxxxxx';

image

Screenshot

内部的な変更のため無し

@naokinaokiboo naokinaokiboo changed the title Feature/optimize post announcement お知らせ通知を高速化する Feb 26, 2024
@naokinaokiboo naokinaokiboo self-assigned this Feb 26, 2024
@naokinaokiboo naokinaokiboo force-pushed the feature/optimize-post-announcement branch from d53f34c to 39290d8 Compare February 26, 2024 06:52
@naokinaokiboo naokinaokiboo force-pushed the feature/optimize-post-announcement branch from 39290d8 to c02003a Compare February 26, 2024 06:55
@naokinaokiboo naokinaokiboo marked this pull request as ready for review February 27, 2024 06:10
@naokinaokiboo
Copy link
Contributor Author

@taco-nantai
お疲れ様です。
ご都合の良いタイミングでレビューをお願いできますでしょうか🙏(全く急ぎではありません)
もしご都合悪いなどありましたら、その旨、仰っていただけたらと思います。
よろしくお願い致します。🙇‍♂️

@naokinaokiboo naokinaokiboo requested a review from a user February 27, 2024 06:13
@naokinaokiboo
Copy link
Contributor Author

@taco-nantai
お疲れ様です。
こちら、リアクションが無いようなので、気づかれていないかも?と思い、再度ご連絡させて頂きました。🙏
もし、メッセージご確認頂ければ、リアクションだけお願いします。🙇‍♂️
(レビューが急いで欲しいという意図ではありません。)

@ghost
Copy link

ghost commented Mar 2, 2024

@naokinaokiboo
お疲れ様です。
メールに気付かずレスポンスが遅れてしまい申し訳ありません🙇‍♂️
レビュー依頼の件かしこまりました。
2~3日以内に対応させていただきます。

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

動作確認とコードの確認を行いました。
問題なさそうなのでApproveしました!👍

@naokinaokiboo
Copy link
Contributor Author

@taco-nantai
お忙しい中、レビューありがとうございました!🙏

@naokinaokiboo
Copy link
Contributor Author

@komagata
チームメンバーにApproveをいただいたので、レビューをお願いいたします🙏

@naokinaokiboo naokinaokiboo requested a review from komagata March 6, 2024 07:44
Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

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

確認させて頂きました。OKです〜🙆‍♂️

@komagata komagata merged commit fe3e4da into main Mar 9, 2024
5 checks passed
@komagata komagata deleted the feature/optimize-post-announcement branch March 9, 2024 17:01
@github-actions github-actions bot mentioned this pull request Mar 9, 2024
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants