Conversation
hirokiej
force-pushed
the
feature/show-user-unchecked-report-count-only-mentor
branch
from
3309a3e to
080d079
Compare
|
@machida さん キリの良いところまで実装しましたので、デザイン付をよろしくお願いいたします🙇 |
hirokiej
force-pushed
the
feature/show-user-unchecked-report-count-only-mentor
branch
2 times, most recently
from
1fc50e8 to
ccc04cb
Compare
hirokiej
force-pushed
the
feature/show-user-unchecked-report-count-only-mentor
branch
from
ccc04cb to
30a91bd
Compare
|
@machida さん 残りの件数によって表示される文章はその後に実装いたします。 |
|
@hirokiej デザイン入れましたー 件数でデザインが変わるので注意してください🙏 |
|
@machida さん |
|
@hirokiej 確認ありがとうございます！よろしくお願いしますー |
hirokiej
force-pushed
the
feature/show-user-unchecked-report-count-only-mentor
branch
from
e62a703 to
bff43bc
Compare
|
""" Walkthroughユーザーごとの未チェックレポート数のキャッシュ管理機能が追加され、それに伴いキャッシュの取得・削除メソッド、ActiveRecordスコープ、デコレーターモジュール、UI表示、スタイル、テストが拡張・新設されました。また、未チェックレポート数のキャッシュの整合性を保つため、関連するコールバックや通知処理も修正されています。 Changes
Sequence Diagram(s)sequenceDiagram
participant StaffUser
participant View
participant Cache
participant Report
participant UserDecorator
StaffUser->>View: レポート詳細ページを表示
View->>Cache: user_unchecked_report_count(report.user)
Cache->>Report: unchecked, not WIP, userスコープで件数取得
Cache-->>View: 件数返却
View->>UserDecorator: unchecked_report_message(件数, user)
UserDecorator-->>View: メッセージ返却
View-->>StaffUser: 件数とメッセージを表示
sequenceDiagram
participant Report
participant Callbacks
participant Cache
Report->>Callbacks: レポート削除/チェック作成・削除
Callbacks->>Cache: delete_unchecked_report_count
Callbacks->>Cache: delete_user_unchecked_report_count(user_id)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 RuboCop (1.76.1)app/decorators/user_decorator.rbrubocop-minitest extension supports plugin, specify test/decorators/user_decorator/report_status.rbrubocop-minitest extension supports plugin, specify app/decorators/user_decorator/report_status.rbrubocop-minitest extension supports plugin, specify
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
hirokiej
force-pushed
the
feature/show-user-unchecked-report-count-only-mentor
branch
from
bff43bc to
b59205d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/views/reports/_report_body.html.slim (2)
10-10: ビューでのキャッシュ呼び出しについて検討してくださいビューテンプレート内で直接
Cache.user_unchecked_report_count(report.user)を呼び出していますが、パフォーマンスの観点から、この処理をコントローラーで事前に実行し、インスタンス変数として渡すことを検討してください。コントローラーで以下のように事前に取得することをお勧めします：
# コントローラー内で +@unchecked_count = Cache.user_unchecked_report_count(@report.user) if staff_login?# ビューで -- count = Cache.user_unchecked_report_count(report.user) +- count = @unchecked_count
17-18: リンクテキストの一貫性を確保「#{report.user.login_name}さんの日報一覧へ」というリンクテキストが、メッセージ内容と重複している可能性があります。ユーザビリティの観点から、より簡潔な表現を検討してください。
以下のような簡潔な表現を検討してください：
-| #{report.user.login_name}さんの日報一覧へ +| 日報一覧を見る
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/helpers/reports_helper.rb(2 hunks)app/javascript/stylesheets/application.sass(1 hunks)app/javascript/stylesheets/application/blocks/cards/_card-body-main-actions.sass(1 hunks)app/javascript/stylesheets/application/blocks/event/_event-main-actions.sass(2 hunks)app/javascript/stylesheets/application/blocks/micro-report/_micro-report.sass(0 hunks)app/models/cache.rb(1 hunks)app/models/report.rb(1 hunks)app/views/reports/_report_body.html.slim(1 hunks)test/helpers/reports_helper_test.rb(1 hunks)
💤 Files with no reviewable changes (1)
- app/javascript/stylesheets/application/blocks/micro-report/_micro-report.sass
🧰 Additional context used
🧠 Learnings (1)
app/views/reports/_report_body.html.slim (1)
Learnt from: tyrrell-IH
PR: fjordllc/bootcamp#8807
File: app/views/welcome/job_support.html.slim:391-392
Timestamp: 2025-06-21T22:30:20.116Z
Learning: このプロジェクトにはRailsの組み込みmarkdownヘルパが利用できないため、markdownのHTML変換には自前でヘルパを実装する必要がある。
🧬 Code Graph Analysis (1)
test/helpers/reports_helper_test.rb (1)
app/helpers/reports_helper.rb (2)
user_report_count_class(43-54)unchecked_report_message(56-64)
🔇 Additional comments (14)
app/javascript/stylesheets/application.sass (1)
16-16: 新しいスタイルシートのインポートが適切に追加されていますカード関連のインポートと一緒にグループ化されており、既存のパターンに従っています。
app/models/report.rb (1)
62-62: ユーザーでのフィルタリングスコープが適切に実装されていますRailsの慣習に従い、シンプルで明確な実装になっています。既存のスコープパターンと一貫性があります。
app/javascript/stylesheets/application/blocks/event/_event-main-actions.sass (2)
18-19: イベントアクションの色調整が適切に行われています未参加で参加可能な状態の背景色とテキスト色がより見やすく調整されています。
30-30: フォントサイズの調整で可読性が向上していますわずかなサイズ増加により、説明文の可読性が改善されています。
test/helpers/reports_helper_test.rb (2)
18-30: レポート件数のCSSクラス分類のテストが包括的です成功、プライマリ、警告、危険の各範囲をカバーしており、境界値のテストも含まれています。
32-37: 未チェックレポートメッセージのテストが適切です0件、1件（最後）、複数件の各シナリオをテストしており、ユーザー名の組み込みも確認されています。
app/models/cache.rb (2)
15-19: ユーザー固有の未チェックレポート数キャッシュが適切に実装されています既存のキャッシュメソッドパターンに従い、適切なスコープ組み合わせで実装されています。
21-23: レポートモデルのキャッシュ削除コールバックの配置確認
delete_user_unchecked_report_countメソッドがレポート作成・更新・削除時のコールバックとして登録されているか確認できませんでした。以下をご確認ください：
app/models/report.rbに以下のような登録があるか
after_create :delete_user_unchecked_report_countafter_update :delete_user_unchecked_report_countafter_destroy :delete_user_unchecked_report_countapp/views/reports/_report_body.html.slim (1)
11-11: CSSクラスの動的生成が適切に実装されていますヘルパーメソッドを使用したCSSクラスの動的生成により、未チェック件数に応じた適切なスタイリングが実現されています。メンター専用表示の条件分岐も適切です。
app/javascript/stylesheets/application/blocks/cards/_card-body-main-actions.sass (2)
2-11: レスポンシブデザインの実装が適切ですメディアクエリを使用したレスポンシブデザインの実装が適切で、異なる画面サイズに対応したパディングとマージンの調整が行われています。
34-42: フレックスボックスレイアウトの実装が優秀ですアイテムの中央配置とレスポンシブな最大幅設定により、様々な画面サイズで適切な表示が実現されています。
app/helpers/reports_helper.rb (3)
4-8: 定数定義が明確で理解しやすいです未チェック件数に基づく分類の定数定義が明確で、コードの可読性が高いです。無限範囲記法
(10..)の使用も適切です。
43-54: CSS クラスマッピングロジックが適切ですcase文を使用したシンプルで明確なCSS クラスマッピングが実装されています。範囲オブジェクトを使用した条件分岐も理解しやすいです。
56-64: XSSリスクなし:login_nameはバリデーションと自動エスケープで保護されています
User#login_nameは以下のバリデーションでHTMLタグやスクリプト挿入を防止
- 除外ルール（
RESERVED_LOGIN_NAMES）- 文字数制限（最小3文字）
- 存在性・一意性
- 形式制約（半角英数字とハイフンのみを許可）
- Railsのビューでは返却された文字列はデフォルトでエスケープされるため、ヘルパーメソッド内での文字列補間でもXSSは発生しません
以上より、現状の実装で追加対応は不要です。
| &.is-success | ||
| background-color: #f8fff2 | ||
| border: 1px solid var(--success) | ||
| color: var(--success-text) | ||
| &.is-primary | ||
| border: 1px solid var(--primary) | ||
| background-color: #f5f5ff | ||
| color: var(--primary-text) | ||
| &.is-warning | ||
| border: 1px solid var(--warning) | ||
| background-color: #fff9e7 | ||
| color: var(--warning-text) | ||
| &.is-danger | ||
| border: 1px solid var(--danger) | ||
| background-color: var(--danger-tint) | ||
| color: var(--danger-text) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
アクセシビリティの改善を検討してください
色のみでステータスを示すスタイリングは、色覚に障害のあるユーザーにとってアクセシブルではありません。アイコンやボーダースタイルなどの追加の視覚的手がかりを検討してください。
以下のような改善案を検討してください：
&.is-success
background-color: #f8fff2
border: 1px solid var(--success)
color: var(--success-text)
+ &::before
+ content: "✓"
+ margin-right: 0.5rem同様に他の状態についても適切なアイコンやインジケーターを追加することをお勧めします。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| &.is-success | |
| background-color: #f8fff2 | |
| border: 1px solid var(--success) | |
| color: var(--success-text) | |
| &.is-primary | |
| border: 1px solid var(--primary) | |
| background-color: #f5f5ff | |
| color: var(--primary-text) | |
| &.is-warning | |
| border: 1px solid var(--warning) | |
| background-color: #fff9e7 | |
| color: var(--warning-text) | |
| &.is-danger | |
| border: 1px solid var(--danger) | |
| background-color: var(--danger-tint) | |
| color: var(--danger-text) | |
| &.is-success | |
| background-color: #f8fff2 | |
| border: 1px solid var(--success) | |
| color: var(--success-text) | |
| &::before | |
| content: "✓" | |
| margin-right: 0.5rem | |
| &.is-primary | |
| border: 1px solid var(--primary) | |
| background-color: #f5f5ff | |
| color: var(--primary-text) | |
| &.is-warning | |
| border: 1px solid var(--warning) | |
| background-color: #fff9e7 | |
| color: var(--warning-text) | |
| &.is-danger | |
| border: 1px solid var(--danger) | |
| background-color: var(--danger-tint) | |
| color: var(--danger-text) |
🤖 Prompt for AI Agents
In
app/javascript/stylesheets/application/blocks/cards/_card-body-main-actions.sass
lines 12 to 27, the current styling uses only color changes to indicate status,
which is not accessible for users with color vision deficiencies. To fix this,
add additional visual indicators such as icons or distinct border styles for
each status class (is-success, is-primary, is-warning, is-danger). Ensure these
indicators are clearly visible and consistent across all statuses to improve
accessibility.
There was a problem hiding this comment.
@hirokiej こちらの指摘に対応してください。（問題なければそのようにコメントして議論が決着するところまでやってください）
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@hirokiej 色での判断は不要ですー。理由は、提出物は期限があり、その期限にどれくらい迫っているかをすぐに確認できるように色が用意されているが、日報にはそれがないためです。
There was a problem hiding this comment.
@machida さん
ご返信ありがとうございます！
提出物のUIの意図も理解できました🙏
それでは、こちら問題なしということで対応いたします🙇
@coderabbitai
こちら、色での判断はしていないので問題ありません。
There was a problem hiding this comment.
@hirokiej さん、ご説明ありがとうございます！
machidaさんからの説明もとても参考になりました。提出物と日報でのUI設計の意図の違いがよく理解できました。確かに、こちらの実装では件数が数値として明確に表示されており、色はあくまで視覚的な補助として機能しているため、アクセシビリティの問題はありませんね。
ご対応いただき、ありがとうございました！
(´・ω・｀)
✏️ Learnings added
Learnt from: hirokiej
PR: fjordllc/bootcamp#8740
File: app/javascript/stylesheets/application/blocks/cards/_card-body-main-actions.sass:12-27
Timestamp: 2025-07-29T07:46:11.138Z
Learning: In this codebase, color coding serves different purposes for different features: submissions (提出物) use color indicators to show deadline urgency which is important for quick assessment, while daily reports (日報) don't require such urgency indicators. When implementing status indicators, the primary information should be conveyed through text/numbers rather than color alone for accessibility.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
hirokiej
force-pushed
the
feature/show-user-unchecked-report-count-only-mentor
branch
from
b59205d to
d8bd834
Compare
|
@su-su-su-su さん |
hirokiej
force-pushed
the
feature/show-user-unchecked-report-count-only-mentor
branch
from
5c1d939 to
199e349
Compare
|
@hirokiej 申し訳ございません。 |
|
@su-su-su-su さん @kushimegu さん |
hirokiej
force-pushed
the
feature/show-user-unchecked-report-count-only-mentor
branch
from
ff0151c to
6f94e8b
Compare
|
@komagata さん 修正も致しましたので、お手隙の際に再度ご確認よろしくお願いいたします🙇 |
komagata
deleted the
feature/show-user-unchecked-report-count-only-mentor
branch
5 participants
Issue
概要
メンターでログインしたとき、日報の個別ページにそのユーザーの未チェックの日報数を表示した
変更確認方法
feature/show-user-unchecked-report-count-only-mentorをローカルに取り込むkomagata、passwordtesttestでログインをする。i.「MaruMaru Inc.の研修生のテスト日報14です」で「marumarushain14さんの未チェックの日報はこれで最後です。」という文言を確認し、「日報を確認」をクリック。
ii.ページを更新し、「marumarushain14さんの日報へ」と変更されていることを確認する。
iii.「 2日目 昨日よりできなかった 」で「hatsunoさんの未チェックの日報が3件あります。」の文言を確認する。
IV.「 今日は頑張りました」で「hajimeさんの未チェックの日報が5件あります。」
V.「 日報70 」で「with-hyphenさんの未チェックの日報が34件あります。」の文言を確認する。
6.メンターモードをオフにして、表示が消えることを確認する。
Screenshot
変更前
変更後
Summary by CodeRabbit
新機能
スタイル
バグ修正
テスト