[休会] ユーザーが休会したら、管理者、メンターに通知が欲しい

[machida]

Discordには通知しているので、サイト内通知とメール通知を追加したい。

休会の全機能（一部はリリースされている）と復帰機能はまだメインブランチに入ってないのでこちらのブランチからブランチを作って、PRもこちらのブランチに送ってください。

#5369

[keiz1213]

@komagata

お疲れさまです！3点質問があります。

circle ci について

質問

PRにmainにあるcircle ciの設定ファイルを追加してpushしようと思っているのですが問題ないでしょうか？

背景

PRにpushした後にcircle ciでbuild errorが出てしまいます。原因としてはこのブランチにはcircle ciの設定ファイルが無いからだと判断しました。

Discord通知について

質問

DiscordNotifierをNotificationFacadeから分離しようと考えてますがよろしいでしょうか？
通知関連は今後active_delivery でまとめるということなので念の為質問しました。

背景

メンターと管理者へ休会の通知が行くように、hibernation_controller内のnotify_to_mentors_and_adminsを使って実装しました。

# 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.hibernatedでActivityNotifierとDiscordNotifierを同時に呼び出しており、メンターと管理者の人数分だけDiscordNotifierが呼び出され同じ通知が複数discordの方に飛んでしまいました。

# notification_facade

  def self.hibernated(sender, receiver)
    ActivityNotifier.with(sender: sender, receiver: receiver).hibernated.notify_now
    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

なので、DiscordNotifierをNotificationFacadeから分離しようと考えてますが問題ないでしょうか？例えばこんな感じで実装しようと思います。

# notification_facade

  def self.hibernated(sender, receiver)
    ActivityNotifier.with(sender: sender, receiver: receiver).hibernated.notify_now
    # 削除 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

# 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

  def notify_to_chat
    DiscordNotifier.with(sender: current_user).hibernated.notify_now
  end

DiscordNotifierのwebhook_urlの取得方法について

質問

webhook_urlの取得方法は現状では次のようになっています。

  def hibernated(params = {})
    params.merge!(@params)
    webhook_url = params[:webhook_url] || Rails.application.secrets[:webhook][:admin]

    notification(
      body: "#{params[:sender].login_name}さんが休会しました。",
      name: 'ピヨルド',
      webhook_url: webhook_url
    )
  end

• Rails.application.secrets[:webhook][:admin]

を

• ENV['DISCORD_ALL_WEBHOOK_URL']

に変更しても問題ないでしょうか？

背景

開発環境でRails.application.secrets[:webhook][:admin]ではwebhook_urlを取得できず、エラーになるためです。

[komagata]

@keiz1213

PRにmainにあるcircle ciの設定ファイルを追加してpushしようと思っているのですが問題ないでしょうか？

mainの内容を反映させるには追加ではなく、mainブランチの最新からまたrebaseすれば大丈夫です。（これは開発時はしょっちゅうする作業になります）

$ git checkout main
$ git pull origin main
$ git checkout 作業してるブランチ
$ git rebase main

[keiz1213]

@komagata
回答ありがとうございます😄
今回はまだmainにマージされてないブランチへPRを作成したのでmainをリベースしていいものか判断がつきませんでした。回答いただいた通りmainをリベースしようと思います。

それと、上記の質問と同時に2つ質問させて頂きましたがそちらの方はいかがでしょうか？お時間がありましたらよろしくお願いします🙏

[komagata]

@keiz1213

DiscordNotifierをNotificationFacadeから分離しようと考えてますがよろしいでしょうか？

はい。

ENV['DISCORD_ALL_WEBHOOK_URL']
に変更しても問題ないでしょうか？

secretsの中で結局ENVから設定してた気がするので同じかな〜と思ったんですが、ちょっと調べてもらえるとありがたいです〜

[keiz1213]

@komagata
すみません。また質問があります。

今回のこのissueは#5369 からブランチを切って、PRを#5369 に送る流れになっているのですが、#5369 から作成したブランチにおいてmainの変更をgit pull --rebase origin mainで取り込んだところ大量の関係の無い差分が出てしまいました。

原因としては#5369 が約1ヶ月ほどmainの変更を取り込んでないためと思います。なので解決策として、#5369 の方でmainを取り込んでもらって、そのブランチを自分のブランチに取り込むことで不要な差分が出なくなると思うのですがその方向性で間違ってないでしょうか？よろしくおねがいします。

[komagata]

@keiz1213

原因としては#5369 が約1ヶ月ほどmainの変更を取り込んでないためと思います。なので解決策として、#5369 の方でmainを取り込んでもらって、そのブランチを自分のブランチに取り込むことで不要な差分が出なくなると思うのですがその方向性で間違ってないでしょうか？

はい。

後で #5369 にmainの最新を取り込んでおきます〜。

[keiz1213]

@komagata
mainのリベースありがとうございます🙏不要な差分がなくなりました。

先日回答いただいたDiscordのwebhook_urlの件ですがsecretsから正常に取得できました。
自分がうまくいかなかったのは単純にシェルの設定ミスでした🙏

[keiz1213]

@komagata
こちらのissueはcloseしてもよろしいでしょうか？

[komagata]

@keiz1213 はい