研修終了日が7日以内の研修生の提出物一覧をメンターのダッシュボードに表示

[MikotoMakizuru]

Issue

• 研修終了日まで7日以内の人の提出物一覧をダッシュボードに出す #7912

概要

研修生には必ずではありませんが、研修終了日が設定されています。今回の実装では、研修終了日が 7 日以内の研修生の提出物一覧をメンターのダッシュボードに表示するようにしました。これにより、メンターは研修終了が近い研修生の提出物を優先的にレビューすることが可能になります。

変更確認方法

1. feature/show-products-trainees-whose-training-end-date-within-7days をローカルに取り込む
2. rails db:seed を実行して初期データ投入（この操作はローカルのDBを初期化するので、初期化しても問題ないタイミングで行ってください。）
3. foreman start -f Procfile.dev でローカル環境を立ち上げる
4. ログイン画面で「ユーザー名 or メールアドレス」mentormentaro、「パスワード」testtest でログイン
5. ダッシュボードタブ右上に研修終了日が7日以内の提出物一覧が表示されていることを確認

Screenshot

変更前

変更後

補足事項

今回の実装により、研修終了日が 7 日以内の提出物一覧が表示されることで、「研修終了日が 7 日以内」かつ「提出から 5 日経過」の提出物が以下の画像のように 2 件表示される仕様となります。この仕様についてはこちらのコメントで相談したうえで実装を行いました。

気になっていること（不安要素）

今回の実装では、「研修終了日が 7 日以内の提出物一覧が表示されること」を確認するテストを作成しました。

assert_text 'OS X Mountain Lionをクリーンインストールするの提出物'
assert_text 'kensyu-end-within-1-week (ケンシュウ モウスコシデシュウリョウ)'

今回変更を行ったことにより、(補足事項に記載したように)提出物が 2 件表示されるため、上記のテストだと研修終了日が 7 日以内のカードに表示されていなくても、他のカードに表示されていればテストがパスしてしまいます。そこで、対象の提出物と絞るため、こちらの記事を参考に、within でスコープを絞りました。繰り返し要素の各枠には .card-list-item が使われていたため、1つ目と2つ目の要素をそれぞれ指定しています。

しかし、このテストは .card-list-item の 1 つ目と 2 つ目に必ず特定の提出物が表示されることを前提としているため、新たに .card-list-item を使用したカードが表示されると、テストが失敗するリスクがあります。これは、変更に対して脆いテストコードとなってしまいます。伊藤さんの記事の解法4のように ID を指定して各提出物を特定できればテストの安定性を高められますが、今回は View の修正までは行っていません。もし他に良い改善案があれば、ご指摘いただけると幸いです。

[MikotoMakizuru]

@maedana
研修終了日が7日以内の提出物一覧を追加しました。

現在、カードタイトルに .a-elapsed-days のクラスを使用することで文字の中央寄せや文字間隔を他のカードと統一することができますが、elapsed days が経過日数を意味するため、今回追加したカードタイトルとは適切ではないと感じています。質問タイムで相談したように、クラス名の抽象化をお願いしたいです。

[machida]

@MikotoMakizuru class名を変えてpushしましたー
レビューに進めてくださいー

[MikotoMakizuru]

@reckyy
はじめまして、PR のレビューをお願いしたいのですが可能でしょうか。
よろしくお願いします。

[reckyy]

@MikotoMakizuru
お疲れ様です。
レビューは可能です！
が、#8066 (comment) にて @Shrimprin さんにも依頼されてますが、そちらは気にしなくても大丈夫ですか？
ご返信お待ちしております。

[MikotoMakizuru]

@reckyy
返信なかったので別の方を探しておりました。
#8066 (comment) は削除しておきます。

レビューお願いいたします！🙇‍♂️

[reckyy]

@MikotoMakizuru
なるほどです。承知しました！
3日以内にはレビューさせていただきます〜。

GitHubの通知に気づかない方もいるので、急ぎで返信が欲しい場合はDiscordで連絡してる方もいます！（GitHub上でコメントした後に）

[MikotoMakizuru]

@reckyy

3日以内にはレビューさせていただきます〜。

ありがとうございます、よろしくお願いします。

GitHubの通知に気づかない方もいるので、急ぎで返信が欲しい場合はDiscordで連絡してる方もいます！（GitHub上でコメントした後に）

