Conversation
sharoa119
force-pushed
the
feature/display_members_scheduled_to_study_now_on_dashboard
branch
from
37819a9 to
ce15991
Compare
|
@machida さん |
sharoa119
force-pushed
the
feature/display_members_scheduled_to_study_now_on_dashboard
branch
from
ce15991 to
4288012
Compare
|
@sharoa119 すいません、こちら見逃してました！！すぐやりますー 
↑
僕をPRのアサインに入れておいてもらえると助かります 🙏
|
|
@machida さん よろしくお願いいたします🙇♀️ |
|
@sharoa119 |
machida
force-pushed
the
feature/display_members_scheduled_to_study_now_on_dashboard
branch
from
1fe1840 to
1e6d91b
Compare
Walkthroughダッシュボードに「現在学習中」のユーザーアイコン一覧を表示するViewComponentとスタイル、時間範囲ヘルパー、モデルスコープ、コントローラ変数および対応するテストとフィクスチャ修正を追加。 Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as ブラウザ
participant Controller as HomeController
participant Model as User / LearningTimeFrame
participant View as home/index.html.slim
participant Component as CurrentUserIconListComponent
participant Helper as TimeRangeHelper
Browser->>Controller: GET /home
Controller->>Model: User.currently_learning_except(current_user)
activate Model
Note over Model: LearningTimeFrame.active_nowで<br/>現在の曜日・時間でフィルタ
Model-->>Controller: `@users_for_time_slot`
deactivate Model
Controller-->>View: render with `@users_for_time_slot`
View->>View: 条件判定 (`@users_for_time_slot.present`?)
alt ユーザーが存在
View->>Component: render(users: `@users_for_time_slot`)
activate Component
Component->>Helper: time_range(Time.zone.now)
activate Helper
Helper-->>Component: "(曜) HH:MM 〜 HH:MM"
deactivate Helper
Component->>Component: each `@users` -> render `users/icon` partial
Component-->>View: HTML断片
deactivate Component
else ユーザー無し
View->>View: コンポーネントをスキップ
end
View-->>Browser: HTMLレスポンス
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.rb⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (17)📓 Common learnings📚 Learning: 2025-09-08T04:55:46.649ZApplied to files:
📚 Learning: 2025-09-04T01:39:22.261ZApplied to files:
📚 Learning: 2025-07-23T20:42:19.974ZApplied to files:
📚 Learning: 2025-06-29T03:44:15.179ZApplied to files:
📚 Learning: 2025-07-26T15:24:14.799ZApplied to files:
📚 Learning: 2025-07-07T05:28:03.676ZApplied to files:
📚 Learning: 2025-09-12T21:16:47.639ZApplied to files:
📚 Learning: 2025-09-04T01:27:23.422ZApplied to files:
📚 Learning: 2025-09-04T01:50:47.773ZApplied to files:
📚 Learning: 2025-09-02T19:01:08.804ZApplied to files:
📚 Learning: 2025-09-12T21:18:00.834ZApplied to files:
📚 Learning: 2025-09-12T21:17:45.295ZApplied to files:
📚 Learning: 2025-09-11T14:47:53.256ZApplied to files:
📚 Learning: 2025-06-27T14:06:44.472ZApplied to files:
📚 Learning: 2025-08-31T03:17:44.812ZApplied to files:
📚 Learning: 2025-07-15T12:33:04.885ZApplied to files:
⏰ 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)
🔇 Additional comments (1)
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.81.7)db/schema.rbrubocop-minitest extension supports plugin, specify ... [truncated 255 characters] ... fig_loader_resolver.rb:76:in 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. Comment |
|
@sharoa119 すいません、遅くなりました🙇♂️ デザインを入れました！！ user_icon_list_component を current_user_icon_list_component という名前に変更して、抽象度を下げました。user_icon_list_component を名乗るには、今回の部分に特化したコンポーネントだったからです。 user_icon_component を削除し、従来の partial を使うようにしました。 （曜日）XX:00 〜 XX:00 を表示するためのhelperを作りました。 |
|
最新の main を取り込んだので、 手元で、 git pull --rebase origin feature/display_members_scheduled_to_study_now_on_dashboard をお願いします。 |
sharoa119
force-pushed
the
feature/display_members_scheduled_to_study_now_on_dashboard
branch
from
1e6d91b to
c01f183
Compare
|
@hirokiej さん |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
app/components/users/current_user_icon_list_component.html.slim (2)
9-9: 時刻の取得をViewからコンポーネントクラスに移動することを検討してくださいテンプレート内で直接
Time.zone.nowを呼び出すのではなく、コンポーネントクラス側で時刻を処理する方が適切です。これにより、テストが容易になり、関心の分離も向上します。コンポーネントクラスでcurrent_timeを定義し、テンプレートから参照する方法：
- = time_range(Time.zone.now) + = time_range(current_time)コンポーネントクラス側：
private def current_time Time.zone.now end
13-17: コメントアウトされたコードの整理を検討してくださいUserIconComponentのコメントアウトされた行が残っています。PRコメントによると将来的にコンポーネント化する予定とのことですが、コードベースをクリーンに保つため、不要なコメントアウトコードは削除することを推奨します。
- //= render UserIconComponent.new(user: user)test/components/users/current_user_icon_list_component_test.rb (1)
33-36: テスト内のクエリをコントローラーの実装と一致させてくださいテスト内で直接クエリを書くのではなく、実際のコントローラーの実装に合わせることで、将来的な変更に対してより堅牢なテストになります。
もしコントローラーでスコープを使用するようにリファクタリングした場合、テストも合わせて更新してください：
- users_for_time_slot = User.students_and_trainees - .joins(:learning_time_frames) - .merge(LearningTimeFrame.active_now) - .where.not(id: current_user.id) + users_for_time_slot = User.currently_learning_except(current_user)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/components/users/current_user_icon_list_component.html.slim(1 hunks)app/components/users/current_user_icon_list_component.rb(1 hunks)app/controllers/home_controller.rb(1 hunks)app/helpers/time_range_helper.rb(1 hunks)app/javascript/stylesheets/application.sass(1 hunks)app/javascript/stylesheets/application/blocks/dashboard/_current-user-icon-list.sass(1 hunks)app/javascript/stylesheets/application/blocks/micro-report/_micro-report.sass(0 hunks)app/models/learning_time_frame.rb(1 hunks)app/views/home/index.html.slim(1 hunks)test/components/users/current_user_icon_list_component_test.rb(1 hunks)
💤 Files with no reviewable changes (1)
- app/javascript/stylesheets/application/blocks/micro-report/_micro-report.sass
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.rb`: # refactoring - まずFat Controllerを避け、次にFat Modelを避ける。 - Serviceクラスの乱用を...
**/*.rb: # refactoring
- まずFat Controllerを避け、次にFat Modelを避ける。
- Serviceクラスの乱用を避ける。
- controller concernを作ろうとしたらPORO(Plain Old Ruby Object)やActiveRecordモデルでの実装で代替できないか検討する。
Rails Patterns
- ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）
- 複雑なActiveRecordクエリがあり、再利用できそうな場合はQueryObjectパターンを検討する。（rails-patterns gemを導入しているのでそれのQuery機能を使う）
- Viewにpartialを作る場合はViewComponentを使うことを検討する。
- 複数のActiveRecordモデルを操作する１つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。（interactor gemを導入しているのでそれを使う）
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。（interactor gemを導入しており、その中にOrganizerの機能があるので使う）
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
app/controllers/home_controller.rbapp/components/users/current_user_icon_list_component.rbapp/helpers/time_range_helper.rbtest/components/users/current_user_icon_list_component_test.rbapp/models/learning_time_frame.rb
`test/**/*`: # Test - TestCase名は英語で書く。 - どうしても避けられない時以外にsystem testでsleepは使わない。 # Unit Test model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
test/components/users/current_user_icon_list_component_test.rb
🧠 Learnings (1)
test/components/users/current_user_icon_list_component_test.rb (1)
Learnt from: kitarou888
PR: fjordllc/bootcamp#8277
File: test/models/searcher_test.rb:326-331
Timestamp: 2025-07-10T12:59:27.746Z
Learning: モデルテストにおいて、1つのクラスメソッドの複数の挙動を検証する場合、機能の異なる側面を同じテストメソッドでテストすることは、包括的なテストとして適切である。特に`only_me`のような機能では、異なる検索条件（空文字と具体的な検索語）を使い分けることで、より広範囲な動作保証が可能となる。
🧬 Code Graph Analysis (2)
app/helpers/time_range_helper.rb (1)
app/models/concerns/watchable.rb (1)
time(48-56)
app/models/learning_time_frame.rb (1)
app/javascript/dateFormatter.js (1)
weekday(12-12)
🔇 Additional comments (7)
app/models/learning_time_frame.rb (1)
9-24: スコープのロジックは適切です現在時刻に基づくアクティブな学習時間枠を取得するロジックは正しく実装されています。時間帯とタイムゾーンの処理も適切です。
app/helpers/time_range_helper.rb (1)
4-10: 時間範囲フォーマットの実装は適切です時間の範囲を適切にフォーマットし、日本語の曜日表示も正しく実装されています。1時間の範囲計算とフォーマットロジックは問題ありません。
app/javascript/stylesheets/application.sass (1)
22-22: CSSインポートの追加は適切です新しいコンポーネントのスタイルインポートが適切な場所に、既存のパターンに従って追加されています。
app/views/home/index.html.slim (1)
20-21: コンポーネントの条件付きレンダリングは適切です
@users_for_time_slotの存在チェックによる条件付きレンダリングは適切で、空のコンポーネントが表示されることを防いでいます。配置も既存のダッシュボードコンテンツとの整合性が取れています。app/components/users/current_user_icon_list_component.rb (1)
3-8: ViewComponentの実装は適切ですViewComponentの実装がRailsのパターンに従っており、
TimeRangeHelperのインクルードも適切です。シンプルで焦点を絞った実装になっています。app/javascript/stylesheets/application/blocks/dashboard/_current-user-icon-list.sass (1)
1-32: SASS構造とレスポンシブデザインは適切に実装されていますコンポーネントのスタイリングは適切に構成されており、以下の点で良好です：
- BEM風の命名規則に従っている
- flexboxを使用した適切なレイアウト
- メディアクエリによるレスポンシブ対応
text-blockミックスインを使用した一貫性のあるタイポグラフィtest/components/users/current_user_icon_list_component_test.rb (1)
8-20: テストケース名を英語で統一してくださいコーディングガイドラインに従い、TestCase名は英語で書く必要があります。現在のテスト名は適切ですが、一貫性を保つために確認しました。
| scope :active_now, lambda { | ||
| now = Time.zone.now | ||
| weekday_map = { | ||
| 'sunday' => '日', | ||
| 'monday' => '月', | ||
| 'tuesday' => '火', | ||
| 'wednesday' => '水', | ||
| 'thursday' => '木', | ||
| 'friday' => '金', | ||
| 'saturday' => '土' | ||
| } | ||
| weekday = weekday_map[now.strftime('%A').downcase] | ||
| hour = now.hour | ||
|
|
||
| where(week_day: weekday, activity_time: hour) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
曜日マッピングの重複を解決してください
weekday_mapのロジックがTimeRangeHelperのwdays配列と重複しています。一貫性を保つため、共通の定数またはモジュールに抽出することを検討してください。
以下のような共通定数を作成することを提案します：
+ WEEKDAY_MAPPING = {
+ 'sunday' => '日',
+ 'monday' => '月',
+ 'tuesday' => '火',
+ 'wednesday' => '水',
+ 'thursday' => '木',
+ 'friday' => '金',
+ 'saturday' => '土'
+ }.freeze
+
scope :active_now, lambda {
now = Time.zone.now
- weekday_map = {
- 'sunday' => '日',
- 'monday' => '月',
- 'tuesday' => '火',
- 'wednesday' => '水',
- 'thursday' => '木',
- 'friday' => '金',
- 'saturday' => '土'
- }
- weekday = weekday_map[now.strftime('%A').downcase]
+ weekday = WEEKDAY_MAPPING[now.strftime('%A').downcase]
hour = now.hour📝 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.
| scope :active_now, lambda { | |
| now = Time.zone.now | |
| weekday_map = { | |
| 'sunday' => '日', | |
| 'monday' => '月', | |
| 'tuesday' => '火', | |
| 'wednesday' => '水', | |
| 'thursday' => '木', | |
| 'friday' => '金', | |
| 'saturday' => '土' | |
| } | |
| weekday = weekday_map[now.strftime('%A').downcase] | |
| hour = now.hour | |
| where(week_day: weekday, activity_time: hour) | |
| } | |
| WEEKDAY_MAPPING = { | |
| 'sunday' => '日', | |
| 'monday' => '月', | |
| 'tuesday' => '火', | |
| 'wednesday'=> '水', | |
| 'thursday' => '木', | |
| 'friday' => '金', | |
| 'saturday' => '土' | |
| }.freeze | |
| scope :active_now, lambda { | |
| now = Time.zone.now | |
| weekday = WEEKDAY_MAPPING[now.strftime('%A').downcase] | |
| hour = now.hour | |
| where(week_day: weekday, activity_time: hour) | |
| } |
🤖 Prompt for AI Agents
In app/models/learning_time_frame.rb around lines 9 to 24, the weekday mapping
logic duplicates the wdays array in TimeRangeHelper, causing inconsistency. To
fix this, extract the weekday_map into a shared constant or module that both
LearningTimeFrame and TimeRangeHelper can use. Replace the inline weekday_map in
this scope with a reference to that shared constant to maintain consistency and
avoid duplication.
There was a problem hiding this comment.
🧩 Analysis chain
@sharoa119 承知いたしました。共通定数を作成して曜日マッピングの重複を解決いたします。
Task: Create shared weekday mapping constant
- First examine TimeRangeHelper implementation to understand current duplication
- Create shared constant for weekday mapping
- Update LearningTimeFrame to use shared constant
- Update TimeRangeHelper to use shared constant
🏁 Script executed:
# Check the current implementation of TimeRangeHelper
cat app/helpers/time_range_helper.rb
Length of output: 328
@sharoa119 TimeRangeHelperの実装を確認しました。現在の重複を解決するための共通定数を作成いたします。
Task: Create shared weekday mapping constant
- Create shared module with both array and hash formats for weekday mapping
- Update LearningTimeFrame to use shared constant
- Update TimeRangeHelper to use shared constant
🧬 Code Graph Analysis Results
Possibly relevant snippets from app/javascript/dateFormatter.js (line 12)
const weekday = dateObj.toLocaleDateString('ja-JP', { weekday: 'short' })This snippet shows a way to get the weekday in Japanese short format from a JavaScript Date object, which relates to the weekday mapping in LearningTimeFrame model.
No other relevant code snippets found related to defining or centralizing common constants for weekdays or activity times.
⏳ Preparing changes...
There was a problem hiding this comment.
A summary of the changes CodeRabbit can apply:
Refactor multiple Ruby files to centralize Japanese weekday mappings by creating a shared WeekdayMapping module in app/lib and updating all references to use this module’s constants, thereby eliminating duplicate weekday arrays and hashes across the codebase.
Refactor the codebase by extracting weekday-to-Japanese mappings into a new shared module
WeekdayMappinginapp/lib/weekday_mapping.rb, then updateLearningTimeFrameandTimeRangeHelperto include this module and replace their inline weekday mappings with references to the shared constants for improved code reuse and maintainability.Refactor the codebase by creating a shared WeekdayMapping module with Japanese weekday constants, then update LearningTimeFrame and TimeRangeHelper files to use this module instead of duplicating weekday mappings, improving maintainability and consistency.
- ✅ Create PR with these edits
- 📌 Create commit in current branch
- 📋 Get copyable edits
|
返信が遅くなってしまいすみません💦 |
|
@hirokiej さん |
|
@sharoa119 生徒の方にレビュー依頼する前に coderabbit の指摘点に対応してみてください〜 |
sharoa119
force-pushed
the
feature/display_members_scheduled_to_study_now_on_dashboard
branch
from
c01f183 to
3838d1b
Compare
|
komagataさんがご指摘くださった点に加えて、一部のコミットメッセージが主観的であったり、変更履歴として他の方が意図を把握しづらい可能性があるように見受けられました。 以下の記事が参考になると思いますので、よければご確認いただけたらと思います🙇 また、私の方でも同様の内容で記事を執筆しているので、よければこちらもご覧ください🙇 |
|
@hirokiej さん コミットへのご指摘、ありがとうございます！ 整理できそうな箇所はまとめてみたいと思います。しばしお待ちください🙇♀️ |
|
@sharoa119 さん
いえ、全然大丈夫ですー！ あと、コミットしなさすぎの件について、すでにご存知かもしれませんが、 |
sharoa119
force-pushed
the
feature/display_members_scheduled_to_study_now_on_dashboard
branch
6 times, most recently
from
d11e79c to
35955c3
Compare
7 participants
Issue
概要
ダッシュボードに（全てのロール、メンターも管理者も研修生もアドバイザーも）、
今が学習予定時間の人のアイコンを表示する。
（例）今が水曜日の10:30だったら、
水曜日の10:00 - 11:00 が学習予定の人全員（現役生、研修生のみ）のアイコンを表示する
変更確認方法
feature/display_members_scheduled_to_study_now_on_dashboardをローカルに取り込むforeman start -f Procfile.devでローカル環境を立ち上げるkimuraのアカウントでログイン。パスワードはtesttestkimura,kensyu,hatsunoを現在の時刻に合わせてrails cでデータを手動追加してから動作確認を行う。以下は手順です。
kensyu,hatsunokimura（自身のアイコンは表示されない仕様のため）kimuraでログイン中に「kimuraのアイコンが表示されない」のは仕様通りです。Screenshot
変更前
変更後
Summary by CodeRabbit