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

ユーザーが休会した時にメンターと管理者にサイト内通知とメール通知が行くようにした #5454

Merged
merged 10 commits into from
Sep 14, 2022

Conversation

keiz1213
Copy link
Contributor

@keiz1213 keiz1213 commented Aug 29, 2022

issue

概要

ユーザーが休会した時に管理者、メンターにサイト内通知メール通知が行われるようにしました。

備考

  • 完全な休会機能はまだmainブランチにmergeされていません。なので当ブランチは休会からの復帰機能を追加 #5369 から作成したものであり、PRはそこへのPRとなります。
  • Discordへの通知は既に実装済みとのことなので、今回追加した通知はサイト内通知メール通知になります。
    • ※(注)「実装済み」とのことですが今回の変更に伴い、DiscordNotifierの呼び出し方法を変更しました。またテストも無かったのでテストを追加しました。

DiscordNotifierの呼び出し方法について

実装済みのDiscord通知では同じ通知が複数回通知されてしまうため、DiscordNotifierの呼び出しをNotificationFacadeから独立させました。

# hibernation_controller

  def notify_to_mentors_and_admins
    User.admins_and_mentors.each do |admin_or_mentor|
     # 管理者とメンターの数だけ呼びだされる
      NotificationFacade.hibernated(current_user, admin_or_mentor)
    end
  end
# 変更前のNotificationFacade

  def self.hibernated(sender, receiver)
    ActivityNotifier.with(sender: sender, receiver: receiver).hibernated.notify_now
    # このDiscordNotifierを独立させた
    DiscordNotifier.with(sender: sender, receiver: receiver).hibernated.notify_now
    return unless receiver.mail_notification?

    NotificationMailer.with(
      sender: sender,
      receiver: receiver
    ).hibernated.deliver_later(wait: 5)
  end

Discordへの通知を確認するための準備

Discordへの通知を確認するために自身のDiscordアカウントで

  • 確認用のサーバーの作成
  • チャンネルのwebhook_urlの取得

をお願いいたします。

確認用のサーバーの作成とwebhookURLの取得方法

  1. サーバーの新規作成
    _k-n📮(nakamura)__fjordbootcamp-_Discord
  2. オリジナルの作成
    _k-n📮(nakamura)__fjordbootcamp-_Discord
  3. 自分と友達のためを選択
    _k-n📮(nakamura)__fjordbootcamp-_Discord
  4. 適当な名前を入力
    _k-n📮(nakamura)__fjordbootcamp-_Discord
  5. 編集をクリック
    test___test-_Discord
  6. 連携サービスをクリック
    概要___test_-_Discord
  7. webフックを作成
    連携サービス___test_-_Discord
  8. webフックをコピー
    連携サービス___test_-_Discord

取得したwebhookURLをローカルのマシンの環境変数に設定をお願いします。

# 自分は.zshrcに設定しました
export DISCORD_ADMIN_WEBHOOK_URL='作成したサーバー内チャンネルのwebhook_url'
$ source .zshrc

変更確認方法

  1. feature/notification-of-hibernationをローカルに取り込む。
  2. rails sでサーバーを立ち上げる
  3. komagatamachidamentormentaroadminonlyunadmentor、でログインする
  4. ヘッダーの通知をクリックし、休会通知が来ていないことを確認する。(mentormentaroの場合、hatsunoが休会しました。という通知がありますが無視してください)
  5. kimuraでログインし、ヘッダーのユーザーメニューから休会手続きを行う。
  6. 2のユーザーで再度ログインし、ヘッダーの通知をクリックし、kimuraの休会通知が来ていることを確認する。
  7. http://localhost:3000/letter_opener にアクセスし2のユーザーにメールが送信されていることを確認する。
  8. discordにkimuraの休会通知が来ているか確認する。

@keiz1213 keiz1213 self-assigned this Aug 29, 2022
@keiz1213 keiz1213 force-pushed the feature/notification-of-hibernation branch from d035c39 to 37ae72f Compare September 11, 2022 03:51
@keiz1213 keiz1213 force-pushed the feature/notification-of-hibernation branch from 37ae72f to cc888f7 Compare September 12, 2022 04:52
@@ -53,7 +53,7 @@ class CurrentUserTest < ApplicationSystemTestCase
visit_with_auth '/current_user/edit', 'komagata'
fill_in 'user[times_url]', with: 'https://example.com/channels/1234/5678/'
click_button '更新する'
assert_text '分報チャンネル URL は Discord のチャンネル URL を入力してください'
assert_text '分報チャンネル URLはDiscordのチャンネルURLを入力してください'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