承知です、今後は Discord でも連絡してみたいと思います。

[reckyy]

@MikotoMakizuru
お疲れ様です。
コードに関しては、2点コメントしています。
コードと関係ない部分を2点コメントします。

お手隙の際にご確認をお願いいたします！

---

1. ダッシュボードのデザイン

画面のサイズをディスプレイの半分にした時に、カードの幅が異なっていましたがこちら意図された変更でしょうか？
ご確認をお願いいたします。（もしかしたら自分の環境起因かもしれないので、その場合はお知らせください。 🙇 ）

• mainブランチでのダッシュボード

• 本ブランチでのダッシュボード

2. 不安要素について

今回変更を行ったことにより、(補足事項に記載したように)提出物が 2 件表示されるため、上記のテストだと研修終了日が 7 日以内のカードに表示されていなくても、他のカードに表示されていればテストがパスしてしまいます。

テストコードを拝見しましたが、Productをテスト時に作成しているのでこの事象は起こらないように思いました。いかがでしょうか。

そもそも今回追加されたテストが必ず落ちてしまいます。。（ファイル単体でテストすると）
ちょっと原因については調べれていないですが、まず@MikotoMakizuru さんの手元では必ず通るかどうかお伺いしたいです。

[MikotoMakizuru]

@reckyy

画面のサイズをディスプレイの半分にした時に、カードの幅が異なっていましたがこちら意図された変更でしょうか？

私の環境でも確認できました。ユーザ名が長いとカードの幅が広がってしまうみたいです。

ウィンドウ幅が広いと「...」で省略され、ある一定のサイズまで狭めると全て表示するみたいです。あまりよろしくない仕様かと思うので「ウィンドウ幅を狭めるとユーザ名が省略されなくなる」といった別 issue で登録するか相談してみます。（デザインの部分なので町田さんが対応されるかもしれません）

2024-09-21.22.08.28.mov

---

そもそも今回追加されたテストが必ず落ちてしまいます。。（ファイル単体でテストすると）
ちょっと原因については調べれていないですが、まず@MikotoMakizuruさんの手元では必ず通るかどうかお伺いしたいです。

私の環境では、以下の通りパスしますね。。。

• ファイル指定して実行したとき

$ bin/rails test test/system/home_test.rb
Running via Spring preloader in process 83815
Run options: --seed 62766

# Running:

........................................................
[Minitest::CI] Generating test report in JUnit XML format...


Finished in 34.673798s, 1.6151 runs/s, 3.9223 assertions/s.
56 runs, 136 assertions, 0 failures, 0 errors, 0 skips

• 今回追加したテストを指定したとき

$ bin/rails test test/system/home_test.rb:325
Running via Spring preloader in process 86298
Run options: --seed 56859

# Running:

.
[Minitest::CI] Generating test report in JUnit XML format...


Finished in 4.294147s, 0.2329 runs/s, 2.5616 assertions/s.
1 runs, 11 assertions, 0 failures, 0 errors, 0 skips

CI も通っているので、ちょっと原因がわからないですね。reckyy さんの実行結果などを載せていただければ、なにかわかるかもしれないです。🙇‍♂️

[reckyy]

@MikotoMakizuru
お疲れ様です。
2点コメントしましたので、お手隙の際にご確認ください。
コメントへの対応については、修正でも返信でもどちらでも大丈夫です！次の返信の時点でApproveします〜。

遅くなりましたが、懸念点への自分の意見を共有します。

今回変更を行ったことにより、(補足事項に記載したように)提出物が 2 件表示されるため、上記のテストだと研修終了日が 7 日以内のカードに表示されていなくても、他のカードに表示されていればテストがパスしてしまいます。そこで、対象の提出物と絞るため、こちらの記事を参考に、within でスコープを絞りました。繰り返し要素の各枠には .card-list-item が使われていたため、1つ目と2つ目の要素をそれぞれ指定しています。
しかし、このテストは .card-list-item の 1 つ目と 2 つ目に必ず特定の提出物が表示されることを前提としているため、新たに .card-list-item を使用したカードが表示されると、テストが失敗するリスクがあります。これは、変更に対して脆いテストコードとなってしまいます。伊藤さんの記事の解法4のように ID を指定して各提出物を特定できればテストの安定性を高められますが、今回は View の修正までは行っていません。もし他に良い改善案があれば、ご指摘いただけると幸いです。

