提出物を更新した時の通知の文面を変更した #6772

Kassy0220 · 2023-07-30T09:16:58Z

Issue

提出物を作成したときと更新したときで通知の文面を変えたい #6647

概要

以下の2つの通知に関して、提出物が更新された場合に飛ぶ通知の文面が〇〇さんが「×××」の提出物を提出しました。となっていたのを、〇〇さんの提出物が更新されましたとなるよう変更しました。

提出物が作成or更新された際に、そのプラクティスをwatchしているメンターに飛ぶ通知
企業の研修生が提出物を作成or更新した際に、同じ企業に属しているアドバイザーに飛ぶ通知

変更確認方法

最初に、次の下準備をお願いします。

feature/change-notification-message-when-product-is-updatedをローカルに取り込む
foreman start -f Procfile.devでサーバを起動
komagataでログイン
プラクティス一覧ページを開き、「Terminalの基礎を覚える」プラクティスを選択。

プラクティスをWatchする。

提出物を提出し、更新した場合

kensyuでログイン
「Terminalの基礎を覚える」プラクティスの提出物を提出する
senpaiでログインし、kensyuさんが「Terminalの基礎を覚える」の提出物を提出しましたという通知が来ていることを確認する
komagataでログインし、kensyuさんが「Terminalの基礎を覚える」の提出物を提出しましたという通知が来ていることを確認する
kensyuでログインし、2で提出した提出物を再提出する
senpaiでログインし、kensyuさんの提出物が更新されましたという通知が来ていることを確認する
komagataでログインし、kensyuさんの提出物が更新されましたという通知が来ていることを確認する

7まで確認が終わったら、次の確認に備えて2で作成した提出物を削除します。

最初に提出物をWIPで保存し、提出した場合

kensyuでログイン
「Terminalの基礎を覚える」プラクティスの提出物を、WIPで保存する
2で保存した提出物を、提出する
senpaiでログインし、kensyuさんが「Terminalの基礎を覚える」の提出物を提出しましたという通知が来ていることを確認する
komagataでログインし、kensyuさんが「Terminalの基礎を覚える」の提出物を提出しましたという通知が来ていることを確認する

5まで確認が終わったら、次の確認に備えて2で保存した提出物を削除します。

提出物を提出し、WIPで保存し、更新した場合

kensyuでログイン
「Terminalの基礎を覚える」プラクティスの提出物を提出する
senpaiでログインし、kensyuさんが「Terminalの基礎を覚える」の提出物を提出しましたという通知が来ていることを確認する
komagataでログインし、kensyuさんが「Terminalの基礎を覚える」の提出物を提出しましたという通知が来ていることを確認する
kensyuでログインし、2で提出した提出物を、WIPで保存する。
5で保存した提出物を、提出する。
senpaiでログインし、kensyuさんの提出物が更新されましたという通知が来ていることを確認する
komagataでログインし、kensyuさんの提出物が更新されましたという通知が来ていることを確認する

Screenshot

変更前

プラクティスをWatchしているメンターに飛ぶ通知

同じ企業に所属しているアドバイザーに飛ぶ通知

変更後

プラクティスをWatchしているメンターに飛ぶ通知

同じ企業に所属しているアドバイザーに飛ぶ通知

Kassy0220 · 2023-07-31T02:28:51Z

@rira100000000

お疲れ様です🍵
こちらのPRのレビューをお願いしたいのですが、よろしいでしょうか？

急ぎではありませんので、よろしくお願いいたします🙏

rira100000000 · 2023-07-31T11:26:10Z

@Kassy0220
レビュー依頼ありがとうございます！承知しました！
1週間以内にはできるかと思いますので、お待ちください🙏

rira100000000

遅くなってしまいすいませんでした！
1点だけコメントさせていただきました。

細かいことなんですが、PRのタイトルが「提出物を更新した時の…」の方が適当かと思いました。
よろしくお願いします🙏

rira100000000 · 2023-08-05T13:22:10Z

app/models/product_notifier_for_colleague.rb

