通知ページの「全て」以外のタブに未読数を表示する #4027

Aseiide · 2022-01-24T09:23:33Z

通知ページの全てのタブに未読数を表示したい #3907

実装内容

通知ページのタブに未読数を表示するようにしました。
「全て」のタブに関しては、 #3590 で既に実装されておりましたので、自分は「全て」以外のタブに未読数を表示するように変更しました。

変更前

再現手順

アカウント: muryou でログイン
通知ページ/notificationsにアクセス
コメントに未読が1件あるのに、タブに未読数がついていない

変更後

再現手順

アカウント: muryou でログイン
通知ページ/notificationsにアクセス
コメントに未読数が1件で、タブにも1と表示できている

他のアカウントでも確認

アカウント: senpai

1/25 14:53追記
以下に再現手順を示します。

anyloginを使ってsenpaiアカウントでログイン
通知ページ/notificationsにアクセス
↑のスクリーンショットと同じ画面で、各タブに未読数が表示されていることが確認できると思います。

レビューしてほしい点

実装したものの、テストについて以下の点が気になっているのでレビューしていただけますと幸いです。

現在書いているテストでは、未読数に該当するテストのhtml要素が存在するか、しないか、ということでテストを記述しています。
全てを含めて、タブは7個あり、その全てについてテストができていない状態です。
このテストでいいのか、別の書き方をしたほうがいいのか、わからなかったです。

その他: PR#3947がcloseしてる件について

issue着手時に実装方針がわからず、急ぎで相談したときに#3947を作成しました。ペアプロ時に別のブランチを切って実装を進めてしまったため、#3947はcloseしています。

Aseiide · 2022-01-25T03:48:01Z

test/system/notification/tabs_badges_test.rb

+
+class Notification::WatchesTest < ApplicationSystemTestCase
+  test 'タブに未読数が表示されていること' do
+    visit_with_auth '/notifications', 'sotugyou'


アカウントをsotugyouにしているのは通知が存在するのがsotugyouだったためです。

Aseiide · 2022-01-25T03:48:40Z

test/system/notification/tabs_badges_test.rb

+  end
+
+  test 'タブに未読数が表示されていないこと' do
+    visit_with_auth '/notifications', 'machida'


アカウントをmachidaにしているのは、テスト時点で通知が存在しないアカウントがmachidaだったためです

Aseiide · 2022-01-25T03:50:21Z

@naomichi-h
こちら、レビューお願いいたします。できる限りdescriptionで説明しているつもりなのですが、わからない点がございましたらメンションください。

naomichi-h · 2022-01-25T04:50:37Z

@Aseiide レビュー依頼ありがとうございます。
現在動作確認中で、muryouアカウントに関しては再現できたのですが、senpaiアカウントに関しては、スクリーンショットと同じ通知画面になりませんでした。
descriptionにある、「Senpaiアカウントでも確認済」に関して、具体的にどういった手順で確認を行ったのか教えていただけますか？

Aseiide · 2022-01-25T05:35:14Z

@naomichi-h
早速の対応、ありがとうございます。出先なので夕方までには再現手順を追記します。
少しお待ち下さい！

Aseiide · 2022-01-25T05:56:51Z

@naomichi-h
PRのDescriptionに再現手順追記いたしました。
以下、Descriptionと同じ文面です

1/25 14:53追記
以下に再現手順を示します。

anyloginを使ってsenpaiアカウントでログイン
通知ページ/notificationsにアクセス
↑のスクリーンショットと同じ画面で、各タブに未読数が表示されていることが確認できると思います。

よろしくお願いいたします。

naomichi-h · 2022-01-25T06:04:30Z

@Aseiide rails db:seedしてから追記していただいた再現手順を試したのですが、同じ画面にならないですね...。
一度、rails db:seedしてから同じ画面になるか試していただけませんか？

Aseiide · 2022-01-25T06:12:49Z