今回のissueとは関係がありませんが、テストを通すため修正しました

@keiz1213 keiz1213 marked this pull request as ready for review September 13, 2022 06:23
@keiz1213
Copy link
Contributor Author

@yuma-matsui
お疲れさまです!以前レビューさせていただいた時のissueと似ているissueだったのでレビュー依頼させていただきました。DiscordNotifierが複数回呼び出される問題について僕の方では独立させて実装することにしました💡
全然急いでないので、お時間があるときにでもよろしくお願いいたします🙏

@yuma-matsui
Copy link
Contributor

@keiz1213
レビュー依頼ありがとうございます!!
本日中に確認させていただきます〜

fill_in 'hibernation[scheduled_return_on]', with: Time.current.next_month
fill_in 'hibernation[reason]', with: 'テストのため'
click_button '休会する'
page.driver.browser.switch_to.alert.accept
Copy link
Contributor

@yuma-matsui yuma-matsui Sep 13, 2022

Choose a reason for hiding this comment

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

ダイアログの確認についてはCapybaraのaccept_confirmメソッドを使った方が良さそうです。

Capybaraで確認ダイアログを操作する場合は page.accept_confirm を使う

accept_confirm do
 click_button '休会する'
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

確かにこちらのほうがスッキリ書けてわかりやすいです!スッキリ書けるだけではなく他にも色々メリットがあるのですね💡勉強になります。こちら修正しました。

visit_with_auth notifications_path, 'komagata'
find('#notifications.loaded', wait: 10)
within first('.card-list-item') do
assert_no_selector '.card-list-item-title__link-label', text: ' kimuraさんが休会しました。'
Copy link
Contributor

@yuma-matsui yuma-matsui Sep 13, 2022

Choose a reason for hiding this comment

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

text: ' kimuraさんが休会しました。'
こちら余計なspaceが含まれてしまっているかもです。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

全然気づきませんでした😅こちら修正しました。

visit_with_auth notifications_path, 'komagata'
find('#notifications.loaded', wait: 10)
within first('.card-list-item') do
assert_no_selector '.card-list-item-title__link-label', text: ' kensyuさんが休会しました。'
Copy link
Contributor

@yuma-matsui yuma-matsui Sep 13, 2022

Choose a reason for hiding this comment

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

同上です。

fill_in 'hibernation[scheduled_return_on]', with: Time.current.next_month
fill_in 'hibernation[reason]', with: 'テストのため'
click_button '休会する'
page.driver.browser.switch_to.alert.accept
Copy link
Contributor

Choose a reason for hiding this comment

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

同上です。

visit_with_auth notifications_path, 'komagata'
find('#notifications.loaded', wait: 10)
within first('.card-list-item') do
assert_no_selector '.card-list-item-title__link-label', text: ' senpaiさんが休会しました。'
Copy link
Contributor

Choose a reason for hiding this comment

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

同上です。

fill_in 'hibernation[scheduled_return_on]', with: Time.current.next_month
fill_in 'hibernation[reason]', with: 'テストのため'
click_button '休会する'
page.driver.browser.switch_to.alert.accept
Copy link
Contributor

Choose a reason for hiding this comment

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

同上です。

Copy link
Contributor

@yuma-matsui yuma-matsui left a comment

Choose a reason for hiding this comment

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

@keiz1213
お疲れ様です。
いくつかコメントしました!

ご確認をお願いします。

@keiz1213
Copy link
Contributor Author

@yuma-matsui
確認ありがとうございます!指摘頂いた点を修正しました。ご確認お願いいたします🙏

Copy link
Contributor

@yuma-matsui yuma-matsui left a comment

Choose a reason for hiding this comment

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

修正ありがとうございます!!
LGTMです🙆‍♂️

@keiz1213
Copy link
Contributor Author

@komagata
こちらメンバーの方からapproveいただきましたので確認お願いいたします🙏

@keiz1213 keiz1213 requested a review from komagata September 13, 2022 13:21
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です〜🙆‍♂️

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.

3 participants