@@ -24,8 +24,9 @@ def notify_advisers(product)
  end

  def send_notification(product:, receivers:)
+    notification = product.updated_after_submission? ? :product_update : :submitted


こことproduct_notifier_for_practice_watcher.rbに追加されてる同処理はあえて重複させているのでしょうか？
例えばproduct.rbにnotification_typeみたいなメソッドを作ってそこで判断させれば、notifyの()内で直接呼び出せ、DRYにできる気がしました。
あえて重複させている理由があれば教えてください～🙏

この箇所は重複させている意味はなく、私の見落としになります💦
通知のタイプを判断するメソッドを追加しました。

def notification_type updated_after_submission? ? :product_update : :submitted end

notification_typeで通知の種類を提供することで、通知を送る側(product_notifier_for_practice_watcher.rbなど)はそれを利用すれば良く、役割が分かりやすくなりました。
テストコードも何をテストしているのか分かりやすくなったので、コードがよりよくなったと思います🙌

Kassy0220 · 2023-08-05T22:35:14Z

@rira100000000
レビューありがとうございます！

ご指摘の箇所(PRのタイトルとコードの重複)を修正しましたので、ご確認をよろしくお願いいたします🙏

rira100000000

修正ありがとうございます！
テストもわかりやすくなっていい感じですね！
approveとさせていただきます～。

Kassy0220 · 2023-08-06T07:28:19Z

@rira100000000
レビューありがとうございました！とても参考になりました🙌

@komagata
チームメンバーのレビューが通ったため、レビューをお願いいたします🙇🏻

komagata

テストが落ちているようです。

テストが成功してからのレビュー依頼をお願いします。

Kassy0220 · 2023-08-16T22:04:31Z

@komagata
こちらテストが通りましたので、ご確認をよろしくお願いいたします🙏

komagata · 2023-08-22T23:24:55Z

app/models/product.rb

@@ -196,4 +196,14 @@ def delete_last_commented_at
  def delete_commented_at
    update_commented_at(comments.last)
  end
+
+  def notification_type


komagata · 2023-08-22T23:26:24Z

test/models/product_test.rb

@@ -159,4 +159,35 @@ class ProductTest < ActiveSupport::TestCase
    product_id_list = Product.self_assigned_no_replied_products(mentor.id).pluck(:id)
    assert_not_includes product_id_list, no_replied_wip_product.id
  end
+
+  test '#notification_type return :product_update when product is updated after submission' do


モデルのテストなので下記のように単純にメソッドに対応したテストで良いかもです。

test '#notification_type' do # ... end test '#updated_after_submission?' do # ... end

（すでにある既存のテストもその方が望ましいですがレビューで漏れていたようです。そのうち直したい…）

次のコミットでテスト名を修正し、2つに分けていた#notification_typeのテストを一つにまとめました。
101a5aa

また、次のコミットで#updated_after_submission?のテストを追加しました。
1ea6a37

既存のテスト名が好ましくない箇所を修正するIssueも作成しました。
Productモデルのテスト名を修正したい · Issue #6835 · fjordllc/bootcamp

修正もできるだけ早く行いますので、よろしくお願いします🙏

Kassy0220 · 2023-08-28T10:45:29Z

@komagata
ご指摘の箇所の修正を行い、漏れていたテストを追加しましたので、ご確認をよろしくお願いいたします🙏

komagata

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

Kassy0220 self-assigned this Jul 31, 2023

Kassy0220 marked this pull request as ready for review July 31, 2023 02:29

Kassy0220 requested a review from rira100000000 July 31, 2023 02:29

Kassy0220 force-pushed the feature/change-notification-message-when-product-is-updated branch 4 times, most recently from 7ddc292 to 9d3dbc7 Compare August 4, 2023 22:22

rira100000000 reviewed Aug 5, 2023

View reviewed changes

Kassy0220 changed the title ~~提出を更新した時の通知の文面を変更した~~ 提出物を更新した時の通知の文面を変更した Aug 5, 2023

rira100000000 approved these changes Aug 6, 2023

View reviewed changes

Kassy0220 requested a review from komagata August 6, 2023 07:28