@naomichi-h
承知しました。 rails db:seed からやってみます。
1点思い当たる点としては、動作確認をするために、わざと通知が飛ぶようにする必要がありました。
自分が試したのは、いろいろなアカウントでログインしてsenpaiにメンションやコメントを飛ばすという手段を取っていました。テストデータなどは作っていなかったです。
なので、senpaiアカウントに関しては自分の環境だけでしか再現できていないのかもしれないです、、

Aseiide · 2022-01-25T06:16:33Z

@naomichi-h
自分もbin/rails db:seed を実行してからsenpaiにログインしたところ、通知が1に戻っていました。
通知が溜まっているアカウントがなく、一つのアカウントだけで動作確認をするのは良くないかなーと思い他のアカウントでの動作確認を試みたのですが、このような場合ってどのようにしたら良いでしょうか？

naomichi-h · 2022-01-25T06:18:36Z

test/system/notification/tabs_badges_test.rb

+require 'application_system_test_case'
+
+class Notification::WatchesTest < ApplicationSystemTestCase
+  test 'タブに未読数が表示されていること' do


現状のテストでは、「各タブのバッジが動作しているか」を確かめるには不十分かと思われます。
また、テスト名は、他のテストに習い、英語で書くのがよいと思います。
テストの方針ですが、それぞれのタブについてテストを書くのはいかがでしょうか？

例としては、コメントタブであれば、

test 'show comment badge count' do visit_with_auth '/notifications', 'sotugyou' assert_selector "ul.page-tabs__items li.page-tabs__item:nth-child(4) a.page-tabs__item-link div.page-tabs__item-count ", text: "1" end test 'not show comment badge count' do visit_with_auth '/notifications', 'machida' assert_no_selector "ul.page-tabs__items li.page-tabs__item:nth-child(4) a.page-tabs__item-link div.page-tabs__item-count " end

のような感じです。
また、バッジの増減は、以下のようなモデル.createを間に挟めば簡単に表現できるので、これを使えば、適したuserを作成しなくても、その他のタブのテストができると思います。

Notification.create( kind: 7, user: @komagata, sender: @kimura, message: "#{@kimura.login_name}さんがはじめての日報を書きました！", link: "/reports/#{report.id}", read: false )

テスト名は、他のテストに習い、英語で書くのがよいと思います。

そうですね、こちらは英語に変更して対応します。

テストの方針ですが、それぞれのタブについてテストを書くのはいかがでしょうか？

自分がそれぞれのタブに関するテストを書かなかったのは、そこまでしてテストする必要があるか疑問だったからです。
それぞれのタブについて、通知あり/なしで2通りあり、それがタブ7個分なので14個(現実的ではないですが、仮に全てのパターンに対応するなら2^7=128通り)似たようなテストが並ぶほどまでテストせずとも、ある1個の要素のあるなしがわかればテストとしては十分ではないかと考えたからです。
自分はチーム開発に入ったばかりなので、チーム全体としてどういう方針なのかわからず、テスト対象を1個だけとしました。
実装として未読のバッジが実装されているのがQ&Aや通知マークなどありましたが、テストが存在せず、どのようにしたらよいかわからずレビューをお願いしたという経緯でした。

また、バッジの増減は、以下のようなモデル.createを間に挟めば簡単に表現できるので、これを使えば、適したuserを作成しなくても、その他のタブのテストができると思います。

自分がbootcampのリポジトリを把握していないのですが、この次に書いてあるNotification.createはfixturesに書くということですかね？
どのファイルに書けばよいかわからなかったので教えていただけると嬉しいです。

なるほどです。テストとして十分かどうか、そもそもテストが必要なのかは、私では判断できかねますので、一度駒形さんに伺ってみてはいかがでしょうか。

提案したNotidication.createはテスト内に書く内容です。

bootcamp/test/system/reports_test.rb

Line 449 in fbbf4c1

test 'delete report with notification' do

にあるような記述です。

naomichi-h · 2022-01-25T06:25:42Z

