ActionCompletedButtonComponentを再利用可能にする#9331
Conversation
📝 WalkthroughWalkthroughActionCompletedButtonComponentの初期化引数を Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant JS as action_completed_button.js
participant API as API::TalksController
participant DB as Database
Browser->>JS: ボタンクリック
JS-->>JS: isLoading=true, ボタン無効化
JS->>API: PATCH update_path\n(body: { [modelName]: { action_completed } })
API->>DB: talk.update!(params)
DB-->>API: 保存成功
API-->>JS: 204 No Content
JS-->>Browser: UI更新（ボタン文言・アイコン・説明、トースト）
JS-->>JS: isLoading=false, ボタン有効化
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 分 Possibly related issues
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 |
![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
🧹 Nitpick comments (1)
app/javascript/action_completed_button.js (1)
4-17: 将来の複数ボタン対応を見越すならquerySelectorAll＋ ボタン単位の状態にしておくと安心です現状
.check-buttonは1つだけ存在する前提でquerySelectorとモジュールスコープのisLoadingを使っており、このPRの要件は満たしています。ただ、同一ページに複数の「対応済にするボタン」を置きたくなった場合、先頭の1つしか動かない形になるので、再利用性を少し上げるならボタンごとにハンドラとisLoadingを持たせる実装にしておくと拡張が楽だと思います。例えば次のような形です:
-document.addEventListener('DOMContentLoaded', () => { - const button = document.querySelector('.check-button') - if (!button) { - return - } - - let isLoading = false; - const updatePath = button.dataset.updatePath - const modelName = button.dataset.modelName - button.addEventListener('click', async () => { - if (isLoading) return - - isLoading = true - button.disabled = true +document.addEventListener('DOMContentLoaded', () => { + const buttons = document.querySelectorAll('.check-button') + if (!buttons.length) { + return + } + + buttons.forEach((button) => { + let isLoading = false + const updatePath = button.dataset.updatePath + const modelName = button.dataset.modelName + + button.addEventListener('click', async () => { + if (isLoading) return + + isLoading = true + button.disabled = true @@ 以降のクリックハンドラ内部は現行ロジックのまま - isLoading = false - button.disabled = false - }) + isLoading = false + button.disabled = false + }) + }) })現状の要件では必須ではないので、余裕があるタイミングでの対応で十分だと思います。
Also applies to: 22-27, 54-56
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/components/action_completed_button_component.html.slim(2 hunks)app/components/action_completed_button_component.rb(1 hunks)app/controllers/api/talks_controller.rb(1 hunks)app/javascript/action_completed_button.js(2 hunks)app/views/talks/show.html.slim(1 hunks)test/components/action_completed_button_component_test.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
app/**/*.{rb,js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Rails app code should be organized in
app/directory with subdirectories:models/,controllers/,views/,jobs/,helpers/, and frontend code underjavascript/(Shakapacker)
Files:
app/components/action_completed_button_component.rbapp/controllers/api/talks_controller.rbapp/javascript/action_completed_button.js
**/*.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:
app/components/action_completed_button_component.rbapp/controllers/api/talks_controller.rbtest/components/action_completed_button_component_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:
app/components/action_completed_button_component.rbapp/controllers/api/talks_controller.rbtest/components/action_completed_button_component_test.rb
**/*.slim
📄 CodeRabbit inference engine (AGENTS.md)
Slim templates should be linted according to
config/slim_lint.yml
Files:
app/components/action_completed_button_component.html.slimapp/views/talks/show.html.slim
app/javascript/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
JavaScript/TypeScript code in
app/javascript/should be linted with ESLint and formatted with Prettier viayarn lintscripts; use React 17 and Shakapacker/Webpack 5
Files:
app/javascript/action_completed_button.js
**/*.js
⚙️ CodeRabbit configuration file
**/*.js: - どうしても避けられない時以外に新規にVue.js, Reactのコードを追加しない。
- fetchメソッドの代わりにrequest.jsを使う。
Files:
app/javascript/action_completed_button.js
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/components/action_completed_button_component_test.rb
test/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep tests deterministic and use fixtures stored in
test/fixtures/for test data
Files:
test/components/action_completed_button_component_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/components/action_completed_button_component_test.rb
🧠 Learnings (3)
📓 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: sjabcdefin
Repo: fjordllc/bootcamp PR: 9090
File: app/javascript/user-sns.js:2-25
Timestamp: 2025-08-25T08:00:11.369Z
Learning: app/javascript/user-sns.js において、アクセシビリティ改善（aria-label属性の追加など）は他の箇所との整合性を保つため、別途統一して対応する方針である
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 を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。
Learnt from: ryufuta
Repo: fjordllc/bootcamp PR: 9155
File: app/controllers/api/correct_answers_controller.rb:20-22
Timestamp: 2025-09-15T07:39:39.350Z
Learning: API::CorrectAnswersController#update では、ベストアンサー取り下げという単純な状態変更のため、update! を使用して DB エラー時に 500 を返し、成功時はデフォルトレスポンスを使用するのが適切。クライアント側のバリデーションエラーは発生しにくく、明示的なエラーハンドリングは不要。
📚 Learning: 2025-09-15T07:39:39.350Z
Learnt from: ryufuta
Repo: fjordllc/bootcamp PR: 9155
File: app/controllers/api/correct_answers_controller.rb:20-22
Timestamp: 2025-09-15T07:39:39.350Z
Learning: API::CorrectAnswersController#update では、ベストアンサー取り下げという単純な状態変更のため、update! を使用して DB エラー時に 500 を返し、成功時はデフォルトレスポンスを使用するのが適切。クライアント側のバリデーションエラーは発生しにくく、明示的なエラーハンドリングは不要。
Applied to files:
app/controllers/api/talks_controller.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:
app/javascript/action_completed_button.js
🔇 Additional comments (5)
app/components/action_completed_button_component.html.slim (1)
7-8: data属性の追加でコンポーネントの再利用性がきれいに担保されています
data-update-path/data-model-nameが両方のボタンに付与されており、JS側のdataset.updatePath/dataset.modelNameとも整合しているので、他画面からも同じコンポーネントをそのまま使い回しやすい構造になっていて良いと思います。Also applies to: 22-23
app/controllers/api/talks_controller.rb (1)
10-11: 状態更新APIとしてupdate!＋204 No Contentの構成は妥当です単純な
action_completedのトグルであればupdate!にしてDBエラー時のみ500に任せる実装としつつ、成功時はhead :no_contentでボディなし204を返す構成で問題ないと思います。API::CorrectAnswersController の方針とも揃っており、一貫した設計になっていて良さそうです。Based on learnings, ...test/components/action_completed_button_component_test.rb (1)
1-18: 完了/未完了の両状態をカバーするViewComponentテストが追加されていて良いです
is_initial_action_completedの true/false それぞれで期待テキストを検証しており、同時にupdate_path/model_nameをコンストラクタ引数に含めることで、新しいインターフェースが壊れていないことも担保できていて良いと思います。app/views/talks/show.html.slim (1)
65-65: ActionCompletedButtonComponent 呼び出しのインターフェース更新が他レイヤーと整合しています
api_talk_path(@talk)とmodel_name: 'talk'の組み合わせにより、JS側の{ talk: { action_completed } }というリクエストボディとtalk_paramsの strong parameters がちょうど一致しており、コンポーネントの再利用化に向けた最初の呼び出し箇所として問題ないと思います。app/components/action_completed_button_component.rb (1)
4-12: コンストラクタの引数整理とprivateなreader化がシンプルで扱いやすいです
is_initial_action_completed/update_path/model_nameを明示的なキーワード引数にしつつ、内部では@is_action_completedなどにマッピングし、Viewからはprivateなreader経由でしか触れないようにしてあるのは責務がはっきりしていて良いと思います。HTMLテンプレート側の呼び出しとも齟齬がなく、インターフェース変更がきれいにまとまっています。
CodeRabbitの指摘を受けて修正：#9331 (comment)
ryufuta
force-pushed
the
chore/make-action-completed-button-reusable
branch
from
3484fba to
77163f5
Compare
|
@ryufuta まずComponentを再利用可能にしてからやるのいいですね。そしてそれを別Issueとするのもとってもいいですね。 |
CodeRabbitの指摘を受けて修正：#9331 (comment)
ryufuta
force-pushed
the
chore/make-action-completed-button-reusable
branch
from
77163f5 to
23bee5c
Compare
ryufuta
force-pushed
the
chore/make-action-completed-button-reusable
branch
from
23bee5c to
7752d2a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/javascript/action_completed_button.js (1)
55-57: ネットワークエラー時のユーザーフィードバックを検討ネットワークエラー（タイムアウト、接続失敗等）が発生した場合、
console.warnでログ出力されますが、ユーザーには何も通知されません。ボタンは再度有効になるので再試行は可能ですが、UX向上のためトースト通知を追加することを検討してください。🔎 提案
} catch (error) { + toast('通信エラーが発生しました', 'error') console.warn(error) }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/components/action_completed_button_component.html.slimapp/components/action_completed_button_component.rbapp/javascript/action_completed_button.jsapp/views/talks/show.html.slimtest/components/action_completed_button_component_test.rb
🚧 Files skipped from review as they are similar to previous changes (1)
- test/components/action_completed_button_component_test.rb
🧰 Additional context used
📓 Path-based instructions (5)
**/*.slim
📄 CodeRabbit inference engine (AGENTS.md)
Slim templates should be linted according to
config/slim_lint.yml
Files:
app/components/action_completed_button_component.html.slimapp/views/talks/show.html.slim
app/**/*.{rb,js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Rails app code should be organized in
app/directory with subdirectories:models/,controllers/,views/,jobs/,helpers/, and frontend code underjavascript/(Shakapacker)
Files:
app/javascript/action_completed_button.jsapp/components/action_completed_button_component.rb
app/javascript/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
JavaScript/TypeScript code in
app/javascript/should be linted with ESLint and formatted with Prettier viayarn lintscripts; use React 17 and Shakapacker/Webpack 5
Files:
app/javascript/action_completed_button.js
**/*.js
⚙️ CodeRabbit configuration file
**/*.js: - どうしても避けられない時以外に新規にVue.js, Reactのコードを追加しない。
- fetchメソッドの代わりにrequest.jsを使う。
Files:
app/javascript/action_completed_button.js
**/*.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:
app/components/action_completed_button_component.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:
app/components/action_completed_button_component.rb
🧠 Learnings (17)
📓 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/models/graduation_notifier.rb:23-28
Timestamp: 2025-09-12T21:16:47.639Z
Learning: Rails upgrade PRにおいて、configuration systemの変更やwebhook設定の改善提案も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
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: 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 を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:52-56
Timestamp: 2025-09-12T21:17:45.295Z
Learning: Rails upgrade PRにおいて、configuration systemの改善提案（shared configuration fallback等）も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
Learnt from: sjabcdefin
Repo: fjordllc/bootcamp PR: 9090
File: app/javascript/user-sns.js:2-25
Timestamp: 2025-08-25T08:00:11.369Z
Learning: app/javascript/user-sns.js において、アクセシビリティ改善（aria-label属性の追加など）は他の箇所との整合性を保つため、別途統一して対応する方針である
Learnt from: ryufuta
Repo: fjordllc/bootcamp PR: 9155
File: app/controllers/api/correct_answers_controller.rb:20-22
Timestamp: 2025-09-15T07:39:39.350Z
Learning: API::CorrectAnswersController#update では、ベストアンサー取り下げという単純な状態変更のため、update! を使用して DB エラー時に 500 を返し、成功時はデフォルトレスポンスを使用するのが適切。クライアント側のバリデーションエラーは発生しにくく、明示的なエラーハンドリングは不要。
Learnt from: sjabcdefin
Repo: fjordllc/bootcamp PR: 9247
File: app/controllers/checks_controller.rb:0-0
Timestamp: 2025-10-22T06:04:36.036Z
Learning: ChecksController#createおよび#destroyでは、Checkの作成・削除とActiveSupport::Notifications.instrumentによるイベント発行(プラクティスのステータス更新)を同一トランザクション内で実行し、いずれかが失敗した場合は両方をロールバックする。これによりWebUI表示とDB状態の整合性を保証している。
📚 Learning: 2025-12-04T01:50:46.369Z
Learnt from: ryufuta
Repo: fjordllc/bootcamp PR: 9331
File: app/javascript/action_completed_button.js:46-53
Timestamp: 2025-12-04T01:50:46.369Z
Learning: Rails request.js の FetchResponse クラスでは、HTTP ステータスコードは `response.statusCode` プロパティでアクセスする必要がある。`response.status` ではなく `response.statusCode` を使用する。
Applied to files:
app/javascript/action_completed_button.js
📚 Learning: 2025-07-04T07:15:17.639Z
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 8877
File: app/javascript/check-stamp.js:22-24
Timestamp: 2025-07-04T07:15:17.639Z
Learning: Rails request.jsライブラリでは、FetchResponseクラスのjsonプロパティはgetterとして定義されており、response.json()ではなくresponse.jsonとして使用する必要がある。
Applied to files:
app/javascript/action_completed_button.js
📚 Learning: 2025-07-04T07:15:17.639Z
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 8877
File: app/javascript/check-stamp.js:22-24
Timestamp: 2025-07-04T07:15:17.639Z
Learning: Rails request.jsライブラリでは、FetchResponseクラスのjsonプロパティはgetterとして定義されており、response.json()ではなくresponse.jsonとして使用する必要がある。また、Content-Typeがapplication/jsonでない場合は自動的にエラーを投げる安全性チェックも含まれている。
Applied to files:
app/javascript/action_completed_button.js
📚 Learning: 2025-07-26T01:11:03.921Z
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 8877
File: app/javascript/watches.js:78-78
Timestamp: 2025-07-26T01:11:03.921Z
Learning: Rails request.jsのFetchResponseクラスでは、text、json、htmlプロパティはgetterとして定義されており、response.text()ではなくresponse.textとしてアクセスする必要がある。getterは自動的にレスポンスをキャッシュし、jsonの場合はContent-Typeの検証も行う。
Applied to files:
app/javascript/action_completed_button.js
📚 Learning: 2025-07-04T07:17:55.949Z
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 0
File: :0-0
Timestamp: 2025-07-04T07:17:55.949Z
Learning: Rails request.jsのFetchResponseクラスでは、json、text、htmlなどはgetterとして定義されており、response.json()ではなくresponse.jsonとしてアクセスする必要がある。また、getterはContent-Typeの自動チェック機能を含んでいる。
Applied to files:
app/javascript/action_completed_button.js
📚 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:
app/javascript/action_completed_button.js
📚 Learning: 2025-07-04T07:17:55.949Z
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 0
File: :0-0
Timestamp: 2025-07-04T07:17:55.949Z
Learning: Rails request.jsのFetchResponseクラスでは、jsonとtextはgetterとして定義されており、それぞれPromiseを返す。jsongetterは自動的にContent-Typeをチェックし、application/jsonでない場合はエラーを投げる。また、レスポンスは一度パースされるとキャッシュされる仕組みになっている。
Applied to files:
app/javascript/action_completed_button.js
📚 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:
app/javascript/action_completed_button.js
📚 Learning: 2025-07-07T05:28:03.676Z
Learnt from: su-su-su-su
Repo: fjordllc/bootcamp PR: 0
File: :0-0
Timestamp: 2025-07-07T05:28:03.676Z
Learning: fjordllc/bootcampプロジェクトでは、fixtureファイル内のERB構文で改行制御文字(-)は使用せず、標準的な<% %>と<%= %>のみを使用する方針が統一されている。<% -%>や<%- %>を使用するとSyntaxErrorが発生する。
Applied to files:
app/javascript/action_completed_button.js
📚 Learning: 2025-11-13T09:20:36.030Z
Learnt from: ryufuta
Repo: fjordllc/bootcamp PR: 9115
File: db/migrate/20250902024949_add_unique_index_to_correct_answers.rb:1-5
Timestamp: 2025-11-13T09:20:36.030Z
Learning: fjordllc/bootcamp プロジェクトでは、データの整合性を保つためのデータ修正には data-migrate gem を使用し、db/data/ ディレクトリ配下にデータマイグレーションファイルを作成する運用を採用している。スキーマ変更（db/migrate/）とデータ変更（db/data/）を分離することで、より安全で管理しやすい構成を実現している。
Applied to files:
app/javascript/action_completed_button.js
📚 Learning: 2025-06-29T03:44:15.179Z
Learnt from: hirokiej
Repo: fjordllc/bootcamp PR: 8740
File: app/helpers/reports_helper.rb:56-64
Timestamp: 2025-06-29T03:44:15.179Z
Learning: このプロジェクト（fjordllc/bootcamp）では、ja.ymlファイルで一部の単語や文章のみI18n対応されているが、多くのテキストは日本語でハードコーディングされており、完全な国際化対応は行われていない。新しい機能でもI18n対応は不要と判断される。
Applied to files:
app/javascript/action_completed_button.js
📚 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:
app/javascript/action_completed_button.js
📚 Learning: 2025-09-12T21:16:47.639Z
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のスコープ維持が重要。
Applied to files:
app/javascript/action_completed_button.js
📚 Learning: 2025-09-12T21:17:26.029Z
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:19-23
Timestamp: 2025-09-12T21:17:26.029Z
Learning: Rails アップグレードPRにおいて、config_for(:secrets) を使った webhook URL 設定の shared セクション未参照問題は、設定システム全体の変更として別PRで対応すべきである。
Applied to files:
app/javascript/action_completed_button.js
📚 Learning: 2025-09-12T21:17:45.295Z
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:52-56
Timestamp: 2025-09-12T21:17:45.295Z
Learning: Rails upgrade PRにおいて、configuration systemの改善提案（shared configuration fallback等）も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
Applied to files:
app/javascript/action_completed_button.js
📚 Learning: 2025-09-11T14:47:53.256Z
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/models/product.rb:59-61
Timestamp: 2025-09-11T14:47:53.256Z
Learning: Rails アップグレードPRでは、アップグレードに直接関連しない性能改善や機能追加の提案は避けるべき。PRのスコープを維持することが重要で、そのような改善提案は別PRで対応すべきである。
Applied to files:
app/javascript/action_completed_button.js
🧬 Code graph analysis (1)
app/javascript/action_completed_button.js (2)
app/javascript/components/ActionCompletedButton.jsx (1)
isActionCompleted(11-13)app/javascript/vanillaToast.js (1)
toast(3-13)
⏰ 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 (5)
app/components/action_completed_button_component.html.slim (1)
7-8: LGTM!data属性の変更がJavaScriptの
dataset.updatePathとdataset.modelNameの読み取りと正しく対応しています。コンポーネントをより汎用的に再利用可能にする適切なリファクタリングです。Also applies to: 22-23
app/views/talks/show.html.slim (1)
65-65: LGTM!
api_talk_path(@talk)とmodel_name: 'talk'の組み合わせにより、APIコントローラーが期待するパラメータ構造（{ talk: { action_completed: ... } }）と正しく整合しています。将来的に他のモデル（助成金申請、研修申し込み等）でこのコンポーネントを再利用する際も、適切なパスとモデル名を渡すだけで対応できる設計になっています。app/javascript/action_completed_button.js (2)
1-10: LGTM! コーディングガイドラインに準拠した実装
@rails/request.jsのpatchを使用しており、fetchの代わりにrequest.jsを使うというガイドラインに従っています。二重クリック防止のisLoadingガードも適切に実装されています。
47-54: エラーハンドリングが適切に修正されています過去のレビューコメントで指摘された
response.errorの問題が修正され、response.statusCodeとresponse.textを使用して適切にエラー情報をログ出力しています。@rails/request.jsのFetchResponseの正しいAPIを使用しています。app/components/action_completed_button_component.rb (1)
3-13: LGTM! クリーンなViewComponent実装コンポーネントの初期化パラメータを
commentable_idからupdate_pathとmodel_nameに変更したことで、汎用的に再利用可能になりました。attr_readerをprivateにしているのも、テンプレート内でのみ使用されるため適切です。コーディングガイドラインに沿ったViewComponentの活用です。
|
@zamami |
zamami
left a comment
zamami
left a comment
There was a problem hiding this comment.
commentable_id 固定だった実装を update_path / model_nameに変えて再利用可能にした点は理解できました。通信処理の整理request.js失敗時ログも一貫しており、意図が明確なPRだと感じました。
OKです！
|
レビューが遅くなってお手数おかけしてすいません💦 |
|
@zamami |
|
@okuramasafumi |
| button.addEventListener('click', function () { | ||
| if (!button) return | ||
|
|
||
| let isLoading = false |
There was a problem hiding this comment.
isLoadingとbutton.disabledは同じ役割なので、button.disabledのみを使うことでも同じことができるかなと思いました（取得してきたボタンが最初からdisabledだと挙動がかわりますが）
There was a problem hiding this comment.
ご指摘ありがとうございます。
確かにisLoadingは不要ですね。
ActionCompletedButton.jsxにあったコードを参考にしたのですがReact特有の処理もコピペしてしまったようです😓
| console.warn(error) | ||
| }) | ||
| toast( | ||
| isActionCompleted ? '対応済みにしました' : '未対応にしました', |
There was a problem hiding this comment.
【今回の変更とは直接関係ないので別のissueとしてやることもできます】
isActionCompletedに関する処理が複数行あるのが少し気になるかもしれません。newButtonTextなどをまとめて返す（オブジェクトとして）のが見通しがよさそう。
例：
const { newButtonText, iconClass, newMessage, toastMessage } = isActionCompleted ? { forCompleted } : { forNotCompleted }forCompletedなどには実際の値が入りますが省略しています。
There was a problem hiding this comment.
修正しました。
私も読みにくさを感じていた箇所だったのでご指摘いただいて勉強になりました。
`app/components/action_completed_button_component.html.slim`からはprivateでも参照可能
#9331 (comment) の指摘への対応
ryufuta
force-pushed
the
chore/make-action-completed-button-reusable
branch
from
7752d2a to
7f39c06
Compare
|
@okuramasafumi |
okuramasafumi
left a comment
okuramasafumi
left a comment
There was a problem hiding this comment.
遅くなってしまってすみません、LGTM!
|
@okuramasafumi @komagata |
|
@ryufuta (CC: @okuramasafumi ) マージしました〜！ |
CodeRabbitの指摘を受けて修正：#9331 (comment)
CodeRabbitの指摘を受けて修正：#9331 (comment)
4 participants
Issue
概要
相談部屋の「対応済にするボタン」とお問い合わせ個別ページの「対応済にするボタン」は全く同じ機能・見た目だが、前者はViewComponentとバニラJSで、後者はReactで実装されている。
後者を非React化する上記のIssueに取り組むにあたって、本PRでは前者を再利用可能にした。
後者の非React化は本PRのマージ後に別のPRで、お問い合わせ機能の設計変更とともに実施する。
本PRの変更内容は以下。
※ #8535 (comment) にある通り、「対応済にするボタン」は今後、助成金コースの申請や研修の申し込みにも利用する予定のため、もともと再利用できるように実装する必要があった。
変更確認方法
chore/make-action-completed-button-reusableをローカルに取り込む参考：#8488 (comment)
Screenshot
見た目の変更はありません。
Summary by CodeRabbit
新機能
改善
テスト
✏️ Tip: You can customize this high-level summary in your review settings.