Conversation
`textarea.js`は他で使ってなかったため削除
Walkthrough（概要）レポートテンプレートコントローラーのモーダルオープン時にテキスエリアの再初期化前に未初期化を実行し、フォームの説明フィールドの条件付きCSSクラス割り当てを削除する変更。 Changes（変更内容）
Estimated code review effort（推定レビュー工数）🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs（関連する可能性のあるPR）
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 |
- `_form.html.slim`のtextarea から`.js-markdown`を削除 - Stimulusのみで初期化
|
@matuaya |
|
@smallmonkeykey |
|
@matuaya |
|
@matuaya |
|
@smallmonkeykey |
|
@matuaya |
違うんです・・！🙇♀️💦 |
`uninitialize()`を実行してから`initialize()`を呼ぶように変更し、毎回1回だけ初期化されるように修正
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/javascript/controllers/report_template_controller.js (2)
24-30: [任意の改善提案] connect内でも初期化前にuninitializeを検討現在の変更により、ビュー側から
js-markdownクラスが削除され、二重初期化の問題は解決されています。ただし、Turbolinks/Turbo環境ではコントローラーが切断・再接続される可能性があるため、より防御的なコードとしてinitializeの前にuninitializeを呼び出すことも検討できます。この対応は必須ではありませんが、
replaceReportやopenModalと同様のパターンに統一できます:connect() { if (this.reportTarget.value === '') { this.reportTarget.value = this.registeredTemplateValue } this.registeredTemplateValue = this.templateInputTarget.value + TextareaInitializer.uninitialize('.js-report-content') TextareaInitializer.initialize('.js-report-content') }
82-104: [任意のリファクタリング提案] fetchメソッドの代わりにrequest.jsの使用を検討コーディングガイドラインでは
fetchメソッドの代わりに request.js の使用が推奨されています。registerTemplateメソッド（82-104行目）とupdateTemplateメソッド（120-138行目）でfetchが使用されていますが、これは既存のコードであり、今回のバグ修正の範囲外です。必要に応じて、別PRでの対応を検討してください。コーディングガイドラインに基づく。
Also applies to: 120-138
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/javascript/controllers/report_template_controller.js(1 hunks)app/views/reports/_form.html.slim(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit configuration file
**/*.js: - どうしても避けられない時以外に新規にVue.js, Reactのコードを追加しない。
- fetchメソッドの代わりにrequest.jsを使う。
Files:
app/javascript/controllers/report_template_controller.js
🧠 Learnings (1)
📓 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: ryufuta
Repo: fjordllc/bootcamp PR: 8992
File: app/models/correct_answer_notifier.rb:23-23
Timestamp: 2025-07-30T07:26:36.599Z
Learning: PRのスコープを維持することが重要であり、バグ修正PRにおいて、修正対象以外の機能（例：ルーティングテスト）の追加提案は適切ではない。そのような改善が必要な場合は別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 を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。
⏰ 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/controllers/report_template_controller.js (1)
55-62: LGTM! モーダル表示時のテキストエリアをクリーンな状態で再初期化
uninitializeを追加することで、モーダルを開くたびにテキストエリアがクリーンな状態で初期化されます。これはreplaceReportメソッドと同じパターンであり、イベントハンドラの重複登録を防ぐ適切な実装です。app/views/reports/_form.html.slim (1)
82-86: LGTM! 二重初期化を防ぐコアな修正
js-markdownクラスの条件付き追加ロジックを削除し、静的なクラスリストに変更したことで、application.jsからの初期化が防止されます。これにより、Stimulusコントローラー（js-report-contentクラス経由）のみがTextareaInitializerを管理するようになり、画像の二重貼り付け問題が解決されます。
|
@matuaya |
|
@smallmonkeykey |
|
@matuaya |
matuaya
left a comment
matuaya
left a comment
There was a problem hiding this comment.
動作バッチリ問題ありませんでした
Approveさせていただきます！
|
@matuaya |
|
@komagata |
3 participants
Issue
概要
日報本文の更新時とコピーボタンによる新規作成時に画像をアップロードして画像を貼り付けると1度の貼り付けにも関わらず、二重で貼り付けられてしまっていたので修正しました。
また、モーダル内のテンプレート本文に画像をペーストすると、モーダルを開き直すたびに貼り付け画像が重複して増えていくバグが発生するので修正しました。
変更確認方法
コピーボタンによる新規作成時に画像をアップロードするの確認方法
1.任意の生徒のユーザーでログインする
2.すでに書いてある日報のページから
コピーボタンをクリックし新規作成に移る3.本文に画像をペーストする
4.1枚のみペーストされるか確認する
モーダル内のテンプレート本文の画像の確認方法
1.任意の生徒のユーザーでログインする
1.日報画面で
テンプレート登録またはテンプレート変更をクリックする2.モーダルが開いた状態で、テンプレート本文欄に画像をペーストする
→ 画像が1枚貼られる
3.
キャンセルをクリックしてモーダルを閉じる4.再び
テンプレート登録またはテンプレート変更をクリックする5.同じように画像をペーストする
→ 画像が2枚ではなく、1枚貼られることを確認する
原因と修正
日報本文の更新時とコピーボタンによる新規作成時の件
TextareaInitializerが同じtextareaに対してJSのファイルとStimulusの両方から初期化されていたため、画像アップロードイベントが二重登録されていました。
具体的には以下の2箇所で初期化が行われていました。
app/javascript/packs/application.jsにてimport'../textarea.js'により、'.js-markdown'があるページで textarea.js が実行app/javascript/controllers/report_template_controller.js内で.js-report-contentに対して再度 TextareaInitializer.initialize() を実行これにより、画像ペーストが2回貼られしまっていました。
また、新規作成時にこの不具合が発生しなかったのは、 以下の条件分岐によって
.js-markdownが付与されていなかったためです。class: "a-text-input js-warning-form js-report-content markdown-form__text-area #{params[:action] == 'new' && params[:id].blank? ? '' : 'js-markdown'}",この分岐でした。
JSファイルでの実行は不要なため、この
.js-markdown関連の分岐を削除し、Stimulus側でのみTextareaInitializerを管理するように変更しました。モーダル内のテンプレート本文の画像が複数貼られる件
モーダルを開くたびにTextareaInitializer.initializeが複数回実行され、同じtextareaにpasteイベントが重複登録されていました。
uninitializeを実行してから initializeを呼ぶように変更し、毎回1回だけ初期化されるように修正しました。
Screenshot
変更前
変更後
Summary by CodeRabbit
バグ修正