@Aseiide

自分もbin/rails db:seed を実行してからsenpaiにログインしたところ、通知が1に戻っていました。
通知が溜まっているアカウントがなく、一つのアカウントだけで動作確認をするのは良くないかなーと思い他のアカウントでの動作確認を試みたのですが、このような場合ってどのようにしたら良いでしょうか？

確認のためのテストデータが欲しいのであれば、seedファイルを書くのが一つの方法だと思います。
bootcampでは、db/fixtures内にまとめられています。

testに関してコメントしましたので、確認をお願いいたします。

Aseiide · 2022-01-25T07:03:57Z

@naomichi-h

レビューに手間を取らせてしまってすみません、、。

テストとして十分かどうか、そもそもテストが必要なのかは、私では判断できかねますので、一度駒形さんに伺ってみてはいかがでしょうか。

テストに関しては、次のレビューでkomagataさんに聞いて指示を仰ぎたいと思います。

次のような流れで行きたいと思います

テストの内容が日本語になっているので英語に修正
改めてnaomichiさんにレビュー依頼
naomichiさんのレビューをパスしたらkomagataさんにレビュー依頼

今日中に修正ができるか微妙で、明日までには修正してレビュー依頼送らせてください。もう少しだけレビューにお付き合いくださいませ。このような流れで大丈夫でしょうか？

naomichi-h · 2022-01-25T07:13:01Z

@Aseiide いえいえ、全然問題ないですよ！
そうですね。まず最初に、discordか、もしくはここでメンション付けて駒形さんにtestについての指針を伺うのが良いと思います。
その後はその流れで良いかと思います〜。

Aseiide · 2022-01-25T09:09:12Z

@komagata
テストの方針について相談です。
現段階では、タブの1つについて未読数が表示されるか/されないかという簡単なテストにとどめてあるのですが、7個の全てのタブについて、表示されるか/されないかテストを書いたほうが良いでしょうか?

Aseiide · 2022-01-26T05:15:56Z

test/system/notification/tabs_badges_test.rb

@@ -3,15 +3,15 @@
 require 'application_system_test_case'

 class Notification::WatchesTest < ApplicationSystemTestCase
-  test 'タブに未読数が表示されていること' do
+  test 'unreads count are displayed on the tab' do


テスト内容の表記を英語にしました

Aseiide · 2022-01-26T05:16:01Z

test/system/notification/tabs_badges_test.rb

    visit_with_auth '/notifications', 'sotugyou'

    assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(4) > a > div' do
      assert_selector 'div.page-tabs__item-count.a-notification-count'
    end
  end

-  test 'タブに未読数が表示されていないこと' do
+  test 'unreads count are not displayed on the tab' do


テスト内容の表記を英語にしました

Aseiide · 2022-01-26T05:16:44Z

test/system/notification/tabs_badges_test.rb

@@ -2,7 +2,7 @@

 require 'application_system_test_case'

-class Notification::WatchesTest < ApplicationSystemTestCase
+class Notification::TabsBadgesTest < ApplicationSystemTestCase


他のテストからコピペしたときにクラス名が間違っていたので修正しました

Aseiide · 2022-01-26T05:18:20Z

@naomichi-h
以下2点修正しましたのでレビューお願いいたします。

テスト内容を日本語から英語に変える
テストのクラス名が間違っていた

komagata · 2022-01-26T05:38:43Z

@Aseiide

現段階では、タブの1つについて未読数が表示されるか/されないかという簡単なテストにとどめてあるのですが、7個の全てのタブについて、表示されるか/されないかテストを書いたほうが良いでしょうか?

YESです。

Aseiide · 2022-01-26T05:43:42Z

@komagata
了解です。7個のタブすべてについてテストを書き直します。

naomichi-h · 2022-01-26T05:49:50Z

@Aseiide テストを書き終えたらまたお声かけください〜

Aseiide · 2022-01-26T09:05:48Z