Kassy0220 force-pushed the feature/change-notification-message-when-product-is-updated branch 2 times, most recently from a777540 to 58f405b Compare August 11, 2023 22:01

komagata reviewed Aug 15, 2023

View reviewed changes

Kassy0220 force-pushed the feature/change-notification-message-when-product-is-updated branch from 58f405b to 0f99e33 Compare August 16, 2023 09:32

Kassy0220 force-pushed the feature/change-notification-message-when-product-is-updated branch from 0f99e33 to ffba731 Compare August 18, 2023 09:46

komagata reviewed Aug 22, 2023

View reviewed changes

Kassy0220 force-pushed the feature/change-notification-message-when-product-is-updated branch from ffba731 to 5001ab6 Compare August 23, 2023 09:00

Kassy0220 added 7 commits August 28, 2023 07:58

提出物が提出後に更新されたかを確認するメソッドをProductモデルに追加

103d62f

追加したメソッドに対するテストを追加

a2dfbde

提出物を更新時に飛ぶ通知の文面を、「XXさんの提出物が更新されました」に変更

777cdbb

重複していたコードを修正

1301cd3

テストを修正

9749fbf

Product#notification_type のテストを修正

101a5aa

Product#updated_after_submission?のテストを追加

1ea6a37

Kassy0220 force-pushed the feature/change-notification-message-when-product-is-updated branch from 35ebdd6 to 1ea6a37 Compare August 27, 2023 22:58

Kassy0220 mentioned this pull request Aug 28, 2023

Productモデルのテスト名を修正したい #6835

Closed

komagata approved these changes Aug 29, 2023

View reviewed changes

komagata merged commit 03fb229 into main Aug 29, 2023
3 checks passed

komagata deleted the feature/change-notification-message-when-product-is-updated branch August 29, 2023 15:29

github-actions bot mentioned this pull request Aug 29, 2023

Release 2023-08-31 04:35:29 #6841

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

提出物を更新した時の通知の文面を変更した #6772

提出物を更新した時の通知の文面を変更した #6772

Kassy0220 commented Jul 30, 2023 •

edited

Loading

Kassy0220 commented Jul 31, 2023

rira100000000 commented Jul 31, 2023

rira100000000 left a comment

rira100000000 Aug 5, 2023

Kassy0220 Aug 5, 2023

Kassy0220 commented Aug 5, 2023

rira100000000 left a comment

Kassy0220 commented Aug 6, 2023

komagata left a comment

Kassy0220 commented Aug 16, 2023

komagata Aug 22, 2023

komagata Aug 22, 2023

Kassy0220 Aug 28, 2023

Kassy0220 commented Aug 28, 2023

komagata left a comment

提出物を更新した時の通知の文面を変更した #6772

提出物を更新した時の通知の文面を変更した #6772

Conversation

Kassy0220 commented Jul 30, 2023 • edited Loading

Issue

概要

変更確認方法

提出物を提出し、更新した場合

最初に提出物をWIPで保存し、提出した場合

提出物を提出し、WIPで保存し、更新した場合

Screenshot

変更前

変更後

Kassy0220 commented Jul 31, 2023

rira100000000 commented Jul 31, 2023

rira100000000 left a comment

Choose a reason for hiding this comment

rira100000000 Aug 5, 2023

Choose a reason for hiding this comment

Kassy0220 Aug 5, 2023

Choose a reason for hiding this comment

Kassy0220 commented Aug 5, 2023

rira100000000 left a comment

Choose a reason for hiding this comment

Kassy0220 commented Aug 6, 2023

komagata left a comment

Choose a reason for hiding this comment

Kassy0220 commented Aug 16, 2023

komagata Aug 22, 2023

Choose a reason for hiding this comment

komagata Aug 22, 2023

Choose a reason for hiding this comment

Kassy0220 Aug 28, 2023

Choose a reason for hiding this comment

Kassy0220 commented Aug 28, 2023

komagata left a comment

Choose a reason for hiding this comment

Kassy0220 commented Jul 30, 2023 •

edited

Loading