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

提出物一覧ページの「未アサイン」タブのバッジに表示する件数を適切に増減させる #3795

Conversation

AudioStakes
Copy link
Contributor

@AudioStakes AudioStakes commented Dec 17, 2021

目的

以下の Issue の対応

Refs: #3646

やったこと

担当者が更新されたときに提出物の未アサイン件数のキャッシュを削除するようにした

以下2パターンで削除するようにした

  1. 未アサインの(担当のいない)提出物に担当がついたとき
  2. アサイン済みの(担当のいる)提出物から担当が外れたとき

未アサイン件数のキャッシュを削除する条件に「削除された提出物が『未アサイン』であること」を追加した

アサイン済み(担当のいる)の提出物が削除されたときは未アサイン件数は変わらないため、その場合はキャッシュを削除せず、「未アサイン」の場合のみキャッシュを削除するようにした。今までは「未アサイン」かどうかによらずキャッシュを削除していた。

テストを追加した

未アサイン件数のキャッシュが増減することを確認するテストを追加

動作確認

すべての確認項目に共通すること

  • $ rails dev:cacheを実行してキャッシュを有効にした状態で確認する
    • 詳細は下記の「動作確認をするときの注意事項」に記載
  • 確認対象は「未アサイン」タブのバッジに表示される件数(以下、未アサイン件数)
  • メンター ( komagata など)としてログインする
  • 操作の始めに提出物一覧ページの未アサイン件数を確認する

確認項目

  • 「未アサイン」の提出物の担当になったあとに画面を更新すると、未アサイン件数が1件減っていること
  • 「自分の担当」の提出物の担当から外れたあとに画面を更新すると、未アサイン件数が1件増えていること
  • 新しい提出物が作成されたあと、未アサイン件数が1件増えていること
  • 「未アサイン」の提出物が削除されたあと、未アサイン件数が1件減っていること

以下、補足説明

未アサイン件数の表示箇所(「未アサイン」タブのバッジ)のキャプチャ

image

動作確認をするときの注意事項

development モードと test モードの場合はデフォルトでキャッシュが無効になっているため、ローカルでサーバーを起動して動作確認するとき(development モードのとき)や $ rails test xxx を実行するとき(test モードのとき)は、以下のようにして事前にキャッシュを有効にしておく必要があります。

development モード

test モード

config/environments/test.rb を修正し、キャッシュストアとして :memory_storeActiveSupport::Cache::MemoryStore )を使うように設定する

diff --git a/config/environments/test.rb b/config/environments/test.rb
index e080eba6d..c20b14524 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -25,7 +25,7 @@ Rails.application.configure do
   # Show full error reports and disable caching.
   config.consider_all_requests_local       = true
   config.action_controller.perform_caching = false
-  config.cache_store = :null_store
+  config.cache_store = :memory_store

   # Raise exceptions instead of rendering exception templates.
   config.action_dispatch.show_exceptions = false

#3646 でテストが落ちる原因

未アサイン(担当のいない)の提出物に担当がついたときに、未アサイン件数のキャッシュが削除されていないため

※ キャッシュが削除されないと、オリジナルの値に変更があっても更新されない

未アサイン件数が増減する(キャッシュを削除するべき)条件と、その対応の要不要

#3646 テストの手順以外にもキャッシュが削除されていないケースがないか確認するために調査した。
調査した結果、以下の表の条件で未アサイン件数が増減することがわかった。また、それぞれの条件について対応の要不要を調べた。

凡例
○: 対応不要(キャッシュを削除できている)
△: 対応が必要(キャッシュを削除できているけれど、削除不要なときも削除している)
×: 対応が必要(キャッシュを削除できていない)

条件No. キャッシュ削除の条件 ○/☆/★
1 未アサインの提出物に担当がついたとき ×
2 アサイン済みの提出物から担当が外れたとき ×
3 提出物が作成されたとき(常に未アサイン)
4 未アサインの提出物が削除されたとき △(未アサインかどうかをチェックしていない)

キャッシュ削除の具体的な条件

具体的にどのような条件でキャッシュを削除するか表に整理した。↑の表と同じ条件は「条件No. 」の値を同じにしている。

※表の読み方: 【モデル】のレコードを 【作成/更新/削除】するときにレコードが【条件1】かつ【条件2】かつ【条件3】... の場合はキャッシュを削除する。なお、その後はキャッシュに保持する値が【キャッシュの増減値】される。

条件No. モデル 作成/更新/削除 条件1 条件2 増減値
1 Product 作成 未アサイン(作成時は未アサインなのでチェック不要) (なし) +1
2 Product 削除 未アサイン (なし) -1
3 Product 更新 更新前が未アサイン 更新後がアサイン済 -1
4 Product 更新 更新前がアサイン済 更新後が未アサイン +1

@AudioStakes AudioStakes self-assigned this Dec 17, 2021
@AudioStakes AudioStakes force-pushed the feature/delete-unassigned-count-cache-when-original-value-changed branch from e7b2c20 to 5c9ab06 Compare December 17, 2021 09:46
@AudioStakes AudioStakes force-pushed the feature/delete-unassigned-count-cache-when-original-value-changed branch from 5c9ab06 to 69963b7 Compare January 12, 2022 02:55
@AudioStakes AudioStakes force-pushed the feature/delete-unassigned-count-cache-when-original-value-changed branch from 69963b7 to a1b414c Compare January 18, 2022 06:02
@AudioStakes AudioStakes marked this pull request as ready for review January 18, 2022 06:10
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.

テストの作成もありがとうございます〜!

@komagata komagata merged commit aec88e8 into main Jan 18, 2022
@komagata komagata deleted the feature/delete-unassigned-count-cache-when-original-value-changed branch January 18, 2022 14:15
@github-actions github-actions bot mentioned this pull request Jan 18, 2022
32 tasks
@AudioStakes
Copy link
Contributor Author

@komagata
ご確認ありがとうございますー!

一つ質問があります。お時間のあるときに、返答してもらえますとありがたいです🙏
昨日、この PR が CI でテストされていないことが気になっていました。
僕の記憶では以前は CI でテストされていたものの、昨日 main を rebase して force push した後から CI のテストがなくなったように思います。
CI でテストされない原因や再び CI でテストさせる方法を調べてみたものの、あまり参考になるような情報を見つけられていません。
何かご存知のことがあれば教えてもらいたいですー🙇‍♂️

@komagata
Copy link
Member

@AudioStakes なんででしょう、ちょっと僕にもわからないです〜

@AudioStakes
Copy link
Contributor Author

@komagata
ご返信ありがとうございます🙏
他の PR では起きていないようなので、今のところは調査しなくても大丈夫そうです。
頻発するようであれば、改めて調査してみたいと思いますー

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