残りのタスク

watch中の未読数ありのテストを書く
フォロー中の未読数ありのテストを書く
テスト内容が日本語なのを英語に直す
コンフリクトを解消する

Aseiide · 2022-01-30T04:50:27Z

memo
コンフリクトしているので明日ペアプロ予定

Aseiide · 2022-01-31T12:37:58Z

@naomichi-h

現状の実装

PR #3642 が先にマージされたことによって_tabs.html.slimでコンフリクトを起こしていたので本日解消いたしました。
現状の実装としては、メンションタブについては、キャッシュを使って全件数と未読を表示しています。こちらは @AudioStakes さんによってPR#3642 で実装済です。

「全て」と「メンション」タブ以外については、このPRで実装しており、キャッシュは使っていません。
キャッシュを使っていない点については、MTG時に共有済です。

レビューしてほしい点

実装する過程で新たにレビューしてほしい点がありましたので意見を頂きたいです。
テストファイルについてです。
自分はtest/system/notification/tabs_badges_test.rbというファイルを新たに作ってテストを書いたのですが、test/system/notifications_test.rbというテストファイルがありました。
どっちに書くのが良いかわからなかったのでアドバイスいただけると嬉しいです。

レビューお願いいたします。

naomichi-h · 2022-01-31T14:26:34Z

test/system/notification/tabs_badges_test.rb

+require 'application_system_test_case'
+
+class Notification::TabsBadgesTest < ApplicationSystemTestCase
+  test 'unread badges are displayed' do


現状のテストでは、「同一ユーザーのステータスが変化したときに、バッジが変化するかどうか」がテストできていないので、タブのバッジ機能を十分テストできていないのかなと思います。

また、assert_selectorにはHTMLのタグを渡して場所を指定するのではなく、classを渡したほうが場所が明確になり、テストがエラーになった際の作業がやりやすくなると思います。

私は、バッジの増をモデル.createを使用して確認し、減は未読の項目をクリックして確認するテストを追加した方が良いと思いますが、あくまでも提案なので、他に方法があればそちらでも良いと思います。

@naomichi-h レビューありがとうございます。

現状のテストでは、「同一ユーザーのステータスが変化したときに、バッジが変化するかどうか」がテストできていない

それぞれのテストで、ログインするユーザーが違うということですかね?
自分もテストを書いていて気になっていた点ではあるので、sotugyouに合わせてバッジ無しのテストを書き直したいと思います。その際は未読の項目をクリックして確認する方法を取りたいと思います。

assert_selectorにはHTMLのタグを渡して場所を指定するのではなく、classを渡したほうが場所が明確になり、テストがエラーになった際の作業がやりやすくなると思います。

今のテストコードで、どの場所をテストしているか分かりづらい、というのは自分も思っていました。
7つのタブには全て同じclassが付与されています。class="page-tabs__item-count a-notification-count"
全て同じclassなので、classを渡した場合、「各タブについて個別にテストする」ということができなくなると思って今の書き方にしています。
自分の知識不足なのですが、同名のclassでも別々にテストする方法があるということでしょうか?
この点わからなかったのでご教示いただけると幸いです。

それぞれのテストで、ログインするユーザーが違うということですかね?

「ログインしているユーザーのステータスが変化したときに、ちゃんとバッジが予期している仕様どおりに変化するかどうか？」がテストされていないことを問題にしています。

例えば、

bootcamp/test/system/notifications_test.rb

Line 174 in 4d0eaae

test 'show notification count' do

では、通知バッジが増えるかどうかをテストしていますし、

bootcamp/test/system/notifications_test.rb

Line 358 in 4d0eaae

test 'don\'t show the badge on the mentioned tab if no unread mentions' do

では、通知バッジが消えるかどうかをテストしています。

今回のテストにも、こういったテストが必要かなと思います。

自分の知識不足なのですが、同名のclassでも別々にテストする方法があるということでしょうか?

bootcamp/test/system/reports_test.rb