記事も拝見しましたが、自分もユニークなクラスやIDを付与するのが最善だと思います。ただ、このPR内では現状のテストがベストだと思いました。一応、komagataさんのレビューの際にもご相談されると良いかと思います。

以下、#8066 (comment) への回答です。

1. カードの幅問題

私の環境でも確認できました。ユーザ名が長いとカードの幅が広がってしまうみたいです。
ウィンドウ幅が広いと「...」で省略され、ある一定のサイズまで狭めると全て表示するみたいです。あまりよろしくない仕様かと思うので「ウィンドウ幅を狭めるとユーザ名が省略されなくなる」といった別 issue で登録するか相談してみます。（デザインの部分なので町田さんが対応されるかもしれません）

承知しました！お任せします。

2. testが落ちる件

こちらごめんなさい、今日回してみると問題なくパスしました。。PCの調子が悪かったのかもしれません。
この件はご放念ください。

[MikotoMakizuru]

@reckyy
お疲れです🍵

記事も拝見しましたが、自分もユニークなクラスやIDを付与するのが最善だと思います。ただ、このPR内では現状のテストがベストだと思いました。一応、komagataさんのレビューの際にもご相談されると良いかと思います。

細かく見ていただいてありがとうございます。komagata さんレビュー時にも相談してみたいと思います。

1. カードの幅問題

私の環境でも確認できました。ユーザ名が長いとカードの幅が広がってしまうみたいです。
ウィンドウ幅が広いと「...」で省略され、ある一定のサイズまで狭めると全て表示するみたいです。あまりよろしくない仕様かと思うので「ウィンドウ幅を狭めるとユーザ名が省略されなくなる」といった別 issue で登録するか相談してみます。（デザインの部分なので町田さんが対応されるかもしれません）

承知しました！お任せします。

相談してみたいと思います！🙇‍♂️

2. testが落ちる件
   こちらごめんなさい、今日回してみると問題なくパスしました。。PCの調子が悪かったのかもしれません。
   この件はご放念ください。

稀によくありますね😄
全然大丈夫です！🙆‍♂️

[reckyy]

@MikotoMakizuru
ご対応ありがとうございます！

自分としてはApproveなのですが、以下のIssueが進行しているようです。
もしご存知でしたら申し訳ないです。

• メンターダッシュボードに表示しているn日経過の提出物一覧を非vue化したい。 #7947

今回のPR内ではproduct.vueを使用していますが、上記PRではproduct.vueやproducts.vueなどのVueファイルをViewComponentに移行しているようなので、上記PRがmergeされてからの方がいいのかなと思いました。
（komagataさん、もしくは @Shrimprin さんにご相談されるのがいいかと思います。）
ご返信お待ちしております。

[MikotoMakizuru]

@reckyy
komagata さん、machida さんに確認してみます〜

[komagata]

@MikotoMakizuru

しかし、このテストは .card-list-item の 1 つ目と 2 つ目に必ず特定の提出物が表示されることを前提としているため、新たに .card-list-item を使用したカードが表示されると、テストが失敗するリスクがあります。これは、変更に対して脆いテストコードとなってしまいます。伊藤さんの記事の解法4のように ID を指定して各提出物を特定できればテストの安定性を高められますが、今回は View の修正までは行っていません。もし他に良い改善案があれば、ご指摘いただけると幸いです。

「研修終了まで7日」などのブロックで絞り込んでいる（within）のであれば問題ないです。

[MikotoMakizuru]

@reckyy

今回のPR内ではproduct.vueを使用していますが、上記PRではproduct.vueやproducts.vueなどのVueファイルをViewComponentに移行しているようなので、上記PRがmergeされてからの方がいいのかなと思いました。
（komagataさん、もしくは @Shrimprin さんにご相談されるのがいいかと思います。）
ご返信お待ちしております。

今週のスクラム MTG で相談した結果、@Shrimprin さんの実装が非 vue 化の対応であり、私が今回 vue で追加実装した部分はマージできないため、非 vue 化のプルリクがマージされたら rebase して rails の view で作り直す形となりました。（ポイントは別で付与していただくそうです。）