Conversation
|
""" Walkthroughタグ編集UIの実装がReactコンポーネントからRuby ViewComponentと純粋なJavaScript（Tagify利用）へ全面的に移行されました。タグのバリデーション・変換・パース・API通信などのロジックも個別のJSモジュールとして再構成されています。関連するビューやヘルパーもReact依存から脱却しています。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant TagJS
participant Server
User->>Browser: 「タグ編集」ボタンをクリック
Browser->>TagJS: タグ編集UIを表示
User->>TagJS: タグを編集
TagJS->>TagJS: バリデーション・変換処理
User->>TagJS: 保存ボタン押下
TagJS->>Server: PUT /api/tags (新タグリスト送信)
Server-->>TagJS: 更新結果レスポンス
TagJS->>Browser: タグリスト再描画・編集UIを隠す
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~45 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)test/components/tag/form_component_test.rbrubocop-minitest extension supports plugin, specify app/components/tag/form_component.rbrubocop-minitest extension supports plugin, specify Note ⚡️ Unit Test Generation - BetaCodeRabbit's unit test generation is now available in Beta! Automatically generate comprehensive unit tests for your code changes, ensuring better test coverage and catching edge cases you might miss. Our AI analyzes your code structure and creates tests that follow best practices and your project's testing patterns. Learn more here, or just try it under ✨ Finishing Touches. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (14)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (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)
✨ 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
app/javascript/parse_tags.js (1)
4-4: 不要なmapメソッドを削除することで処理を簡潔化できます。現在の実装では
map((value) => value)で値をそのまま返しているため、この処理は冗長です。以下の修正により処理をより簡潔にできます：
- return value.split(',').map((value) => value) + return value.split(',')app/views/movies/_movie_header.html.slim (1)
77-77: 不要な.to_sの呼び出しを削除してください。ViewComponentへの移行は正しく実装されていますが、
join(',')は既に文字列を返すため、.to_sの呼び出しは不要です。- = render(Tag::TagComponent.new(initial_tags: movie.tag_list.join(',').to_s, param_name: 'movie[tag_list]', input_id: 'movie_tag_list', taggable_type: 'Movie', taggable_id: movie.id.to_s)) + = render(Tag::TagComponent.new(initial_tags: movie.tag_list.join(','), param_name: 'movie[tag_list]', input_id: 'movie_tag_list', taggable_type: 'Movie', taggable_id: movie.id.to_s))app/javascript/tag.js (2)
36-53: タグレンダリング処理のパフォーマンスを改善してください現在の実装では、タグをレンダリングするたびにDOM要素の検索と削除を行っています。DocumentFragmentを使用することで、より効率的にDOM操作を行えます。
以下の改善を検討してください：
const renderTags = (tags) => { const items = tagListItems.querySelectorAll('.tag-links__item') items.forEach((item) => { if (!item.querySelector('.tag-links__item-edit')) { item.remove() } }) + const fragment = document.createDocumentFragment() tags.forEach((tag) => { const li = document.createElement('li') li.className = 'tag-links__item' li.innerHTML = `<a class="tag-links__item-link" href="/${tagsType.toLowerCase()}s/tags/${encodeURIComponent( tag )}">${tag}</a>` - const editBtnLi = editButton?.parentElement - tagListItems.insertBefore(li, editBtnLi || null) + fragment.appendChild(li) }) + + const editBtnLi = editButton?.parentElement + tagListItems.insertBefore(fragment, editBtnLi || null) }
80-82: ユーザーエクスペリエンスの改善を検討してください
alert()を使用したエラー表示は、ユーザビリティの観点から改善の余地があります。より適切なUI要素（トーストメッセージやインライン表示）を使用することを検討してください。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
app/components/tag/tag_component.html.slim(1 hunks)app/components/tag/tag_component.rb(1 hunks)app/controllers/welcome_controller.rb(3 hunks)app/javascript/components/Tags/Tags.jsx(0 hunks)app/javascript/head-is-sharp-or-octothorpe.js(1 hunks)app/javascript/packs/application.js(1 hunks)app/javascript/parse_tags.js(1 hunks)app/javascript/tag.js(1 hunks)app/javascript/transform-head-sharp.js(1 hunks)app/javascript/validate-tag-name.js(1 hunks)app/views/movies/_movie_header.html.slim(1 hunks)app/views/pages/_doc_header.html.slim(1 hunks)app/views/questions/_question_header.html.slim(1 hunks)app/views/users/show.html.slim(1 hunks)app/views/welcome/certified_reskill_courses/rails_developer_course/_faq.html.slim(1 hunks)app/views/welcome/certified_reskill_courses/rails_developer_course/index.html.slim(1 hunks)app/views/welcome/faqs/_faqs_empty.html.slim(1 hunks)db/fixtures/faq_categories.yml(1 hunks)db/fixtures/faqs.yml(1 hunks)
💤 Files with no reviewable changes (1)
- app/javascript/components/Tags/Tags.jsx
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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変換には自前でヘルパを実装する必要がある。
Learnt from: e-yanagita-gs
PR: fjordllc/bootcamp#8771
File: app/controllers/welcome_controller.rb:57-57
Timestamp: 2025-06-30T01:07:46.170Z
Learning: プロジェクトでは、仕様上データの存在が保証されている場合、nil処理を省略することがある。特にFAQ表示のような特定機能に特化したPRでは、データの存在を前提とした実装が採用される。
app/views/welcome/certified_reskill_courses/rails_developer_course/_faq.html.slim (2)
Learnt from: e-yanagita-gs
PR: fjordllc/bootcamp#8771
File: app/controllers/welcome_controller.rb:57-57
Timestamp: 2025-06-30T01:07:46.170Z
Learning: プロジェクトでは、仕様上データの存在が保証されている場合、nil処理を省略することがある。特にFAQ表示のような特定機能に特化したPRでは、データの存在を前提とした実装が採用される。
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変換には自前でヘルパを実装する必要がある。
app/components/tag/tag_component.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変換には自前でヘルパを実装する必要がある。
app/controllers/welcome_controller.rb (1)
Learnt from: e-yanagita-gs
PR: fjordllc/bootcamp#8771
File: app/controllers/welcome_controller.rb:57-57
Timestamp: 2025-06-30T01:07:46.170Z
Learning: プロジェクトでは、仕様上データの存在が保証されている場合、nil処理を省略することがある。特にFAQ表示のような特定機能に特化したPRでは、データの存在を前提とした実装が採用される。
app/views/movies/_movie_header.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変換には自前でヘルパを実装する必要がある。
app/views/welcome/faqs/_faqs_empty.html.slim (1)
Learnt from: e-yanagita-gs
PR: fjordllc/bootcamp#8771
File: app/controllers/welcome_controller.rb:57-57
Timestamp: 2025-06-30T01:07:46.170Z
Learning: プロジェクトでは、仕様上データの存在が保証されている場合、nil処理を省略することがある。特にFAQ表示のような特定機能に特化したPRでは、データの存在を前提とした実装が採用される。
db/fixtures/faqs.yml (1)
Learnt from: e-yanagita-gs
PR: fjordllc/bootcamp#8771
File: app/controllers/welcome_controller.rb:57-57
Timestamp: 2025-06-30T01:07:46.170Z
Learning: プロジェクトでは、仕様上データの存在が保証されている場合、nil処理を省略することがある。特にFAQ表示のような特定機能に特化したPRでは、データの存在を前提とした実装が採用される。
🪛 ESLint
app/javascript/validate-tag-name.js
[error] 3-3: Irregular whitespace not allowed.
(no-irregular-whitespace)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_and_test
🔇 Additional comments (15)
app/javascript/packs/application.js (1)
82-82: 新しいタグ編集機能の統合が適切に行われています。ReactベースのタグコンポーネントからVanilla JavaScriptによる実装への移行に合わせて、新しいタグ編集モジュールが正しくインポートされています。
app/components/tag/tag_component.html.slim (1)
1-23: タグコンポーネントのSlimテンプレートが適切に実装されています。ReactコンポーネントからRailsViewComponentへの移行において、以下の点が適切に実装されています：
- 表示モードと編集モードの適切な分離
- JavaScript初期化のためのdata属性の設定
- フォームの初期状態が非表示になっている
- 保存・キャンセルボタンの適切な配置
app/javascript/head-is-sharp-or-octothorpe.js (1)
1-6: 関数のリファクタリングが適切に行われています。以下の改善により、関数がより堅牢になりました：
- null/falsy値のチェック追加によるエラー防止
- 不要な
.*を削除した正規表現の簡略化- オブジェクトエクスポートから関数エクスポートへの変更
app/views/users/show.html.slim (1)
50-50: ReactコンポーネントからRailsViewComponentへの移行が正しく実装されています。
Tag::TagComponentの呼び出しにおいて、元のReactコンポーネントのpropsが適切にマッピングされており、編集権限の判定も正しく行われています。app/views/questions/_question_header.html.slim (1)
74-74: Reactコンポーネントからの移行が正しく実装されています。新しいRails ViewComponentへの移行が適切に行われており、必要なパラメータが正しく渡されています。
app/components/tag/tag_component.rb (1)
1-16: ViewComponentの実装が適切です。新しいTag::TagComponentクラスは、必要なパラメータを適切に初期化し、ViewComponentの規約に従って実装されています。
editable?メソッドも明確で使いやすいインターフェースを提供しています。app/javascript/tag.js (1)
1-128: 全体的な実装品質についてReact からVanilla JavaScriptへの移行により、コードの可読性と保守性が向上しています。Tagifyライブラリの適切な使用と、CSRFトークンの取り扱いも正しく実装されています。
db/fixtures/faq_categories.yml (1)
29-31: 新しいFAQカテゴリの追加が適切です新しいFAQカテゴリ「給付制度対象講座について」の追加は、既存のパターンに従い、position値も適切に設定されています。
app/views/welcome/certified_reskill_courses/rails_developer_course/index.html.slim (1)
51-54: FAQセクションの条件付きレンダリングが適切ですFAQが存在する場合の表示と、管理者向けの空状態表示の条件分岐が適切に実装されています。UXの観点からも良い設計です。
app/views/welcome/certified_reskill_courses/rails_developer_course/_faq.html.slim (2)
1-22: FAQパーシャルの構造は適切ですHTMLの構造とCSS클래스の使用は一貫性があり、既存のコードスタイルに適合しています。
20-21: MarkdownInitializerの実装確認をお願いしますJavaScript側で
.js-markdown-viewに対してMarkdownInitializerを適用していますが、実際にマークダウン変換処理と必要なサニタイズが行われているかをご確認ください。
- 確認対象ファイル：
app/javascript/markdown.js
new MarkdownInitializer().replace('.js-markdown-view')の呼び出しMarkdownInitializerモジュールの定義箇所を見つけ、
・マークダウン→HTML変換（例：marked 等）が正しく実装されているか
・XSS対策としてHTMLサニタイズが含まれているかapp/views/welcome/faqs/_faqs_empty.html.slim (1)
1-19: 空状態のUIが適切に実装されています管理者向けの空状態表示は、適切なUXパターンを使用しており、コンテンツ管理のためのコールトゥアクションも含まれています。
app/controllers/welcome_controller.rb (2)
8-9: 定数の分割が適切ですFAQ_CATEGORY_NAMEを用途別に分割することで、コードの可読性と保守性が向上しています。
37-37: FAQカテゴリ「法人利用について」はfixturesで定義されているため、nilチェックは不要です
- app/controllers/welcome_controller.rb:37
FAQCategory.find_by(name: FAQ_CATEGORY_NAME_FOR_CORPORATE)は
db/fixtures/faq_categories.yml にfaq_categories7: name: 法人利用についてとして登録済みのため、常に存在します。db/fixtures/faqs.yml (1)
225-240: faq_categories8 は既に定義済みです
db/fixtures/faq_categories.yml の29行目に"faq_categories8:"が存在するため、fixture ロード時にエラーは発生しません。特に修正は不要です。Likely an incorrect or invalid review comment.
e-yanagita-gs
force-pushed
the
chore/replace-tags-implementation
branch
from
3431dd0 to
a5a020e
Compare
|
@smallmonkeykey |
|
@e-yanagita-gs |
|
@smallmonkeykey |
There was a problem hiding this comment.
非React化お疲れ様です！！
かなりレビューに時間がかかってしまいお待たせしました🙇♂️
レビューしましたのでご確認よろしくお願いいたします。
また、動作確認を行ったのですがタグの保存がされない時がありました。
以下はdevelop環境下で試した動画です。
https://www.awesomescreenshot.com/video/41809160?key=c8f0dbc910ddc388f0a299ba743fbd53
保存される時もあるのですが、おそらく早く打つと保存がされないのかなと考えています。
ちなみに本番環境下(私のフィヨルドの実際のページ)でも同様に保存されない時がありました。
https://www.awesomescreenshot.com/video/41808366?key=3275a533becc9ead379974ad358247a4
どこが問題なのかかなり調査したのですが、まだ原因がわからず、火曜日までに一旦お返事すると約束したので、一旦現状どのような状況下であるかをお伝えしました。
恐れ入れますが、保存周りを再度確認いただければと思います。
| @@ -0,0 +1,5 @@ | |||
| export default function transformHeadSharp(text) { | |||
There was a problem hiding this comment.
現在の transformHeadSharp という名前からは具体的な動作が少しわかりにくいため、
- 具体的な名前であれば
removeHeadSharp - 今回のようにもう少し抽象的で目的ベースの名前にするのであれば
extractTagName
の方がより意図が伝わりやすいのではないかと思いました 👀
There was a problem hiding this comment.
フィードバックありがとうございます！
また、removeHeadSharp や extractTagName といった具体的な代替案までいただき、大変参考になります。
おっしゃる通り、関数名としてはご提案いただいた名前の方が処理内容を的確に表していると思います。
その上でご相談なのですが、今回この関数名を維持したいのには理由があります。
実は、この関数は将来的に削除の可能性がある旧ファイル（app/javascript/components/Tags/transform-head-sharp.js）の役割を引き継ぐものです。
旧ファイルの削除は今回のIssueの範囲外と判断し見送ったのですが、いずれ削除する際に、この新しい関数が後継の処理であることが一目で分かるように、あえて同じ名前を採用しています。
このような背景なのですが、このままの関数名で進めさせていただくのはいかがでしょうか。
There was a problem hiding this comment.
なるほどです👀そのような理由があったのですね！
そのような拝見があれば、このままの関数名で良いと思います👍
説明してくださってありがとうございます🙏
app/javascript/tag.js
Outdated
| try { | ||
| const url = `/api/tags.json?taggable_type=${tagsType}` | ||
| const data = await fetcher(url) | ||
| tagify.settings.whitelist = data.map((tag) => tag.value) |
There was a problem hiding this comment.
Tagifyの公式ドキュメントによるとtagify.settings.whitelistではなくtagify.whitelistをのほうを推奨しているみたいなので確認お願いします🙏
https://github.com/yairEO/tagify?tab=readme-ov-file#faq
There was a problem hiding this comment.
tagify.whitelistに修正しました！
ありがとうございます。
| @editable = editable | ||
| end | ||
|
|
||
| def initial_tags |
There was a problem hiding this comment.
質問です🙋♀️
attr_readerを使われているので＠を付けずに呼び出しが可能だとは思うのですが、何か理由はありますか？👀
There was a problem hiding this comment.
ご指摘ありがとうございます！
純粋に不要な@を削除し忘れておりましたので、修正いたしました🙇♀️
|
@e-yanagita-gs |
e-yanagita-gs
force-pushed
the
chore/replace-tags-implementation
branch
from
c3cd028 to
bcb0558
Compare
|
@smallmonkeykey |
|
@e-yanagita-gs タグについてですが、データベースに保存される仕様だと思っています。 ただ、毎回保存に失敗するわけではなく、「入力してすぐに保存ボタンを押したとき」に保存されないことが起きているようでした。 |
e-yanagita-gs
force-pushed
the
chore/replace-tags-implementation
branch
from
bcb0558 to
4e73d6c
Compare
|
レビューありがとうございます。 不具合の発生条件今回の不具合は、タグ入力欄に新しいテキストを入力した後、 不具合の原因従来の実装では、
という2段階のステップになっていました。 しかし、不具合の発生条件を満たす操作を行うと、①の「変数への反映」が終わる前に、②の「サーバーへ送る」処理が先に動いてしまうことがありました。その結果、更新される前の古いデータが保存されてしまう、という問題が発生してしまったと考えられます。 今回の解決策この問題を解決するため、 JavaScriptの変数を経由するのをやめ、保存ボタンが押された瞬間のTagify入力欄の最新の値 ( これにより、処理のタイミングに依存することなく、常にユーザーの入力と同期が取れた最新のデータが保存されるようになります。 修正後、私の開発環境では、従来不具合が発生していた状況でも不具合が解消されていることを確認済みです。 |
|
@e-yanagita-gs |
|
@smallmonkeykey @komagata |
| @@ -0,0 +1,30 @@ | |||
| # frozen_string_literal: true | |||
There was a problem hiding this comment.
TagComponentというとTag表示部分が第一に浮かんでくると思うので「タグの入力」のComponentということを示す名前がいいかなと思いました。
There was a problem hiding this comment.
ありがとうございます！
Tag::TagComponent の名前を、その役割がより明確に分かるようにTag::FormComponent に変更しました。
e-yanagita-gs
force-pushed
the
chore/replace-tags-implementation
branch
from
4e73d6c to
439dada
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/components/tag/form_component.html.slim(1 hunks)app/components/tag/form_component.rb(1 hunks)app/javascript/components/Tags/Tags.jsx(0 hunks)app/javascript/head-is-sharp-or-octothorpe.js(1 hunks)app/javascript/packs/application.js(1 hunks)app/javascript/parse_tags.js(1 hunks)app/javascript/tag.js(1 hunks)app/javascript/transform-head-sharp.js(1 hunks)app/javascript/validate-tag-name.js(1 hunks)app/views/movies/_movie_header.html.slim(1 hunks)app/views/pages/_doc_header.html.slim(1 hunks)app/views/questions/_question_header.html.slim(1 hunks)app/views/users/show.html.slim(1 hunks)
💤 Files with no reviewable changes (1)
- app/javascript/components/Tags/Tags.jsx
✅ Files skipped from review due to trivial changes (1)
- app/components/tag/form_component.rb
🚧 Files skipped from review as they are similar to previous changes (10)
- app/javascript/packs/application.js
- app/javascript/parse_tags.js
- app/javascript/transform-head-sharp.js
- app/views/movies/_movie_header.html.slim
- app/views/users/show.html.slim
- app/javascript/head-is-sharp-or-octothorpe.js
- app/views/questions/_question_header.html.slim
- app/views/pages/_doc_header.html.slim
- app/javascript/validate-tag-name.js
- app/javascript/tag.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: e-yanagita-gs
PR: fjordllc/bootcamp#8881
File: app/javascript/transform-head-sharp.js:1-8
Timestamp: 2025-07-04T01:28:44.174Z
Learning: Tagifyライブラリは空文字列のタグを自動的に追加しないという仕様があるため、変換処理で空文字列を生成することで不要なタグの追加を防ぐことができる。
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変換には自前でヘルパを実装する必要がある。
app/components/tag/form_component.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変換には自前でヘルパを実装する必要がある。
⏰ 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 (3)
app/components/tag/form_component.html.slim (3)
1-5: データ属性の命名とコンテナ構造が適切ですJavaScript統合のためのデータ属性の設計が良好で、タグの初期値、タイプ、IDが適切に設定されています。
8-11: 編集ボタンの条件分岐が適切です
editable?メソッドによる条件分岐で編集ボタンの表示制御が適切に実装されています。
13-13: 属性値のサニタイズ状況を再確認してください現在、
param_nameとinput_idはinitializeでそのまま受け渡され、コンポーネント内で明示的な検証・サニタイズ処理は行われていません。Slim はデフォルトで属性値をエスケープしますが、以下を必ずご確認ください:
- app/components/tag/form_component.rb
–def initialize(taggable:, param_name:, input_id:, …)で受け取る値が全てアプリ側で生成・管理されており、外部（ユーザー）入力が直接渡ることはないか- app/components/tag/form_component.html.slim (行13)
–input.tags-form__input name=param_name id=input_id value=initial_tagsの属性値に意図しない文字列（"や<、>など）が混入する可能性がないか万一、外部入力が混ざる恐れがある場合は、許可する文字種を制限するか、
ERB::Util.html_escapeなどで明示的にサニタイズすることを検討してください。
|
@komagata |
| @@ -0,0 +1,30 @@ | |||
| # frozen_string_literal: true | |||
There was a problem hiding this comment.
view_componentには必ず対応するテストが欲しい感じです。
There was a problem hiding this comment.
ありがとうございます！
追加しました🙇♀️
e-yanagita-gs
force-pushed
the
chore/replace-tags-implementation
branch
from
0c2707c to
b5d0aca
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/components/tag/form_component_test.rb (1)
29-31: editableのテストカバレッジを改善してください現在のテストはeditable: trueの場合のみをテストしています。editable: falseの場合もテストすることでより包括的なテストカバレッジを実現できます。
以下のテストケースを追加することを検討してください：
def test_not_editable component = Tag::FormComponent.new( taggable: @user, param_name: 'user[tag_list]', input_id: 'user_tag_list', editable: false ) refute component.editable? end
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
app/components/tag/form_component.html.slim(1 hunks)app/components/tag/form_component.rb(1 hunks)app/javascript/components/Tags/Tags.jsx(0 hunks)app/javascript/head-is-sharp-or-octothorpe.js(1 hunks)app/javascript/packs/application.js(1 hunks)app/javascript/parse_tags.js(1 hunks)app/javascript/tag.js(1 hunks)app/javascript/transform-head-sharp.js(1 hunks)app/javascript/validate-tag-name.js(1 hunks)app/views/movies/_movie_header.html.slim(1 hunks)app/views/pages/_doc_header.html.slim(1 hunks)app/views/questions/_question_header.html.slim(1 hunks)app/views/users/show.html.slim(1 hunks)test/components/tag/form_component_test.rb(1 hunks)
💤 Files with no reviewable changes (1)
- app/javascript/components/Tags/Tags.jsx
🚧 Files skipped from review as they are similar to previous changes (12)
- app/javascript/parse_tags.js
- app/components/tag/form_component.html.slim
- app/views/movies/_movie_header.html.slim
- app/javascript/packs/application.js
- app/views/questions/_question_header.html.slim
- app/views/users/show.html.slim
- app/components/tag/form_component.rb
- app/javascript/validate-tag-name.js
- app/views/pages/_doc_header.html.slim
- app/javascript/transform-head-sharp.js
- app/javascript/tag.js
- app/javascript/head-is-sharp-or-octothorpe.js
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rb
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
test/**/*
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
📓 Common learnings
Learnt from: e-yanagita-gs
PR: fjordllc/bootcamp#8881
File: app/javascript/transform-head-sharp.js:1-8
Timestamp: 2025-07-04T01:28:44.174Z
Learning: Tagifyライブラリは空文字列のタグを自動的に追加しないという仕様があるため、変換処理で空文字列を生成することで不要なタグの追加を防ぐことができる。
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/components/tag/form_component_test.rb (1)
app/components/tag/form_component.rb (4)
initial_tags(11-13)taggable_type(15-17)taggable_id(19-21)editable?(23-25)
⏰ 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)
test/components/tag/form_component_test.rb (5)
1-15: セットアップが適切に実装されていますViewComponentのテストパターンに従って正しく実装されており、必要なパラメータを持つコンポーネントインスタンスが適切に作成されています。
17-19: initial_tagsメソッドのテストが適切ですタグリストがカンマ区切りの文字列として正しく返されることを検証しており、適切な単体テストになっています。
21-23: taggable_typeメソッドのテストが適切ですtaggableオブジェクトのクラス名が文字列として正しく返されることを検証しています。
25-27: taggable_idメソッドのテストが適切ですIDが文字列形式で正しく返されることを検証しており、JavaScript連携において重要な機能をテストしています。
5-32: テストクラス全体の構造が良好ですコーディングガイドラインに従って、ViewComponentのメソッド追加に対応する単体テストが適切に実装されています。すべての主要メソッドがカバーされており、テストケース名も英語で記述されています。
|
@komagata |
e-yanagita-gs
force-pushed
the
chore/replace-tags-implementation
branch
from
b5c2ab1 to
2d23fac
Compare
|
@komagata |
|
@komagata |
3 participants
Issue
概要
これまでReactで実装されていたタグの表示・編集コンポーネント（
Tags.jsx）を、Railsのview + 少しのVanilla JS（Tagifyライブラリ）の組み合わせに置き換えるリファクタリングを行いました。これにより、特定のページにおけるReactへの依存をなくし、技術スタックの統一を図ります。今回は、UserやMovieなど、複数のモデルで汎用的に利用されているタグ部品を対象としました。
変更確認方法
chore/replace-tags-implementationブランチをローカルに取り込みます。foreman start -f Procfile.devでサーバーを立ち上げます。1. 基本的な表示と編集モードへの切り替え確認
任意の編集権限を持つユーザー（例:
kimura）でログインし、自身のプロフィールページ（例:/users/kimura）にアクセス。/users/tags/（タグ名））も確認。「タグ編集」リンクをクリック。
2. タグの編集・保存・キャンセル機能の確認
上記の手順で編集モードに切り替えた後、新しいタグ（例:
テスト）を入力して追加し、「キャンセル」ボタンをクリック。再度「タグ編集」をクリックし、新しいタグ（例:
テスト）を追加し、既存のタグを一つ削除してから「保存する」ボタンをクリック。ページを再読み込み。
3. バリデーションと入力補助機能の確認
hello worldのようにスペースを含む文字を入力し、Enterキーを押す。#Rubyのように**#を先頭につけた文字**を入力し、Enterキーを押す。#が自動で除去され、Rubyというタグだけが追加されることを確認。4. 複数モデルでの動作確認
上記「1〜3」の確認手順を、以下の各モデルのページで同様に行い、コンポーネントが汎用的に動作することを確認。
/users/kimura）/docs/（任意のDocのID））/questions/（任意のQuestionのID））/movies/（任意のMovieのID））※注意：Movieモデルのリンクエラーについて
Movieモデルのページでタグのリンクをクリックすると、ルーティングエラーが発生します。
これは、元のReact版でも同様の挙動であり、現在本番環境では動画が存在していないため、今回の修正範囲には含まず、意図的にこの状態を維持しています。
Screenshot
機能の置き換えであり、ユーザーから見た挙動や見た目に変更はないため、スクリーンショットはありません。
Summary by CodeRabbit
新機能
リファクタ
削除