Line 345 in 4d0eaae

assert_selector('ul.learning-times__items li.learning-times__item:nth-child(1)', text: '07:30 〜 08:30')

にあるような感じで別々に指定できないですかね？

@naomichi-h ありがとうございます。頂いたご指摘をもとにテストについては書き直します。

naomichi-h · 2022-01-31T14:27:49Z

@Aseiide コメントしましたので確認おねがいいたします🙏
テストを書く場所については、今の実装で良いと思います。

Aseiide · 2022-02-02T12:30:24Z

@naomichi-h
通知を増やす処理と既読にして要素がないか、ということを検証するように変更しました。
各タブを指定している、assert_selector assert_no_selector はいろいろ書き方を試してみたのですが、自分では今の書き方以外でパスする書き方がわかりませんでした、申し訳ないです。

レビューお願いいたします。

naomichi-h · 2022-02-03T05:51:55Z

@Aseiide testの動作はokだと思いますー。記述方法についてコメントしたので、よかったら参考にしてみてください。
あくまでも提案ですので、現状の書き方で問題ないと思われるなら僕のレビューはokとして、駒形さんのレビューにうつってくださいー。

naomichi-h · 2022-02-03T05:45:05Z

test/system/notification/tabs_badges_test.rb

+    assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(1) > a > div' do
+      assert_selector 'div.page-tabs__item-count.a-notification-count'
+    end
+    assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(2) > a > div' do
+      assert_selector 'div.page-tabs__item-count.a-notification-count'
+    end
+    assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(3) > a > div' do
+      assert_selector 'div.page-tabs__item-count.a-notification-count'
+    end
+    assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(4) > a > div' do
+      assert_selector 'div.page-tabs__item-count.a-notification-count'
+    end
+    assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(5) > a > div' do
+      assert_selector 'div.page-tabs__item-count.a-notification-count'
+    end
+    assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(6) > a > div' do
+      assert_selector 'div.page-tabs__item-count.a-notification-count'
+    end
+    assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(7) > a > div' do
+      assert_selector 'div.page-tabs__item-count.a-notification-count'
+    end


Suggested change

assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(1) > a > div' do

assert_selector 'div.page-tabs__item-count.a-notification-count'

end

assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(2) > a > div' do

assert_selector 'div.page-tabs__item-count.a-notification-count'

end

assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(3) > a > div' do

assert_selector 'div.page-tabs__item-count.a-notification-count'

end

assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(4) > a > div' do

assert_selector 'div.page-tabs__item-count.a-notification-count'

end

assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(5) > a > div' do

assert_selector 'div.page-tabs__item-count.a-notification-count'

end

assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(6) > a > div' do

assert_selector 'div.page-tabs__item-count.a-notification-count'

end

assert_selector '#body > div.wrapper > main > div > div > ul > li:nth-child(7) > a > div' do

assert_selector 'div.page-tabs__item-count.a-notification-count'

end

within all('.page-tabs__item')[0] do

assert_selector '.page-tabs__item-count'

end

within all('.page-tabs__item')[1] do

assert_selector '.page-tabs__item-count'

end

within all('.page-tabs__item')[2] do

assert_selector '.page-tabs__item-count'

end

within all('.page-tabs__item')[3] do

assert_selector '.page-tabs__item-count'

end

within all('.page-tabs__item')[4] do

assert_selector '.page-tabs__item-count'

end

within all('.page-tabs__item')[5] do

assert_selector '.page-tabs__item-count'

end

within all('.page-tabs__item')[6] do

assert_selector '.page-tabs__item-count'

end

とするのはどうでしょう？伊藤さんのこの記事を参考に書いてみました。
unread badges are not displayedの方も同様にして書けます。

Aseiide · 2022-02-04T08:00:34Z

@naomichi-h
ご提案のように、テストのマッチャを書き換えました。
こちらの方がスッキリして読みやすいですね。
レビューお願いいたします。

