Conversation
📝 WalkthroughWalkthrough正規イベントのページヘッダーに Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
tyrrell-IH
force-pushed
the
feature/add-bookmark-button-to-regular-events
branch
from
0068292 to
f00dbb5
Compare
|
@machida 一応スタイルの確認をしていただいても大丈夫でしょうか？ またラベル内の文字が のようになっているのですが、 のようにしたいです。 |
このように br タグを入れる対応でお願いしますー🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/javascript/components/BookmarksInDashboard.jsx (1)
177-189: コードの重複が存在します。この関数は
Bookmarks.jsxの244-256行目と同一です。Bookmarks.jsxのレビューコメントで提案した共通ユーティリティへの抽出を適用してください。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/javascript/components/Bookmarks.jsx(2 hunks)app/javascript/components/BookmarksInDashboard.jsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/javascript/components/Bookmarks.jsx (1)
app/javascript/components/BookmarksInDashboard.jsx (1)
renderBookmarkLabel(177-189)
app/javascript/components/BookmarksInDashboard.jsx (1)
app/javascript/components/Bookmarks.jsx (1)
renderBookmarkLabel(244-256)
⏰ 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)
- GitHub Check: build_and_test
🔇 Additional comments (2)
app/javascript/components/Bookmarks.jsx (1)
184-186: ヘルパー関数の使用は適切です。
renderBookmarkLabelヘルパーの使用により、ラベル表示ロジックが適切に分離されています。app/javascript/components/BookmarksInDashboard.jsx (1)
122-124: ヘルパー関数の使用は適切です。
renderBookmarkLabelヘルパーの使用により、ラベル表示ロジックが適切に分離されています。
tyrrell-IH
force-pushed
the
feature/add-bookmark-button-to-regular-events
branch
from
ca227e6 to
79393ab
Compare
|
@smallmonkeykey |
|
@tyrrell-IH |
|
@smallmonkeykey |
|
@tyrrell-IH |
tyrrell-IH
force-pushed
the
feature/add-bookmark-button-to-regular-events
branch
from
79393ab to
9cbc3c3
Compare
|
@smallmonkeykey |
| @regular_event = regular_events(:regular_event1) | ||
| end | ||
|
|
||
| test 'shows regular event link on list when clicking bookmark button' do |
There was a problem hiding this comment.
質問です！既存のブックマーク関連のテストと違って、RegularEventテストでは 1つのテストで一連の動作をシナリオ的にまとめている構成になっていますが、こちらにした理由はありますか？？
There was a problem hiding this comment.
一連の動作をシナリオ的にまとめているわけではないつもりでした〜
以下test/system/bookmark/products_test.rbと比較して意図を説明します
前提
ブックマークボタンを今回実装したわけではないので、ブックマークボタンに関する動作はテストしていないという前提です。
ブックマーク機能は日報にブックマーク機能を実装 by alto-I · Pull Request #2789 · fjordllc/bootcampで日報に初めて導入され、その後ボタンの実装がBookmarkButton.jsxの非React化 by jun-kondo · Pull Request #9104 · fjordllc/bootcampにてバニラJSで実装されており、いずれにおいてもブックマークボタンの動作
is-activeis-inactiveの切り替えがうまくいくか- ボタンをクリックした際にリクエストが飛ぶかどうか
等はtest/system/bookmarks_test.rbやtest/integration/api/bookmarks_test.rbで保証されていると考えました。
今回のテストの趣旨
上記の前提を踏まえた上で、今回のPRは「定期イベントにブックマークボタンを追加する」なので、テストの内容を
-
テスト1
- ブックマークされていない定期イベントにおいて
- 「ブックマーク」ボタンを押すと
- ボタンの名称が「ブックマーク中」に変更され
current_user/bookmarksにブックマークした定期イベントのリンクが表示される
-
テスト2
- ブックマークされている定期イベントにおいて
- 「ブックマークボタン中」ボタンを押すと
- ボタンの名称が「ブックマーク」に変更され
current_user/bookmarksからブックマークした定期イベントのリンクが消える
の2点で良いかなと判断しました。
test/system/bookmark/products_test.rbとの比較
test/system/bookmark/products_test.rbは以下の通りです
class Bookmark::ProductTest < ApplicationSystemTestCase
setup do
@product = products(:product1)
end
test 'show product bookmark on lists' do
visit_with_auth '/current_user/bookmarks', 'kimura'
assert_text @product.title
end
test 'show active button when bookmarked product' do
visit_with_auth "/products/#{@product.id}", 'kimura'
wait_for_javascript_components
assert_selector '#bookmark-button.is-active'
assert_no_selector '#bookmark-button.is-inactive'
end
test 'show inactive button when not bookmarked product' do
visit_with_auth "/products/#{@product.id}", 'komagata'
wait_for_javascript_components
assert_selector '#bookmark-button.is-inactive'
assert_no_selector '#bookmark-button.is-active'
end
test 'bookmark product' do
visit_with_auth "/products/#{@product.id}", 'komagata'
wait_for_javascript_components
find('#bookmark-button').click
wait_for_javascript_components
assert_selector '#bookmark-button.is-active'
assert_no_selector '#bookmark-button.is-inactive'
visit '/current_user/bookmarks'
assert_text @product.title
end
test 'unbookmark product' do
visit_with_auth "/products/#{@product.id}", 'kimura'
wait_for_javascript_components
find('#bookmark-button').click
wait_for_javascript_components
assert_selector '#bookmark-button.is-inactive'
assert_no_selector '#bookmark-button.is-active'
visit '/current_user/bookmarks'
assert_no_text @product.title
end
end上記のうち
show product bookmark on listsについてはブックマークボタンを押した後に/current_user/bookmarksに表示されていることが確認できれば良いと考えたので、個別のテストは不要と考えました。show active button when bookmarked productとshow inactive button when not bookmarked productについて、is-activeとis-inactiveの切り替えについてはtest/system/bookmarks_test.rbにて動作の確認を行なっているので改めての確認は不要だと考えました。
残るテストはbookmark productとunbookmark productであり、これらのテストは上記「今回のテストの趣旨」で述べた内容と一緒です。
なので
RegularEventテストでは 1つのテストで一連の動作をシナリオ的にまとめている構成になっていますが
とありますが、私の認識としては
test/system/bookmark/products_test.rbにおけるbookmark productunbookmark product
と
test/system/bookmark/regular_events.rbにおけるshows regular event link on list when clicking bookmark buttonupdates bookmarks list when toggling bookmark
はそれぞれ同じことをテストしており、テストをまとめたわけでは無いという認識です
There was a problem hiding this comment.
test/system/bookmark/products_test.rbにおける
bookmark product
unbookmark product
と
test/system/bookmark/regular_events.rbにおける
shows regular event link on list when clicking bookmark button
updates bookmarks list when toggling bookmark
はそれぞれ同じことをテストしており、テストをまとめたわけでは無いという認識です
すみませんでした。確かにそうですね。
is-active is-inactiveの切り替えがうまくいくか
ボタンをクリックした際にリクエストが飛ぶかどうか
等はtest/system/bookmarks_test.rbやtest/integration/api/bookmarks_test.rbで保証されていると考えました。
「ブックマーク」ボタンを押すと
ボタンの名称が「ブックマーク中」に変更され
この部分をassert_selector '#bookmark-button.is-active'ではなく文言でチェックした理由を理解できました。
私の拙い質問にも丁寧に答えてくださり、ありがとうございます！！
|
@tyrrell-IH |
tyrrell-IH
force-pushed
the
feature/add-bookmark-button-to-regular-events
branch
from
9cbc3c3 to
f243d29
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/views/regular_events/_regular_event.html.slim(1 hunks)test/system/bookmark/regular_events_test.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/views/regular_events/_regular_event.html.slim
🧰 Additional context used
📓 Path-based instructions (4)
test/**/*_test.rb
📄 CodeRabbit inference engine (AGENTS.md)
test/**/*_test.rb: Test suite should use Minitest with structure:system/,models/,controllers/, and fixtures intest/fixtures/; test files should be named*_test.rb
Use Minitest + Capybara for system tests; place unit and integration tests under matchingtest/*directories
Files:
test/system/bookmark/regular_events_test.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby code should use 2-space indentation, snake_case for method names, and CamelCase for class names, enforced by RuboCop (
.rubocop.yml)
Files:
test/system/bookmark/regular_events_test.rb
⚙️ CodeRabbit configuration file
**/*.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の機能があるので使う）
Files:
test/system/bookmark/regular_events_test.rb
test/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep tests deterministic and use fixtures stored in
test/fixtures/for test data
Files:
test/system/bookmark/regular_events_test.rb
test/**/*
⚙️ CodeRabbit configuration file
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/system/bookmark/regular_events_test.rb
🧠 Learnings (6)
📓 Common learnings
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:131-135
Timestamp: 2025-09-12T21:18:00.834Z
Learning: Rails アップグレードPRにおいて、product_review_not_completed メソッドの webhook URL 設定の shared フォールバック追加も、設定システム全体の変更として別PRで対応すべきである。
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/models/graduation_notifier.rb:23-28
Timestamp: 2025-09-12T21:16:47.639Z
Learning: Rails upgrade PRにおいて、configuration systemの変更やwebhook設定の改善提案も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
Learnt from: tyrrell-IH
Repo: fjordllc/bootcamp PR: 9306
File: app/javascript/components/Bookmarks.jsx:248-265
Timestamp: 2025-11-17T00:46:30.794Z
Learning: fjordllc/bootcamp プロジェクトでは、Reactからバニラ JavaScript への移行作業が進行中である。そのため、新しいReactファイルの作成は避け、既存のReactコンポーネント内で完結する実装が推奨される。BookmarksInDashboard.jsx は issue #9045 で削除予定。
Learnt from: jun-kondo
Repo: fjordllc/bootcamp PR: 9104
File: app/javascript/bookmark-button.js:5-11
Timestamp: 2025-09-28T00:36:43.971Z
Learning: fjordllc/bootcampアプリケーションでは現在、各ページにブックマークボタンは1つのみ存在し、Turbo Driveは未使用のため、querySelector での単一要素初期化とDOMContentLoadedのみのイベントリスナーで十分である。
Learnt from: mousu-a
Repo: fjordllc/bootcamp PR: 8566
File: app/views/pair_works/_body.html.slim:0-0
Timestamp: 2025-08-31T01:05:07.447Z
Learning: システムテストでボタンやフォーム要素を識別する必要がある場合、動的な値をクラス属性に埋め込むのではなく、id属性やdata属性を使用する。特にJavaScriptでの操作やコントローラーへのデータ送信が不要な場合は、id属性を使用するのが適切。
📚 Learning: 2025-11-26T08:23:30.927Z
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T08:23:30.927Z
Learning: Applies to test/**/*_test.rb : Use Minitest + Capybara for system tests; place unit and integration tests under matching `test/*` directories
Applied to files:
test/system/bookmark/regular_events_test.rb
📚 Learning: 2025-09-12T21:18:00.834Z
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:131-135
Timestamp: 2025-09-12T21:18:00.834Z
Learning: Rails アップグレードPRにおいて、product_review_not_completed メソッドの webhook URL 設定の shared フォールバック追加も、設定システム全体の変更として別PRで対応すべきである。
Applied to files:
test/system/bookmark/regular_events_test.rb
📚 Learning: 2025-07-23T20:31:13.856Z
Learnt from: jun-kondo
Repo: fjordllc/bootcamp PR: 8977
File: app/controllers/reports_controller.rb:63-63
Timestamp: 2025-07-23T20:31:13.856Z
Learning: fjordllc/bootcampプロジェクトの`app/controllers/reports_controller.rb`において、`create`と`update`アクションは両方とも`report.save_uniquely`を使用し、同じ`:report_save`イベントと`'report.save'`イベントを発行する。これは両方とも本質的に「レポートの保存」操作であり、作成と更新を区別する必要がないためである。
Applied to files:
test/system/bookmark/regular_events_test.rb
📚 Learning: 2025-07-10T12:59:27.807Z
Learnt from: kitarou888
Repo: fjordllc/bootcamp PR: 8277
File: test/models/searcher_test.rb:326-331
Timestamp: 2025-07-10T12:59:27.807Z
Learning: モデルテストにおいて、1つのクラスメソッドの複数の挙動を検証する場合、機能の異なる側面を同じテストメソッドでテストすることは、包括的なテストとして適切である。特に`only_me`のような機能では、異なる検索条件（空文字と具体的な検索語）を使い分けることで、より広範囲な動作保証が可能となる。
Applied to files:
test/system/bookmark/regular_events_test.rb
📚 Learning: 2025-07-30T07:26:36.599Z
Learnt from: ryufuta
Repo: fjordllc/bootcamp PR: 8992
File: app/models/correct_answer_notifier.rb:23-23
Timestamp: 2025-07-30T07:26:36.599Z
Learning: Railsの標準的なヘルパーメソッド（例：`question_url`、`user_path`など）が通常の方法で使用されている場合、それらの動作テストは不要である。これらのメソッドはRailsフレームワーク自体でテストされており、アプリケーション固有のロジックではないため。
Applied to files:
test/system/bookmark/regular_events_test.rb
⏰ 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)
- GitHub Check: build_and_test
🔇 Additional comments (2)
test/system/bookmark/regular_events_test.rb (2)
1-8: セットアップは適切です。クラス構造とフィクスチャの読み込みは正しく実装されています。
10-21: JavaScriptコンポーネントの初期化待機が必要です。ブックマークボタンはJavaScriptで動作するため、
products_test.rbと同様にwait_for_javascript_componentsの呼び出しが必要です。これがないと、JavaScriptの初期化前にテストが実行され、不安定なテストになる可能性があります。以下のdiffを適用してください：
test 'shows regular event link on list when clicking bookmark button' do visit_with_auth regular_event_path(@regular_event), 'machida' + wait_for_javascript_components assert_selector '#bookmark-button', text: 'Bookmark' click_button 'Bookmark' + wait_for_javascript_components assert_selector '#bookmark-button', text: 'Bookmark中' visit current_user_bookmarks_path⛔ Skipped due to learnings
Learnt from: komagata Repo: fjordllc/bootcamp PR: 9101 File: app/notifiers/discord_notifier.rb:131-135 Timestamp: 2025-09-12T21:18:00.834Z Learning: Rails アップグレードPRにおいて、product_review_not_completed メソッドの webhook URL 設定の shared フォールバック追加も、設定システム全体の変更として別PRで対応すべきである。Learnt from: jun-kondo Repo: fjordllc/bootcamp PR: 9104 File: app/javascript/bookmark-button.js:5-11 Timestamp: 2025-09-28T00:36:43.971Z Learning: fjordllc/bootcampアプリケーションでは現在、各ページにブックマークボタンは1つのみ存在し、Turbo Driveは未使用のため、querySelector での単一要素初期化とDOMContentLoadedのみのイベントリスナーで十分である。
|
@smallmonkeykey あと以下の通り少し修正を加えました。
よろしくお願いします |
smallmonkeykey
left a comment
smallmonkeykey
left a comment
There was a problem hiding this comment.
丁寧に答えてくださりありがとうございます。
理解できましたので、approveします！
tyrrell-IH
force-pushed
the
feature/add-bookmark-button-to-regular-events
branch
from
e8165b2 to
7d8b9c7
Compare
|
@okuramasafumi チームメンバーのレビューが終わりましたので、レビューをお願いします。 |
|
@okuramasafumi 問題が解決したのち再度ご連絡いたしますので、一旦レビュー依頼は取り下げさせていただきます🙏 |
tyrrell-IH
force-pushed
the
feature/add-bookmark-button-to-regular-events
branch
from
7d8b9c7 to
874907e
Compare
|
@okuramasafumi |
okuramasafumi
left a comment
okuramasafumi
left a comment
There was a problem hiding this comment.
#bookmark_buttonがテストコード中で何度も出てくるのが少し気になったのですが、ローカル変数にするのかインスタンス変数にするのかで少し迷いました。単なる文字列なので、重複は気にしないという方針もあり得ると思いました。ここではコメントするに留めておきます。
|
|
||
| visit current_user_bookmarks_path | ||
| assert_selector 'a.card-list-item-title__link', text: @regular_event.title | ||
| click_link @regular_event.title.to_s |
There was a problem hiding this comment.
ここのto_sは不要な気がします（同じファイルの他の箇所では使われていない）
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/system/bookmark/regular_events_test.rb (2)
10-23: テストロジックは正確で、適切にJavaScriptの初期化を待機しています。ブックマークボタンのクリック、ブックマークリストへの表示、ページ遷移の確認が全て適切にテストされています。以前のレビューフィードバックに基づき、
wait_for_javascript_componentsが正しく配置されています。オプショナルな提案: Line 18の
visit current_user_bookmarks_pathの後にもwait_for_javascript_componentsを追加すると、より一貫性が保たれます。ただし、Capybaraのassert_selectorは要素の出現を待機するため、現状でも問題ない可能性が高いです。
25-45: ブックマークのトグル動作が包括的にテストされています。ブックマークの追加と削除の両方の動作を確認しており、過去のコメントで議論された通り、最初のテストとは異なるシナリオを適切にカバーしています。
wait_for_javascript_componentsの配置も適切です。同様に、Line 33とLine 43の後に
wait_for_javascript_componentsを追加することを検討できますが、これはオプショナルです。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/system/bookmark/regular_events_test.rb
🧰 Additional context used
📓 Path-based instructions (4)
test/**/*_test.rb
📄 CodeRabbit inference engine (AGENTS.md)
test/**/*_test.rb: Test suite should use Minitest with structure:system/,models/,controllers/, and fixtures intest/fixtures/; test files should be named*_test.rb
Use Minitest + Capybara for system tests; place unit and integration tests under matchingtest/*directories
Files:
test/system/bookmark/regular_events_test.rb
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby code should use 2-space indentation, snake_case for method names, and CamelCase for class names, enforced by RuboCop (
.rubocop.yml)
Files:
test/system/bookmark/regular_events_test.rb
⚙️ CodeRabbit configuration file
**/*.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の機能があるので使う）
Files:
test/system/bookmark/regular_events_test.rb
test/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep tests deterministic and use fixtures stored in
test/fixtures/for test data
Files:
test/system/bookmark/regular_events_test.rb
test/**/*
⚙️ CodeRabbit configuration file
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/system/bookmark/regular_events_test.rb
🧠 Learnings (11)
📓 Common learnings
Learnt from: tyrrell-IH
Repo: fjordllc/bootcamp PR: 9306
File: app/javascript/components/Bookmarks.jsx:248-265
Timestamp: 2025-11-17T00:46:30.794Z
Learning: fjordllc/bootcamp プロジェクトでは、Reactからバニラ JavaScript への移行作業が進行中である。そのため、新しいReactファイルの作成は避け、既存のReactコンポーネント内で完結する実装が推奨される。BookmarksInDashboard.jsx は issue #9045 で削除予定。
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:131-135
Timestamp: 2025-09-12T21:18:00.834Z
Learning: Rails アップグレードPRにおいて、product_review_not_completed メソッドの webhook URL 設定の shared フォールバック追加も、設定システム全体の変更として別PRで対応すべきである。
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/models/graduation_notifier.rb:23-28
Timestamp: 2025-09-12T21:16:47.639Z
Learning: Rails upgrade PRにおいて、configuration systemの変更やwebhook設定の改善提案も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
Learnt from: jun-kondo
Repo: fjordllc/bootcamp PR: 9104
File: app/javascript/bookmark-button.js:5-11
Timestamp: 2025-09-28T00:36:43.971Z
Learning: fjordllc/bootcampアプリケーションでは現在、各ページにブックマークボタンは1つのみ存在し、Turbo Driveは未使用のため、querySelector での単一要素初期化とDOMContentLoadedのみのイベントリスナーで十分である。
📚 Learning: 2025-11-26T08:23:30.927Z
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T08:23:30.927Z
Learning: Applies to test/**/*_test.rb : Use Minitest + Capybara for system tests; place unit and integration tests under matching `test/*` directories
Applied to files:
test/system/bookmark/regular_events_test.rb
📚 Learning: 2025-11-26T08:23:30.927Z
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T08:23:30.927Z
Learning: Applies to test/**/*_test.rb : Test suite should use Minitest with structure: `system/`, `models/`, `controllers/`, and fixtures in `test/fixtures/`; test files should be named `*_test.rb`
Applied to files:
test/system/bookmark/regular_events_test.rb
📚 Learning: 2025-09-12T21:18:00.834Z
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:131-135
Timestamp: 2025-09-12T21:18:00.834Z
Learning: Rails アップグレードPRにおいて、product_review_not_completed メソッドの webhook URL 設定の shared フォールバック追加も、設定システム全体の変更として別PRで対応すべきである。
Applied to files:
test/system/bookmark/regular_events_test.rb
📚 Learning: 2025-07-23T20:31:13.856Z
Learnt from: jun-kondo
Repo: fjordllc/bootcamp PR: 8977
File: app/controllers/reports_controller.rb:63-63
Timestamp: 2025-07-23T20:31:13.856Z
Learning: fjordllc/bootcampプロジェクトの`app/controllers/reports_controller.rb`において、`create`と`update`アクションは両方とも`report.save_uniquely`を使用し、同じ`:report_save`イベントと`'report.save'`イベントを発行する。これは両方とも本質的に「レポートの保存」操作であり、作成と更新を区別する必要がないためである。
Applied to files:
test/system/bookmark/regular_events_test.rb
📚 Learning: 2025-07-10T12:59:27.807Z
Learnt from: kitarou888
Repo: fjordllc/bootcamp PR: 8277
File: test/models/searcher_test.rb:326-331
Timestamp: 2025-07-10T12:59:27.807Z
Learning: モデルテストにおいて、1つのクラスメソッドの複数の挙動を検証する場合、機能の異なる側面を同じテストメソッドでテストすることは、包括的なテストとして適切である。特に`only_me`のような機能では、異なる検索条件（空文字と具体的な検索語）を使い分けることで、より広範囲な動作保証が可能となる。
Applied to files:
test/system/bookmark/regular_events_test.rb
📚 Learning: 2025-07-30T07:26:36.599Z
Learnt from: ryufuta
Repo: fjordllc/bootcamp PR: 8992
File: app/models/correct_answer_notifier.rb:23-23
Timestamp: 2025-07-30T07:26:36.599Z
Learning: Railsの標準的なヘルパーメソッド（例：`question_url`、`user_path`など）が通常の方法で使用されている場合、それらの動作テストは不要である。これらのメソッドはRailsフレームワーク自体でテストされており、アプリケーション固有のロジックではないため。
Applied to files:
test/system/bookmark/regular_events_test.rb
📚 Learning: 2025-09-28T00:36:43.971Z
Learnt from: jun-kondo
Repo: fjordllc/bootcamp PR: 9104
File: app/javascript/bookmark-button.js:5-11
Timestamp: 2025-09-28T00:36:43.971Z
Learning: fjordllc/bootcampアプリケーションでは現在、各ページにブックマークボタンは1つのみ存在し、Turbo Driveは未使用のため、querySelector での単一要素初期化とDOMContentLoadedのみのイベントリスナーで十分である。
Applied to files:
test/system/bookmark/regular_events_test.rb
📚 Learning: 2025-11-17T00:46:30.794Z
Learnt from: tyrrell-IH
Repo: fjordllc/bootcamp PR: 9306
File: app/javascript/components/Bookmarks.jsx:248-265
Timestamp: 2025-11-17T00:46:30.794Z
Learning: fjordllc/bootcamp プロジェクトでは、Reactからバニラ JavaScript への移行作業が進行中である。そのため、新しいReactファイルの作成は避け、既存のReactコンポーネント内で完結する実装が推奨される。BookmarksInDashboard.jsx は issue #9045 で削除予定。
Applied to files:
test/system/bookmark/regular_events_test.rb
📚 Learning: 2025-09-04T01:39:22.261Z
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 9102
File: app/controllers/mentor/buzzes_controller.rb:0-0
Timestamp: 2025-09-04T01:39:22.261Z
Learning: fjordllc/bootcamp プロジェクトでは、更新処理の成功時の通知メッセージについて、I18n を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。
Applied to files:
test/system/bookmark/regular_events_test.rb
📚 Learning: 2025-07-15T04:26:24.899Z
Learnt from: masyuko0222
Repo: fjordllc/bootcamp PR: 8934
File: app/controllers/test_feature_toggle_controller.rb:4-6
Timestamp: 2025-07-15T04:26:24.899Z
Learning: テスト用の一時的なコードで、すぐにリバートされる予定の場合、nil安全性などの防御的プログラミングの実装は不要とされることがある。
Applied to files:
test/system/bookmark/regular_events_test.rb
🧬 Code graph analysis (1)
test/system/bookmark/regular_events_test.rb (1)
test/supports/javascript_helper.rb (1)
wait_for_javascript_components(5-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)
- GitHub Check: build_and_test
🔇 Additional comments (1)
test/system/bookmark/regular_events_test.rb (1)
1-8: LGTM! テストのセットアップは適切です。ファイル構造、命名規則、fixtureの使用方法が全てプロジェクトの規約に従っています。
tyrrell-IH
force-pushed
the
feature/add-bookmark-button-to-regular-events
branch
from
c40a40f to
f389c49
Compare
|
@smallmonkeykey 以下に内容の詳細を書きますが、わからないことがあったら何でも聞いてください〜🙏 変更になった箇所
それぞれのブックマーク一覧表示のラベルの部分↓ commit: f389c49 以前の対応状況についてはここに残していますのでご参照ください。 なぜ今回改行の対応をするように変更したのかdescriptionsにも記載していますが、ラベルの表示を実装している箇所は
にてそれぞれ実装されていましたが、各ファイルはそれぞれ
にて削除（バニラJSで再実装）予定であり、私が本PRを作成した当初ではどのPRが先にマージされるかわからず、ラベル改行の実装方法が未定の状態だったので、全てのPRがmainにマージされるのを待ってから別Issueにて改行の対応を行う予定でした。 しかし現在は
という状況なので、本PRにて改行の対応を行い、matuayaさんには本PRを参考に改行の対応をしていただくよう連絡したいと思います。 @okuramasafumi ご迷惑おかけしますがよろしくお願いいたします🙏 |
smallmonkeykey
left a comment
smallmonkeykey
left a comment
There was a problem hiding this comment.
@tyrrell-IH
お待たせしてすみません。とても丁寧に説明してくださってありがとうございます。
コードもキレイで読みやすいと感じました。approveします。
|
@smallmonkeykey @okuramasafumi 追加の修正#9306 (comment) 前回の指摘事項不要な その他指摘事項について#9306 (review) 例えばインスタンス変数する方向で考えた時に |
|
@okuramasafumi @komagata |
|
@karlley マージしました〜 |
5 participants
Issue
概要
定期イベントがブックマークできるよう、
Bookmarkボタンを追加しました。変更確認方法
feature/add-bookmark-button-to-regular-eventsをローカルに取り込むBookmarkボタンを押しブックマーク登録する。最新のブックマーク欄Bookmark中ボタンを押しブックマークを解除する。最新のブックマーク欄Screenshot
変更前
変更後
ラベルの改行について
2025/12/27追記
以下の内容についてはf389c49にて改行の対応済みです
ブックマークリンクのラベルについて
のように、「定期」の文字の後に改行が入っているのが望ましいですが、現在の実装では
のように表示されています。
本来であれば適切な位置で改行を挟むことが望ましいと思います。
しかし、各ラベル要素を構築しているjsxファイル(以下2件)
/current_user/bookmarksはBookmarks.jsx/(ダッシュボードのブックマーク欄)はBookmarksInDashboard.jsxについて
にて削除予定です。
2025/11/19の開発ミーティング夜の部にて駒形さん、町田さんに了承を得た上で
という流れで処理を進めることにしました。
その他
#9104
上記PRを参考にして、本PRを作成しています。
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.