naomichi-h · 2022-02-04T10:08:45Z

@Aseiide 確認しました。okですー。

Aseiide · 2022-02-04T11:24:18Z

@komagata
こちらお手すきでレビューお願いします。

komagata · 2022-02-05T07:37:22Z

app/views/notifications/_tabs.html.slim

@@ -17,3 +17,6 @@
                  = Cache.mentioned_and_unread_notification_count(current_user)
            - else
              = t("notification.#{target}")
+              - if current_user.notifications.by_target(target).unreads.latest_of_each_link.size.positive?


長いので分かりやすい名前をつけてViewHelperに追い出した方がいいかもですね。

@komagata
2点確認したいのですが、

「ViewHelperに追い出した方がいい」とのことですが、こちらはapp/helpers/notification_helperのような形で新しくファイルを作って良いでしょうか?

切り出すとしたら、current_user.notifications.by_target(target).unreads.latest_of_each_link.size.positive?をensure_notifications? (メソッド名は仮で付けました)のような形で丸ごと短くする、というふうに考えたのですが、この方針でよいでしょうか?

よろしくお願いします。

質問タイムメモ

app/helpersにヘルパーファイルを作ってOK

current_user.notifications.by_target(target).unreads.latest_of_each_link.size.positive?このメソッドごとヘルパーに切り出す

komagata · 2022-02-05T07:37:50Z

test/system/notification/tabs_badges_test.rb

+      read: false
+    )
+  end
+  test 'unread badges are displayed' do


上に1行開けた方がいいかも？

komagata · 2022-02-05T07:38:04Z

test/system/notification/tabs_badges_test.rb

+  test 'unread badges are displayed' do
+    visit_with_auth '/notifications', 'sotugyou'
+
+    within all('.page-tabs__item')[0] do


allを使わずfirstを使い、CSS Selectorに:nth-child(0)などを使った方がいいかもです〜。

allと[0]だと指定した要素が見つからないというエラーではなくて、配列の要素が見つからないというエラーになるからです。

もしくは、タブのHTMLの方に分かりやすいIDかクラスをつけるのもいいかもです〜

空行を追加 firstを使ってテストを書き直したメソッドをヘルパーに切り出した

Aseiide · 2022-02-08T13:40:05Z

app/views/notifications/_tabs.html.slim

@@ -17,3 +17,6 @@
                  = Cache.mentioned_and_unread_notification_count(current_user)
            - else
              = t("notification.#{target}")
+              - if ensure_notifications?(target)


メソッドが長かったのでapp/helpers/notification_helper.rbに切り出しました。

Aseiide

@komagata
レビューありがとうございます。

システムテストの修正
メソッドのヘルパーへの切り出し
2点対応しましたのでレビューお願いいたします。

Aseiide · 2022-02-08T13:47:03Z

test/system/notification/tabs_badges_test.rb

+    within first('.page-tabs__item:nth-child(1)') do
+      find('div', text: '10')


first と find を使って対象の要素があるかどうかを検証するようにしました

komagata

確認しました、OKですー🙆‍♂️

Aseiide mentioned this pull request Jan 24, 2022

通知ページのWatch中タブに未読数を表示したい #3947

Closed

Aseiide force-pushed the feature/display-unreads-numbers-on-notifications branch from 921772b to fbbf4c1 Compare January 24, 2022 14:07

Aseiide changed the title ~~[WIP]通知ページの全てのタブに未読数を表示する~~ [WIP]通知ページのタブに未読数を表示する Jan 25, 2022

Aseiide changed the title ~~[WIP]通知ページのタブに未読数を表示する~~ 通知ページの「全て」以外のタブに未読数を表示する Jan 25, 2022

Aseiide commented Jan 25, 2022

View reviewed changes

Aseiide requested a review from naomichi-h January 25, 2022 03:49

naomichi-h reviewed Jan 25, 2022

View reviewed changes

Aseiide marked this pull request as ready for review January 25, 2022 06:51

Aseiide commented Jan 26, 2022

View reviewed changes

Aseiide requested a review from naomichi-h January 26, 2022 05:18

Aseiide force-pushed the feature/display-unreads-numbers-on-notifications branch from 6ee6e86 to c837b07 Compare January 31, 2022 05:32

naomichi-h reviewed Jan 31, 2022

View reviewed changes

Aseiide force-pushed the feature/display-unreads-numbers-on-notifications branch from c837b07 to 319191a Compare February 2, 2022 12:03

machida assigned Aseiide Feb 2, 2022

naomichi-h reviewed Feb 3, 2022

View reviewed changes

Aseiide force-pushed the feature/display-unreads-numbers-on-notifications branch 2 times, most recently from 9493bd4 to 49a9941 Compare February 4, 2022 07:18

komagata requested changes Feb 5, 2022

View reviewed changes

Aseiide added 3 commits February 8, 2022 17:49

各タブに未読数を表示できるようにした

8197659

通知を既読にする処理を入れてテストを書いた

3e90ca0

マッチャの書き方を変更

3308b19

空行を追加 firstを使ってテストを書き直したメソッドをヘルパーに切り出した

Aseiide force-pushed the feature/display-unreads-numbers-on-notifications branch from 49a9941 to 3308b19 Compare February 8, 2022 08:52

rubocop修正

a23f46e

Aseiide commented Feb 8, 2022

View reviewed changes

komagata approved these changes Feb 9, 2022

View reviewed changes

komagata merged commit 941ccf2 into main Feb 9, 2022

komagata deleted the feature/display-unreads-numbers-on-notifications branch February 9, 2022 07:02

github-actions bot mentioned this pull request Feb 9, 2022

Release 2022-02-18 02:36:10 #4129

Merged

54 tasks

saeyama mentioned this pull request Mar 13, 2022

通知一覧のメンションの合計数を削除 #4390

Merged

		within first('.page-tabs__item:nth-child(1)') do
		find('div', text: '10')

通知ページの「全て」以外のタブに未読数を表示する #4027

通知ページの「全て」以外のタブに未読数を表示する #4027

Conversation

Aseiide commented Jan 24, 2022 • edited Loading

実装内容

変更前

変更後

他のアカウントでも確認

レビューしてほしい点

その他: PR#3947がcloseしてる件について

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aseiide commented Jan 25, 2022 • edited Loading

naomichi-h commented Jan 25, 2022

Aseiide commented Jan 25, 2022

Aseiide commented Jan 25, 2022

naomichi-h commented Jan 25, 2022

Aseiide commented Jan 25, 2022

Aseiide commented Jan 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naomichi-h commented Jan 25, 2022

Aseiide commented Jan 25, 2022

naomichi-h commented Jan 25, 2022 • edited Loading

Aseiide commented Jan 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aseiide commented Jan 26, 2022

komagata commented Jan 26, 2022

Aseiide commented Jan 26, 2022

naomichi-h commented Jan 26, 2022

Aseiide commented Jan 26, 2022 • edited Loading

Aseiide commented Jan 30, 2022

Aseiide commented Jan 31, 2022

現状の実装

レビューしてほしい点

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naomichi-h commented Jan 31, 2022

Aseiide commented Feb 2, 2022

naomichi-h commented Feb 3, 2022

Choose a reason for hiding this comment

Aseiide commented Feb 4, 2022

naomichi-h commented Feb 4, 2022

Aseiide commented Feb 4, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aseiide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

komagata left a comment

Choose a reason for hiding this comment

Aseiide commented Jan 24, 2022 •

edited

Loading

Aseiide commented Jan 25, 2022 •

edited

Loading

naomichi-h commented Jan 25, 2022 •

edited

Loading

Aseiide commented Jan 25, 2022 •

edited

Loading

Aseiide commented Jan 26, 2022 •

edited

Loading