Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough教科書機能を追加：ドメインモデル（Textbook/Chapter/Section、ReadingProgress、TermExplanation）、対応するマイグレーション・シード・フィクスチャ、公開／メンター／APIコントローラー、Stimulusフロントエンド、ビュー、ヘルパー、ルーティング、テスト、および機能フラグ用のガードを導入しました。 Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as ブラウザ
participant RP as ReadingProgress<br/>Stimulus
participant API as API::ReadingProgresses<br/>Controller
participant DB as Database
Browser->>RP: セクションをスクロール（data-block-index が可視化）
activate RP
RP->>RP: IntersectionObserver で maxBlock 更新、デバウンス
RP-->>RP: read_ratio/last_block_index/complete 判定
RP->>API: POST /api/textbooks/reading_progresses (JSON + CSRF)
activate API
API->>DB: find_or_initialize_by / save (retry on RecordNotUnique)
DB-->>API: 成功 (201/200) または 422
API-->>RP: JSON レスポンス
deactivate API
alt 完了検出
RP->>Browser: DOM 上で `reading-progress:completed` を dispatch
end
deactivate RP
sequenceDiagram
participant Browser as ブラウザ
participant TTC as TermTooltip<br/>Stimulus
participant API as API::TermExplanations<br/>Controller
participant DB as Database
Browser->>TTC: 用語リンクをクリック (term, sectionId)
activate TTC
alt キャッシュヒット
TTC-->>Browser: キャッシュ済み説明を表示
else キャッシュミス
TTC->>API: GET /api/textbooks/term_explanations/:sectionId?term=...
activate API
API->>DB: TermExplanation を検索
DB-->>API: JSON または 404
API-->>TTC: JSON (explanation) または 404
deactivate API
TTC->>TTC: 取得結果をキャッシュしてツールチップ表示
end
deactivate TTC
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 19
🧹 Nitpick comments (16)
app/views/mentor/textbooks/sections/_form.html.slim (2)
47-51: submitボタンにtype="submit"を明示的に指定
<button>タグはデフォルトでtype="submit"ですが、明示的に指定することでコードの意図が明確になります。♻️ 提案する修正
- button.a-button.is-lg.is-primary.is-block + = f.submit '内容を保存', class: 'a-button is-lg is-primary is-block'Railsの
f.submitを使用することで、フォームビルダーとの一貫性が保たれます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/mentor/textbooks/sections/_form.html.slim` around lines 47 - 51, Replace the bare <button> element with the form builder's submit helper to make the intent explicit and keep consistency with the form builder: use f.submit with the label "内容を保存" and pass the same classes ('a-button is-lg is-primary is-block') and ensure type: 'submit' is set (and that it's rendered inside the form builder block where `f` is defined) so the button behavior and styling remain identical to the original.
1-2: インスタンス変数とローカル変数の混在Line 1で
@section（インスタンス変数）を使用していますが、Line 2以降ではsection（ローカル変数）を使用しています。パーシャルがsection:@section``で呼び出される前提ですが、一貫性のためにローカル変数を使用することを推奨します。♻️ 提案する修正
-= form_with model: [:mentor, `@textbook`, `@chapter`, `@section`], local: true, class: 'form', html: { name: 'section' } do |f| += form_with model: [:mentor, section.textbook_chapter.textbook, section.textbook_chapter, section], local: true, class: 'form', html: { name: 'section' } do |f|または、呼び出し元から
textbookとchapterもローカル変数として渡す方法も検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/mentor/textbooks/sections/_form.html.slim` around lines 1 - 2, The partial mixes instance and local variables: update the form to use the local variable `section` instead of `@section` so it matches the subsequent `render 'errors', object: section`; specifically change the `form_with` model argument (currently `[:mentor, `@textbook`, `@chapter`, `@section`]`) to use the local `section` (and consider also using local `textbook` and `chapter` in that model array if those are passed in as locals) so all usages (`form_with` and `render 'errors', object: section`) are consistent.app/views/mentor/textbooks/chapters/_form.html.slim (2)
1-2: インスタンス変数とローカル変数の混在
sections/_form.html.slimと同様に、Line 1で@textbook、@chapter（インスタンス変数）を使用していますが、Line 2ではchapter（ローカル変数）を使用しています。♻️ 提案する修正
-= form_with model: [:mentor, `@textbook`, `@chapter`], local: true, class: 'form', html: { name: 'chapter' } do |f| - = render 'errors', object: chapter += form_with model: [:mentor, chapter.textbook, chapter], local: true, class: 'form', html: { name: 'chapter' } do |f| + = render 'errors', object: chapterまたは、呼び出し元から
textbookもローカル変数として渡すことを検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/mentor/textbooks/chapters/_form.html.slim` around lines 1 - 2, form と errors 部分でインスタンス変数とローカル変数が混在しているので一貫させてください: 現状の form_with 呼び出し uses `@textbook` と `@chapter` を参照しているため render 'errors', object: chapter を object: `@chapter` に置き換えるか、逆に form_with のモデル引数をローカル変数版 (textbook, chapter) に変更して両方をローカルに揃えるよう修正してください；参照箇所は form_with model: [:mentor, `@textbook`, `@chapter`] と render 'errors', object: chapter を確認して適切に統一してください。
17-19: Railsフォームヘルパーの使用を推奨プレーンな
<button>タグの代わりにf.submitを使用することで、フォームビルダーとの一貫性が保たれます。♻️ 提案する修正
li.form-actions__item.is-main - button.a-button.is-lg.is-primary.is-block - | 内容を保存 + = f.submit '内容を保存', class: 'a-button is-lg is-primary is-block'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/mentor/textbooks/chapters/_form.html.slim` around lines 17 - 19, The template uses a plain <button> element for submitting the form; replace it with the form builder's submit helper (f.submit) to keep consistency with the Rails form builder. Locate the submit button inside the form partial (the li with class form-actions__item.is-main and the button with classes a-button.is-lg.is-primary.is-block) and change it to call f.submit, preserving the button label and CSS classes by passing the label ("内容を保存") and class option (class: "a-button is-lg is-primary is-block") to f.submit.app/views/mentor/textbooks/_form.html.slim (1)
1-2: Line 2 でtextbookローカル変数と Line 1 の@textbookインスタンス変数の混在を統一してください。
new.html.slimとedit.html.slimは両方ともtextbook:@textbook`` を渡しているため、現在は動作していますが、form ブロック内ではf.objectを使用することで、より堅牢で一貫性のあるコードになります。= form_with model: [:mentor, `@textbook`], local: true, class: 'form', html: { name: 'textbook' } do |f| - = render 'errors', object: textbook + = render 'errors', object: f.object🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/mentor/textbooks/_form.html.slim` around lines 1 - 2, The form partial mixes the instance variable `@textbook` with a local textbook when rendering errors; update the partial to use the form object instead of the external variable: replace uses of textbook (e.g. in the render 'errors' call) with f.object (from the form_with block) so the partial consistently references the form's model (symbols to locate: form_with, `@textbook`, render 'errors', and f.object).test/models/reading_progress_test.rb (1)
30-36:reloadしないとcomplete!の保存漏れを拾えません。このテストは同じ
progressインスタンスを見ているだけなので、complete!が DB に保存していなくても通ります。永続化まで契約に含める想定ならreload後に確認したいです。修正案
progress = reading_progresses(:reading_progress_komagata_variables) progress.complete! + progress.reload assert progress.completed? assert_equal 1.0, progress.read_ratio assert_not_nil progress.last_read_at🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/models/reading_progress_test.rb` around lines 30 - 36, テストが同一の progress オブジェクトだけを見ているため complete! が DB に保存されていなくても通ってしまう問題があるので、complete! を呼んだ後に DB の状態を検証するために progress.reload を挿入して永続化を確認してください（対象: the test method and the progress variable from reading_progresses(:reading_progress_komagata_variables) and the complete! invocation; after calling complete! call progress.reload and then assert completed?, read_ratio and last_read_at on the reloaded record).config/routes/mentor.rb (1)
18-22: インデントの不整合があります。
textbooksリソースとその内部のネストされたリソースのインデントが不整合です。namespace :mentorブロック内では2スペースインデントを使用すべきですが、chaptersとsectionsのインデントが過剰になっています。♻️ インデント修正案
- resources :textbooks, only: %i[index new create edit update destroy] do - resources :chapters, only: %i[new create edit update destroy], controller: 'textbooks/chapters' do - resources :sections, only: %i[new create edit update destroy], controller: 'textbooks/sections' - end - end + resources :textbooks, only: %i[index new create edit update destroy] do + resources :chapters, only: %i[new create edit update destroy], controller: 'textbooks/chapters' do + resources :sections, only: %i[new create edit update destroy], controller: 'textbooks/sections' + end + end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/routes/mentor.rb` around lines 18 - 22, resources :textbooks とネストされた resources :chapters／resources :sections のインデントが namespace :mentor ブロックの2スペース基準からずれているため整列させてください; 対象は resources :textbooks, do ブロックとその中の resources :chapters (controller: 'textbooks/chapters') およびその中の resources :sections (controller: 'textbooks/sections') で、各ネストレベルごとに2スペースずつインデントを揃えるように修正してください。test/models/textbook/section_test.rb (1)
39-43: デフォルトスコープのテストについて現在のテストは
positionsがソートされていることを確認していますが、テストデータ自体が既にソート順で定義されている場合、このテストは常にパスします。より堅牢なテストとして、明示的に異なる位置のセクションを作成して検証することを検討してください。♻️ より堅牢なテスト案
test 'default scope orders by position' do chapter = textbook_chapters(:chapter_ruby_basics) # 位置を明示的に逆順で作成 section_b = Textbook::Section.create!( title: 'B', body: 'content', chapter: chapter, position: 2 ) section_a = Textbook::Section.create!( title: 'A', body: 'content', chapter: chapter, position: 1 ) sections = chapter.sections.reload positions = sections.map(&:position) assert_equal positions.sort, positions end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/models/textbook/section_test.rb` around lines 39 - 43, The test 'default scope orders by position' currently relies on fixture order; update it to create Textbook::Section records with explicit, out-of-order position values against textbook_chapters(:chapter_ruby_basics) (e.g., create a section with position 2 then one with position 1), reload chapter.sections, and assert that the returned positions are sorted (use positions.sort == positions) to verify the default scope orders by position; reference the test name, the Textbook::Section model, and chapter.sections when making the change.app/models/textbook/section.rb (1)
14-14:default_scopeの使用について検討
default_scopeは便利ですが、以下の問題を引き起こす可能性があります：
- すべてのクエリに暗黙的に適用される
unscopedを使わないと解除できない- JOINや関連クエリで予期しない動作を引き起こすことがある
代替案として、明示的なスコープの定義も検討できます：
scope :ordered, -> { order(:position) }ただし、セクションは常に位置順で表示されるユースケースが明確であれば、現在の実装でも問題ありません。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/models/textbook/section.rb` at line 14, The use of default_scope { order(:position) } in the Section model can cause implicit ordering to leak into all queries; replace it with an explicit scope by removing default_scope and adding scope :ordered, -> { order(:position) } in the Section class (or keep default_scope only after confirming all callers rely on implicit ordering), then update call sites to use Section.ordered (or association chains like textbook.sections.ordered) where order is required; if you must keep default_scope, add a comment explaining why and where unscoped should be used to avoid surprises.app/views/textbooks/show.html.slim (2)
39-44: 次セクション検索ロジックもDecoratorへ移動を推奨
next_sectionの計算ロジックもビューではなくDecoratorに移動することで、テスタビリティと再利用性が向上します。♻️ Decoratorでの実装例
# app/decorators/textbook_decorator.rb class TextbookDecorator < ApplicationDecorator def next_incomplete_section(user) return nil unless user completed_ids = completed_section_ids(user) all_sections.find { |s| !completed_ids.include?(s.id) } end private def all_sections chapters.flat_map(&:sections) end end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/textbooks/show.html.slim` around lines 39 - 44, Move the next-section lookup out of the view into the TextbookDecorator: implement a method like next_incomplete_section(user) on TextbookDecorator that returns nil if user is blank, computes completed_section_ids(user) (or reuses the existing completed_section_ids helper) and finds the first section from chapters.flat_map(&:sections) whose id is not in completed IDs; then replace the inline logic in the view that defines next_section with a call to `@textbook.decorate.next_incomplete_section`(current_user) and use that result for the link rendering.
19-26: ビューにビジネスロジックが含まれています。Decoratorパターンの使用を検討してください。
completed_idsやcompleted_countの計算、データベースクエリ（reading_progresses.where.pluck）がビュー内で実行されています。コーディングガイドラインに従い、active_decoratorgemを使ったDecoratorパターンへの移行を検討してください。例えば、
TextbookDecoratorで以下のようなメソッドを提供できます：
completed_section_ids(user)completed_count(user)progress_text(user)コーディングガイドラインに基づく: 「ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）」
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/textbooks/show.html.slim` around lines 19 - 26, Move the business logic out of the view into a TextbookDecorator: remove the all_sections, completed_ids, completed_count and DB query from show.html.slim and implement methods on TextbookDecorator such as completed_section_ids(user), completed_count(user) and progress_text(user) that perform the sections collection and reading_progresses.where(...).pluck(...) logic using active_decorator; then call those decorator methods from the view (e.g., textbook.decorate.completed_count(current_user) / completed_section_ids(current_user) / progress_text(current_user)) so the view only renders presentation data.app/views/textbooks/sections/show.html.slim (2)
20-28: 進捗バーの計算ロジックをDecoratorへ移動を検討
progress_percentの計算や現在のインデックス取得はDecoratorに移動することで、ビューをシンプルに保てます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/textbooks/sections/show.html.slim` around lines 20 - 28, View contains logic computing sections, current_index and progress_percent; move this into a decorator to keep the view clean. Add a method on an appropriate decorator (e.g., ChapterDecorator#progress_percent and ChapterDecorator#section_position_label or SectionDecorator#progress_percent) that accepts the current section (or uses decorated association) and returns the computed percentage and a formatted "title (x/y)" label; then replace the inline computation in show.html.slim to call those decorator methods (use the decorator instance for `@chapter` or `@section`) and render the returned values. Ensure the methods reference the same symbols used in the view: `@chapter.sections`, `@section`, progress_percent, and the position label so behavior remains identical.
53-56: 前後セクションナビゲーションのロジックが重複しています
all_sectionsの取得と前後セクションの計算はtextbooks/show.html.slimと同様のパターンです。共通のDecoratorメソッドまたはヘルパーに抽出することで、DRY原則に沿った実装になります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/textbooks/sections/show.html.slim` around lines 53 - 56, このビューにある all_sections = `@textbook.chapters.flat_map`(&:sections) と prev_section / next_section の計算は重複しているので、@textbook に対する共通メソッドに抽出してください。例えば TextbookDecorator または ViewHelper に sections_for_navigation(textbook) を作り、その中で flat_map(&:sections) を行い、previous_section_for(section, sections) と next_section_for(section, sections) を実装して戻り値を返すようにし、view 側では all_sections / prev_section / next_section の直接計算を置き換えて呼び出すだけにしてください。これにより textbooks/show.html.slim とこの show.html.slim 両方で同じロジックを再利用できます。app/controllers/mentor/textbooks_controller.rb (2)
31-33: updateアクションも同様♻️ 修正案
if `@textbook.update`(textbook_params) redirect_to mentor_textbooks_path, notice: '教科書を更新しました。' else - render :edit + render :edit, status: :unprocessable_entity end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/mentor/textbooks_controller.rb` around lines 31 - 33, In the update action of Mentor::TextbooksController, mirror the same failure handling used elsewhere by returning an appropriate HTTP status when rendering the edit template: replace the plain render :edit in the update method with render :edit, status: :unprocessable_entity (i.e., update the update method to call render :edit, status: :unprocessable_entity when the update fails) so clients receive a 422 on validation failures.
21-23: Turbo互換性のためステータスコード追加を検討
Mentor::Textbooks::SectionsControllerと同様に、フォームエラー時にstatus: :unprocessable_entityの追加を検討してください。♻️ 修正案
if `@textbook.save` redirect_to mentor_textbooks_path, notice: '教科書を作成しました。' else - render :new + render :new, status: :unprocessable_entity end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/mentor/textbooks_controller.rb` around lines 21 - 23, In the error branch that currently calls render :new (in Mentor::TextbooksController), add the HTTP status for Turbo compatibility by changing the render call to include status: :unprocessable_entity; locate the render :new usage inside the action(s) that handle form submission (e.g., create/update methods in Mentor::TextbooksController) and update them to render :new, status: :unprocessable_entity to match the behavior used in Mentor::Textbooks::SectionsController.app/controllers/mentor/textbooks/sections_controller.rb (1)
29-31: updateアクションも同様にステータスコード追加を検討♻️ 修正案
if `@section.update`(section_params) redirect_to mentor_textbook_path(`@textbook`), notice: '節を更新しました。' else - render :edit + render :edit, status: :unprocessable_entity end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/mentor/textbooks/sections_controller.rb` around lines 29 - 31, updateアクション（SectionsController#update）にもHTTPステータスを明示的に付与してください: 成功時のredirect_toに適切なステータス（例: :see_other または :ok）を追加し、失敗時の render :edit には :unprocessable_entity を設定してクライアントがエラー状態を判別できるようにしてください。該当箇所は SectionsController の update メソッド内の redirect_to と render :edit を修正してください。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/controllers/api/textbooks/reading_progresses_controller.rb`:
- Around line 4-11: createアクションは(user_id,
textbook_section_id)のユニーク制約違反でActiveRecord::RecordNotUniqueが発生する可能性があるため、ReadingProgressControllerのcreateメソッドで単純な
new+save を使うのではなく、find_or_initialize_by(user: current_user, textbook_section_id:
reading_progress_params[:textbook_section_id]) を使って既存レコードを取得または新規生成し、そのインスタンスに
reading_progress_params を割り当てて保存するよう変更してください；また念のため
ActiveRecord::RecordNotUnique を rescue
して既存レコードを再取得して処理を続ける（または適切なHTTPステータスを返す）ようにして、メソッド名 create と
reading_progress_params、current_user、ReadingProgress を参照して修正してください。
In `@app/controllers/api/textbooks/term_explanations_controller.rb`:
- Around line 3-6: The API controller API::Textbooks::TermExplanationsController
lacks the textbook feature flag guard; include the TextbookFeatureGuard module
and add a before_action :require_textbook_enabled in the controller (same
approach used by textbooks_controller.rb) so the endpoint returns 404 when the
Switchlet textbook feature is disabled; apply the same change pattern to any
sibling API controllers like API::Textbooks::ReadingProgressesController to
ensure consistent behavior.
In `@app/helpers/textbook_helper.rb`:
- Around line 39-53: The code only iterates immediate child text nodes with
node.children.each so text inside inline elements like <strong> or <em> is
skipped; change the iteration to traverse descendant text nodes (e.g. replace
node.children.each with node.xpath('.//text()').each or node.traverse and filter
text nodes) so that the existing replacement logic using pattern,
ERB::Util.html_escape, and
child.replace(Nokogiri::HTML::DocumentFragment.parse(replaced)) runs for text
inside inline elements as well; keep the same variables (node, child, pattern,
section) and the same replacement construction.
- Around line 34-37: 現在の terms.each
で短い語が先に置換されているため「パイプ」と「パイプライン」のような重複語で後続の長い語の tooltip が壊れています。terms
をイテレーションする前に長い語順（長さ降順）にソートしてからループ（参照箇所: terms.each と pattern =
Regexp.new(Regexp.escape(term), Regexp::IGNORECASE)）するように変更し、引き続き term.blank?
チェックを適用してください。
In `@app/javascript/controllers/piyo_companion_controller.js`:
- Around line 23-41: The event listener added in connect() uses
this.handleCompletion.bind(this) which creates a new function each call so
disconnect() cannot remove it; fix by creating and storing a single bound
reference (e.g. this.handleCompletionBound = this.handleCompletion.bind(this))
when connecting, use that stored symbol in addEventListener inside connect(),
and call removeEventListener with the same stored this.handleCompletionBound in
disconnect(); also clear or null out the stored bound reference when tearing
down (and leave dismissTimer clearing as-is).
- Line 1: package.jsonのStimulus依存を非公式の"stimulus"から公式"@hotwired/stimulus":
"^3.2.2"に置き換え、すべてのコントローラーのimport文を更新してください（例:
app/javascript/controllers/piyo_companion_controller.js の import { Controller }
from 'stimulus' を import { Controller } from '@hotwired/stimulus'
に変更）。package.jsonの依存セクションを修正し、app/javascript/controllers/ディレクトリ内の全ファイルで'Controller'をインポートしている箇所を検索して'@hotwired/stimulus'に統一してください。
In `@app/javascript/controllers/reading_progress_controller.js`:
- Around line 60-66: The updateProgress logic can send last_block_index = -1
when this.maxBlockSeen remains -1; before computing readRatio and before
building/sending the payload (the code that reads this.maxBlockSeen around
updateProgress and the later send block where last_block_index is added), add a
guard that treats negative maxBlockSeen as "no blocks seen"—either skip sending
last_block_index or set last_block_index to a non-negative sentinel (e.g. 0 or
null) and ensure readRatio uses Math.max(this.maxBlockSeen + 1, 0) /
this.totalBlocksValue so you never send -1; apply the same guard to the similar
code referenced at lines 76-80.
- Around line 70-84: Replace the raw fetch call in
reading_progress_controller.js (the fetch('/api/textbooks/reading_progresses', {
... }) block) with the centralized request.js helper by calling its post()
function so CSRF headers and response parsing are handled consistently; pass the
same payload object under reading_progress with textbook_section_id:
this.sectionIdValue, last_block_index: this.maxBlockSeen, read_ratio:
Math.min(readRatio, 1.0), and completed: isCompleted || this.completed, and
ensure you handle the returned promise/result the same way other callers of
request.post() do (e.g., .then/.catch or async/await) to maintain unified error
handling and token injection.
In `@app/javascript/controllers/term_tooltip_controller.js`:
- Around line 88-89: The click listener added with
document.addEventListener('click', this.handleOutsideClick, { once: true }) can
survive until the next click; ensure you explicitly remove it wherever the
tooltip is torn down: update removeTooltip() (and the disconnect() path) to call
document.removeEventListener('click', this.handleOutsideClick) and likewise
remove any other symmetric listeners created in the block around lines 97-102;
reference the handleOutsideClick handler when removing so the exact listener is
detached.
- Around line 43-51: Replace the direct fetch call in term_tooltip_controller.js
with the project's request.js get() helper: call
get(`/api/textbooks/term_explanations/${encodeURIComponent(this.sectionIdValue)}?term=${encodeURIComponent(term)}`)
(do not pass CSRF headers), await the result, and access the JSON payload via
the response.json getter (i.e. response.json, not response.json()). Update any
variable names accordingly (term, this.sectionIdValue) and remove the manual
CSRF token header logic.
In `@app/models/textbook/chapter.rb`:
- Around line 9-12: The Chapter model currently allows duplicate position values
per textbook which makes ordering unstable; update the ActiveRecord validation
on :position to enforce uniqueness scoped to :textbook_id (e.g., validates
:position, uniqueness: { scope: :textbook_id }) and keep the existing
presence/numericality checks, and add a migration that creates a unique DB index
on [:textbook_id, :position] (add_index :chapters, [:textbook_id, :position],
unique: true) so the constraint is enforced at the database level; ensure the
migration handles existing duplicates (cleanup or backfill) before adding the
unique index.
In `@app/views/mentor/textbooks/_form.html.slim`:
- Around line 18-21: f.label が属性名を第1引数に取っていないため f.check_box :published
と紐づいていません。f.label を f.label :published, '教科書を公開する', class: 'a-form-label'
の形式に変更して、既存の f.check_box :published と正しく関連付けてください（対象のシンボル: f.label, f.check_box
:published）。
In `@app/views/mentor/textbooks/edit.html.slim`:
- Around line 40-49: The controller's set_textbook currently does `@textbook` =
Textbook.find(params[:id]) causing N+1 queries in the view; update
Mentor::TextbooksController#set_textbook to eager-load associations by using
ActiveRecord includes for chapters and their sections (i.e., load chapters:
:sections) when finding the textbook so `@textbook.chapters.each` and
chapter.sections.each do not trigger extra queries.
In `@app/views/mentor/textbooks/index.html.slim`:
- Around line 42-45: The current link_to in the mentor list uses the public
textbook_path(textbook), causing 404s for unpublished records; update the
template loop (the `@textbooks.each` block and the link_to textbook.title
expression) to point to the mentor-side route (e.g.,
mentor_textbook_path(textbook)) or conditionally choose the route: if
textbook.published? use textbook_path(textbook) else use
mentor_textbook_path(textbook); replace the existing link_to expression
accordingly so unpublished textbooks link to the mentor detail/edit page instead
of the public show.
- Around line 54-60: The edit/delete icon links (link_to calls for
edit_mentor_textbook_path(textbook) and mentor_textbook_path(textbook)) lack
accessible labels; add an aria-label (e.g., aria-label: "教材を編集" for the edit
link and aria-label: "教材を削除" for the delete link) to each link_to helper so
screen readers announce their purpose, and keep the existing classes/data
attributes intact (ensure the delete link still has data: { confirm: ... } and
method: :delete).
In `@app/views/textbooks/chapters/show.html.slim`:
- Around line 1-4: Move the redirect logic out of the view: remove the lines
using `@chapter.sections.first` and redirect_to from
app/views/textbooks/chapters/show.html.slim and add the early-redirect in the
ChaptersController#show action
(app/controllers/textbooks/chapters_controller.rb); inside show, load `@textbook`
and `@chapter` as before, compute first_section = `@chapter.sections.first` and if
present call redirect_to textbook_chapter_section_path(`@textbook`, `@chapter`,
first_section) and return, otherwise allow the view to render normally (keep the
view focused on rendering the "no sections" message and title).
In `@app/views/textbooks/index.html.slim`:
- Around line 33-39: The view is triggering an N+1 by iterating
textbook.chapters.flat_map(&:sections) and calling
reading_progresses.exists?(user: current_user, completed: true) per section
(inside the current_user.present? block calculating total and completed); move
this aggregation into the controller or decorator (e.g., compute total_sections
and completed_sections_count for each textbook once using a single query that
joins sections -> reading_progresses scoped to current_user and completed) and
pass those precomputed values to the view, then replace the textbook-local
per-section checks with the provided completed_sections_count and total_sections
values (use the same variable names total and completed in the view to minimize
changes).
In `@config/routes/textbook.rb`:
- Around line 3-16: This file incorrectly rewraps routes with
Rails.application.routes.draw; remove that outer wrapper so the file exposes
route DSL directly (i.e., start with resources :textbooks ... and the namespace
:api ... blocks) so it can be used via Rails' draw(:textbook) mechanism; keep
the existing route declarations (resources :textbooks, nested resources
:chapters and :sections and the namespace :api with resources
:reading_progresses and :term_explanations) and their controller options
unchanged.
In `@db/seeds/textbook_linux.rb`:
- Around line 5-8: The seed uses Textbook.find_or_create_by! with a
creation-only block so updates to description, published, or position aren't
applied on re-seed; change to use Textbook.find_or_initialize_by(title: 'Linux')
(or find_or_create_by without a block) then assign t.description, t.published,
t.position on the returned record and call save! or update! so existing records
are updated when the seed is re-run (apply same pattern to the other affected
blocks around 545-557).
---
Nitpick comments:
In `@app/controllers/mentor/textbooks_controller.rb`:
- Around line 31-33: In the update action of Mentor::TextbooksController, mirror
the same failure handling used elsewhere by returning an appropriate HTTP status
when rendering the edit template: replace the plain render :edit in the update
method with render :edit, status: :unprocessable_entity (i.e., update the update
method to call render :edit, status: :unprocessable_entity when the update
fails) so clients receive a 422 on validation failures.
- Around line 21-23: In the error branch that currently calls render :new (in
Mentor::TextbooksController), add the HTTP status for Turbo compatibility by
changing the render call to include status: :unprocessable_entity; locate the
render :new usage inside the action(s) that handle form submission (e.g.,
create/update methods in Mentor::TextbooksController) and update them to render
:new, status: :unprocessable_entity to match the behavior used in
Mentor::Textbooks::SectionsController.
In `@app/controllers/mentor/textbooks/sections_controller.rb`:
- Around line 29-31:
updateアクション（SectionsController#update）にもHTTPステータスを明示的に付与してください:
成功時のredirect_toに適切なステータス（例: :see_other または :ok）を追加し、失敗時の render :edit には
:unprocessable_entity を設定してクライアントがエラー状態を判別できるようにしてください。該当箇所は SectionsController
の update メソッド内の redirect_to と render :edit を修正してください。
In `@app/models/textbook/section.rb`:
- Line 14: The use of default_scope { order(:position) } in the Section model
can cause implicit ordering to leak into all queries; replace it with an
explicit scope by removing default_scope and adding scope :ordered, -> {
order(:position) } in the Section class (or keep default_scope only after
confirming all callers rely on implicit ordering), then update call sites to use
Section.ordered (or association chains like textbook.sections.ordered) where
order is required; if you must keep default_scope, add a comment explaining why
and where unscoped should be used to avoid surprises.
In `@app/views/mentor/textbooks/_form.html.slim`:
- Around line 1-2: The form partial mixes the instance variable `@textbook` with a
local textbook when rendering errors; update the partial to use the form object
instead of the external variable: replace uses of textbook (e.g. in the render
'errors' call) with f.object (from the form_with block) so the partial
consistently references the form's model (symbols to locate: form_with,
`@textbook`, render 'errors', and f.object).
In `@app/views/mentor/textbooks/chapters/_form.html.slim`:
- Around line 1-2: form と errors 部分でインスタンス変数とローカル変数が混在しているので一貫させてください: 現状の
form_with 呼び出し uses `@textbook` と `@chapter` を参照しているため render 'errors', object:
chapter を object: `@chapter` に置き換えるか、逆に form_with のモデル引数をローカル変数版 (textbook,
chapter) に変更して両方をローカルに揃えるよう修正してください；参照箇所は form_with model: [:mentor, `@textbook`,
`@chapter`] と render 'errors', object: chapter を確認して適切に統一してください。
- Around line 17-19: The template uses a plain <button> element for submitting
the form; replace it with the form builder's submit helper (f.submit) to keep
consistency with the Rails form builder. Locate the submit button inside the
form partial (the li with class form-actions__item.is-main and the button with
classes a-button.is-lg.is-primary.is-block) and change it to call f.submit,
preserving the button label and CSS classes by passing the label ("内容を保存") and
class option (class: "a-button is-lg is-primary is-block") to f.submit.
In `@app/views/mentor/textbooks/sections/_form.html.slim`:
- Around line 47-51: Replace the bare <button> element with the form builder's
submit helper to make the intent explicit and keep consistency with the form
builder: use f.submit with the label "内容を保存" and pass the same classes
('a-button is-lg is-primary is-block') and ensure type: 'submit' is set (and
that it's rendered inside the form builder block where `f` is defined) so the
button behavior and styling remain identical to the original.
- Around line 1-2: The partial mixes instance and local variables: update the
form to use the local variable `section` instead of `@section` so it matches the
subsequent `render 'errors', object: section`; specifically change the
`form_with` model argument (currently `[:mentor, `@textbook`, `@chapter`,
`@section`]`) to use the local `section` (and consider also using local `textbook`
and `chapter` in that model array if those are passed in as locals) so all
usages (`form_with` and `render 'errors', object: section`) are consistent.
In `@app/views/textbooks/sections/show.html.slim`:
- Around line 20-28: View contains logic computing sections, current_index and
progress_percent; move this into a decorator to keep the view clean. Add a
method on an appropriate decorator (e.g., ChapterDecorator#progress_percent and
ChapterDecorator#section_position_label or SectionDecorator#progress_percent)
that accepts the current section (or uses decorated association) and returns the
computed percentage and a formatted "title (x/y)" label; then replace the inline
computation in show.html.slim to call those decorator methods (use the decorator
instance for `@chapter` or `@section`) and render the returned values. Ensure the
methods reference the same symbols used in the view: `@chapter.sections`,
`@section`, progress_percent, and the position label so behavior remains
identical.
- Around line 53-56: このビューにある all_sections =
`@textbook.chapters.flat_map`(&:sections) と prev_section / next_section
の計算は重複しているので、@textbook に対する共通メソッドに抽出してください。例えば TextbookDecorator または ViewHelper
に sections_for_navigation(textbook) を作り、その中で flat_map(&:sections)
を行い、previous_section_for(section, sections) と next_section_for(section,
sections) を実装して戻り値を返すようにし、view 側では all_sections / prev_section / next_section
の直接計算を置き換えて呼び出すだけにしてください。これにより textbooks/show.html.slim とこの show.html.slim
両方で同じロジックを再利用できます。
In `@app/views/textbooks/show.html.slim`:
- Around line 39-44: Move the next-section lookup out of the view into the
TextbookDecorator: implement a method like next_incomplete_section(user) on
TextbookDecorator that returns nil if user is blank, computes
completed_section_ids(user) (or reuses the existing completed_section_ids
helper) and finds the first section from chapters.flat_map(&:sections) whose id
is not in completed IDs; then replace the inline logic in the view that defines
next_section with a call to
`@textbook.decorate.next_incomplete_section`(current_user) and use that result for
the link rendering.
- Around line 19-26: Move the business logic out of the view into a
TextbookDecorator: remove the all_sections, completed_ids, completed_count and
DB query from show.html.slim and implement methods on TextbookDecorator such as
completed_section_ids(user), completed_count(user) and progress_text(user) that
perform the sections collection and reading_progresses.where(...).pluck(...)
logic using active_decorator; then call those decorator methods from the view
(e.g., textbook.decorate.completed_count(current_user) /
completed_section_ids(current_user) / progress_text(current_user)) so the view
only renders presentation data.
In `@config/routes/mentor.rb`:
- Around line 18-22: resources :textbooks とネストされた resources :chapters／resources
:sections のインデントが namespace :mentor ブロックの2スペース基準からずれているため整列させてください; 対象は
resources :textbooks, do ブロックとその中の resources :chapters (controller:
'textbooks/chapters') およびその中の resources :sections (controller:
'textbooks/sections') で、各ネストレベルごとに2スペースずつインデントを揃えるように修正してください。
In `@test/models/reading_progress_test.rb`:
- Around line 30-36: テストが同一の progress オブジェクトだけを見ているため complete! が DB
に保存されていなくても通ってしまう問題があるので、complete! を呼んだ後に DB の状態を検証するために progress.reload
を挿入して永続化を確認してください（対象: the test method and the progress variable from
reading_progresses(:reading_progress_komagata_variables) and the complete!
invocation; after calling complete! call progress.reload and then assert
completed?, read_ratio and last_read_at on the reloaded record).
In `@test/models/textbook/section_test.rb`:
- Around line 39-43: The test 'default scope orders by position' currently
relies on fixture order; update it to create Textbook::Section records with
explicit, out-of-order position values against
textbook_chapters(:chapter_ruby_basics) (e.g., create a section with position 2
then one with position 1), reload chapter.sections, and assert that the returned
positions are sorted (use positions.sort == positions) to verify the default
scope orders by position; reference the test name, the Textbook::Section model,
and chapter.sections when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f6dfbf43-7ae5-42b6-8626-a36f73806e4d
📒 Files selected for processing (50)
app/controllers/api/textbooks/reading_progresses_controller.rbapp/controllers/api/textbooks/term_explanations_controller.rbapp/controllers/concerns/textbook_feature_guard.rbapp/controllers/mentor/textbooks/chapters_controller.rbapp/controllers/mentor/textbooks/sections_controller.rbapp/controllers/mentor/textbooks_controller.rbapp/controllers/textbooks/chapters_controller.rbapp/controllers/textbooks/sections_controller.rbapp/controllers/textbooks_controller.rbapp/helpers/textbook_helper.rbapp/javascript/controllers/piyo_companion_controller.jsapp/javascript/controllers/reading_progress_controller.jsapp/javascript/controllers/term_tooltip_controller.jsapp/models/reading_progress.rbapp/models/term_explanation.rbapp/models/textbook.rbapp/models/textbook/chapter.rbapp/models/textbook/section.rbapp/models/user.rbapp/views/mentor/textbooks/_form.html.slimapp/views/mentor/textbooks/chapters/_form.html.slimapp/views/mentor/textbooks/chapters/edit.html.slimapp/views/mentor/textbooks/chapters/new.html.slimapp/views/mentor/textbooks/edit.html.slimapp/views/mentor/textbooks/index.html.slimapp/views/mentor/textbooks/new.html.slimapp/views/mentor/textbooks/sections/_form.html.slimapp/views/mentor/textbooks/sections/edit.html.slimapp/views/mentor/textbooks/sections/new.html.slimapp/views/shared/_piyo_companion.html.slimapp/views/textbooks/chapters/show.html.slimapp/views/textbooks/index.html.slimapp/views/textbooks/sections/show.html.slimapp/views/textbooks/show.html.slimconfig/routes.rbconfig/routes/mentor.rbconfig/routes/textbook.rbdb/migrate/20260326000000_create_textbook_tables.rbdb/seeds.rbdb/seeds/textbook_linux.rbtest/fixtures/reading_progresses.ymltest/fixtures/term_explanations.ymltest/fixtures/textbook_chapters.ymltest/fixtures/textbook_sections.ymltest/fixtures/textbooks.ymltest/models/reading_progress_test.rbtest/models/term_explanation_test.rbtest/models/textbook/chapter_test.rbtest/models/textbook/section_test.rbtest/models/textbook_test.rb
app/views/textbooks/index.html.slim
Outdated
Comment on lines
+33
to
+39
| - if current_user.present? | ||
| - total = textbook.chapters.flat_map(&:sections).size | ||
| - completed = textbook.chapters.flat_map(&:sections).count { |s| s.reading_progresses.exists?(user: current_user, completed: true) } | ||
| - if total > 0 | ||
| .card-list__item-option | ||
| span | ||
| = "#{completed}/#{total} セクション完了" |
There was a problem hiding this comment.
一覧描画で section 数ぶん exists? を発行しています。
Line 34-35 は textbook ごとに chapters/sections を辿ったうえで、各 section に対して reading_progresses.exists? を投げています。Linux seed だけでも約 60 section あるので、ログイン時の一覧がすぐ N+1 になります。完了数は controller/decorator 側でまとめて集計済みの値を渡したいです。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/views/textbooks/index.html.slim` around lines 33 - 39, The view is
triggering an N+1 by iterating textbook.chapters.flat_map(&:sections) and
calling reading_progresses.exists?(user: current_user, completed: true) per
section (inside the current_user.present? block calculating total and
completed); move this aggregation into the controller or decorator (e.g.,
compute total_sections and completed_sections_count for each textbook once using
a single query that joins sections -> reading_progresses scoped to current_user
and completed) and pass those precomputed values to the view, then replace the
textbook-local per-section checks with the provided completed_sections_count and
total_sections values (use the same variable names total and completed in the
view to minimize changes).
Member
Author
There was a problem hiding this comment.
@coderabbitai ご指摘はもっともです。ただし現時点ではまず動くものを出すことを優先し、N+1最適化はPhase 2でプロファイリング結果を見ながら対応予定です。
There was a problem hiding this comment.
Rate Limit Exceeded
@komagata have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 27 seconds before sending another message.
Member
Author
CodeRabbit レビュー対応19件中18件を修正して push しました（db8b661）。 対応した項目
今回見送った項目（Phase 2で対応予定）
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/javascript/controllers/term_tooltip_controller.js (2)
88-89:⚠️ Potential issue | 🟡 Minor
{ once: true }により外側クリックで閉じられなくなるケースがありますLine 88 の one-shot リスナーは、ツールチップ内クリックでも消費されるため、その後の外側クリックで閉じなくなります。
onceは外してremoveTooltip()側の明示解除に一本化してください。修正案
- document.addEventListener('click', this.handleOutsideClick, { once: true }) + document.addEventListener('click', this.handleOutsideClick)Also applies to: 91-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/controllers/term_tooltip_controller.js` around lines 88 - 89, The one-shot option on document.addEventListener is causing the outside-click handler to be removed even when clicks occur inside the tooltip; remove the { once: true } option from the document.addEventListener calls that attach handleOutsideClick (and any similar one-shot listeners around lines where the tooltip is shown) and instead ensure removeTooltip() explicitly calls document.removeEventListener('click', this.handleOutsideClick) when the tooltip is dismissed; update showTooltip (or wherever document.addEventListener is called) to attach a persistent listener and rely on removeTooltip() to clean it up so outside clicks always close the tooltip.
43-51:⚠️ Potential issue | 🟠 Major
fetchではなく@rails/request.jsに統一してください（現状はガイドライン違反）Line 43-51 の直接
fetchと Line 58 のresponse.json()は、このリポジトリの JS 方針とズレています。get(..., { responseKind: 'json' })とawait response.json（getter）へ置き換えてください。修正案
import { Controller } from '@hotwired/stimulus' +import { get } from '@rails/request.js' @@ async fetchExplanation(term, cacheKey, anchor) { try { - const csrfToken = document.querySelector('meta[name="csrf-token"]') - const token = csrfToken ? csrfToken.content : '' - - const response = await fetch( + const response = await get( `/api/textbooks/term_explanations/${encodeURIComponent(this.sectionIdValue)}?term=${encodeURIComponent(term)}`, - { - headers: { - Accept: 'application/json', - 'X-CSRF-Token': token - } - } + { responseKind: 'json' } ) @@ - const data = await response.json() + const data = await response.json#!/bin/bash # request.jsルール違反とjson getter呼び出しを確認 rg -n "fetch\(|response\.json\(\)|@rails/request.js|get\(" app/javascript/controllers/term_tooltip_controller.jsAs per coding guidelines「fetchメソッドの代わりにrequest.jsを使う」。
Based on learnings「Rails request.js の FetchResponse では json は getter（response.json）として扱う」。Also applies to: 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/controllers/term_tooltip_controller.js` around lines 43 - 51, Replace the direct fetch call and response.json() usage with `@rails/request.js` APIs: import and use get(...) from '@rails/request.js' to call the endpoint (include the same headers with 'X-CSRF-Token': token and set responseKind: 'json'), and change any response.json() invocation to use the FetchResponse getter (await response.json) instead of calling it as a function; update the call that currently builds the URL with encodeURIComponent(this.sectionIdValue) and term to pass the same URL to get(..., { responseKind: 'json', headers: { Accept: 'application/json', 'X-CSRF-Token': token } }) and replace usages of response.json() with await response.json to retrieve the JSON payload (target the code around the fetch call and the subsequent response.json usage in term_tooltip_controller.js).
🧹 Nitpick comments (2)
app/controllers/mentor/textbooks/chapters_controller.rb (1)
40-42: 任意の改善提案: 一貫性のための eager loading 追加
TextbooksController#set_textbookではincludes(chapters: :sections)を使用していますが、こちらでは単純なfindになっています。現状では問題ありませんが、ビューで章一覧を表示する場合にN+1クエリが発生する可能性があります。現時点ではリダイレクト先で再読み込みされるため実害はありませんが、将来的にedit/newビューで章一覧を表示する場合は考慮が必要です。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/mentor/textbooks/chapters_controller.rb` around lines 40 - 42, The set_textbook method in ChaptersController uses Textbook.find which can cause N+1 queries when the view accesses chapters/sections; change the lookup to eager-load associated chapters and sections (e.g., use Textbook.includes(chapters: :sections).find(params[:textbook_id])) so the `@textbook` loaded in set_textbook matches the eager-loading used in TextbooksController#set_textbook and avoids potential N+1 queries.app/controllers/api/textbooks/reading_progresses_controller.rb (1)
7-19:find_or_initialize_by+retryパターンは適切ですが、リトライ回数の上限追加を検討してください。レースコンディション対応として
RecordNotUniqueを rescue してretryするパターンは一般的なRailsのイディオムであり、通常は2回目で成功します。しかし、予期しないエッジケースでの無限ループを防ぐため、リトライ回数に上限を設けることを推奨します。🛡️ 防御的なリトライ回数上限の追加案
def create + retries ||= 0 `@reading_progress` = current_user.reading_progresses.find_or_initialize_by( textbook_section_id: reading_progress_params[:textbook_section_id] ) `@reading_progress.assign_attributes`(reading_progress_params) if `@reading_progress.save` head(`@reading_progress.previously_new_record`? ? :created : :ok) else head :unprocessable_entity end rescue ActiveRecord::RecordNotUnique + retries += 1 + retry if retries < 3 + head :conflict - retry end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/api/textbooks/reading_progresses_controller.rb` around lines 7 - 19, The create action rescues ActiveRecord::RecordNotUnique and currently calls retry with no limit; add a bounded retry loop: introduce a local attempt counter (e.g., attempts = 0) inside create and increment it each rescue, only retry while attempts < MAX_ATTEMPTS (choose e.g., 3); if max reached re-raise or return head :conflict/:unprocessable_entity. Update the rescue for ActiveRecord::RecordNotUnique to check the counter before retrying and ensure `@reading_progress` assignment/save logic remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/javascript/controllers/term_tooltip_controller.js`:
- Around line 34-36: The tooltip can be reopened by stale async responses; add a
request-id guard so only the latest fetch affects visibility: generate a
monotonically increasing/request token (stored on the controller instance) when
starting fetchExplanation (called from showLoading -> fetchExplanation), attach
it to the request, and before performing the DOM re-show logic currently in the
response handler (the code referenced around the fetchExplanation response
handling that re-displays the tooltip) compare the token from the response with
the controller's current token and abort if they differ; update/clear the token
when the tooltip is explicitly closed to prevent late responses from
re-displaying it.
---
Duplicate comments:
In `@app/javascript/controllers/term_tooltip_controller.js`:
- Around line 88-89: The one-shot option on document.addEventListener is causing
the outside-click handler to be removed even when clicks occur inside the
tooltip; remove the { once: true } option from the document.addEventListener
calls that attach handleOutsideClick (and any similar one-shot listeners around
lines where the tooltip is shown) and instead ensure removeTooltip() explicitly
calls document.removeEventListener('click', this.handleOutsideClick) when the
tooltip is dismissed; update showTooltip (or wherever document.addEventListener
is called) to attach a persistent listener and rely on removeTooltip() to clean
it up so outside clicks always close the tooltip.
- Around line 43-51: Replace the direct fetch call and response.json() usage
with `@rails/request.js` APIs: import and use get(...) from '@rails/request.js' to
call the endpoint (include the same headers with 'X-CSRF-Token': token and set
responseKind: 'json'), and change any response.json() invocation to use the
FetchResponse getter (await response.json) instead of calling it as a function;
update the call that currently builds the URL with
encodeURIComponent(this.sectionIdValue) and term to pass the same URL to
get(..., { responseKind: 'json', headers: { Accept: 'application/json',
'X-CSRF-Token': token } }) and replace usages of response.json() with await
response.json to retrieve the JSON payload (target the code around the fetch
call and the subsequent response.json usage in term_tooltip_controller.js).
---
Nitpick comments:
In `@app/controllers/api/textbooks/reading_progresses_controller.rb`:
- Around line 7-19: The create action rescues ActiveRecord::RecordNotUnique and
currently calls retry with no limit; add a bounded retry loop: introduce a local
attempt counter (e.g., attempts = 0) inside create and increment it each rescue,
only retry while attempts < MAX_ATTEMPTS (choose e.g., 3); if max reached
re-raise or return head :conflict/:unprocessable_entity. Update the rescue for
ActiveRecord::RecordNotUnique to check the counter before retrying and ensure
`@reading_progress` assignment/save logic remains unchanged.
In `@app/controllers/mentor/textbooks/chapters_controller.rb`:
- Around line 40-42: The set_textbook method in ChaptersController uses
Textbook.find which can cause N+1 queries when the view accesses
chapters/sections; change the lookup to eager-load associated chapters and
sections (e.g., use Textbook.includes(chapters:
:sections).find(params[:textbook_id])) so the `@textbook` loaded in set_textbook
matches the eager-loading used in TextbooksController#set_textbook and avoids
potential N+1 queries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2dfa2276-9a74-4b58-886f-90020341507b
📒 Files selected for processing (24)
.cloudbuild/cloudbuild-review.yamlapp/controllers/api/textbooks/reading_progresses_controller.rbapp/controllers/api/textbooks/term_explanations_controller.rbapp/controllers/concerns/textbook_feature_guard.rbapp/controllers/mentor/textbooks/chapters_controller.rbapp/controllers/mentor/textbooks/sections_controller.rbapp/controllers/mentor/textbooks_controller.rbapp/controllers/textbooks/chapters_controller.rbapp/helpers/textbook_helper.rbapp/javascript/controllers/piyo_companion_controller.jsapp/javascript/controllers/reading_progress_controller.jsapp/javascript/controllers/term_tooltip_controller.jsapp/models/textbook/chapter.rbapp/models/textbook/section.rbapp/views/mentor/textbooks/_form.html.slimapp/views/mentor/textbooks/chapters/_form.html.slimapp/views/mentor/textbooks/index.html.slimapp/views/mentor/textbooks/sections/_form.html.slimapp/views/textbooks/chapters/show.html.slimconfig/routes/mentor.rbconfig/routes/textbook.rbdb/seeds/textbook_linux.rbtest/models/reading_progress_test.rbtest/models/textbook/section_test.rb
✅ Files skipped from review due to trivial changes (9)
- .cloudbuild/cloudbuild-review.yaml
- config/routes/textbook.rb
- app/views/mentor/textbooks/chapters/_form.html.slim
- test/models/reading_progress_test.rb
- app/views/mentor/textbooks/index.html.slim
- app/views/mentor/textbooks/sections/_form.html.slim
- app/javascript/controllers/piyo_companion_controller.js
- app/models/textbook/section.rb
- app/javascript/controllers/reading_progress_controller.js
🚧 Files skipped from review as they are similar to previous changes (11)
- app/controllers/api/textbooks/term_explanations_controller.rb
- app/views/textbooks/chapters/show.html.slim
- app/controllers/concerns/textbook_feature_guard.rb
- app/views/mentor/textbooks/_form.html.slim
- app/controllers/textbooks/chapters_controller.rb
- app/helpers/textbook_helper.rb
- db/seeds/textbook_linux.rb
- app/models/textbook/chapter.rb
- app/controllers/mentor/textbooks/sections_controller.rb
- config/routes/mentor.rb
- test/models/textbook/section_test.rb
🚀 Review AppURL: https://bootcamp-pr-9835-fvlfu45apq-an.a.run.app
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/javascript/controllers/reading_progress_controller.js (1)
10-11:visibleBlocksは現状未使用なので削除候補です状態として保持していますが読み取りがなく、意図が伝わりにくくなっています。必要になるまで削除してシンプルに保つのがよいです。
修正例
- this.visibleBlocks = new Set() @@ const index = parseInt(entry.target.dataset.blockIndex, 10) if (entry.isIntersecting) { - this.visibleBlocks.add(index) if (index > this.maxBlockSeen) { this.maxBlockSeen = index } - } else { - this.visibleBlocks.delete(index) }Also applies to: 41-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/controllers/reading_progress_controller.js` around lines 10 - 11, The controller defines an unused state visibleBlocks (and similar unused code around lines 41-47) which should be removed to keep the component simple: locate and remove the visibleBlocks declaration and any related unused variables or initialization (e.g., the Set assigned to visibleBlocks) in reading_progress_controller.js, ensure you also remove any dead code that only reads/initializes visibleBlocks, and if maxBlockSeen is likewise never read, remove that variable too; keep or refactor only the variables that are actually referenced by methods like (any methods that update or read block progress) after searching the file for visibleBlocks and maxBlockSeen to avoid breaking real usages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/javascript/controllers/reading_progress_controller.js`:
- Around line 60-90: The method updateProgress currently sets this.completed and
dispatches the 'completed' event before confirming the POST succeeded; change
updateProgress to be async, await the fetch to
'/api/textbooks/reading_progresses', check response.ok (and optionally parse
errors) and only then set this.completed = true and call
this.dispatch('completed', { detail: { sectionId: this.sectionIdValue }}); if
the request fails, do not mutate this.completed and handle/log the error (e.g.,
catch and log or surface an error state). Ensure you still send the same payload
(reading_progress with textbook_section_id, last_block_index, read_ratio,
completed) but pass completed: isCompleted || this.completed only after success
verification.
---
Nitpick comments:
In `@app/javascript/controllers/reading_progress_controller.js`:
- Around line 10-11: The controller defines an unused state visibleBlocks (and
similar unused code around lines 41-47) which should be removed to keep the
component simple: locate and remove the visibleBlocks declaration and any
related unused variables or initialization (e.g., the Set assigned to
visibleBlocks) in reading_progress_controller.js, ensure you also remove any
dead code that only reads/initializes visibleBlocks, and if maxBlockSeen is
likewise never read, remove that variable too; keep or refactor only the
variables that are actually referenced by methods like (any methods that update
or read block progress) after searching the file for visibleBlocks and
maxBlockSeen to avoid breaking real usages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6168f4ce-82c0-4ecf-bcdc-36990864bbf6
📒 Files selected for processing (6)
app/javascript/controllers/piyo_companion_controller.jsapp/javascript/controllers/reading_progress_controller.jsapp/javascript/controllers/term_tooltip_controller.jstest/fixtures/reading_progresses.ymltest/fixtures/term_explanations.ymltest/fixtures/textbook_sections.yml
✅ Files skipped from review due to trivial changes (2)
- test/fixtures/term_explanations.yml
- test/fixtures/reading_progresses.yml
🚧 Files skipped from review as they are similar to previous changes (3)
- test/fixtures/textbook_sections.yml
- app/javascript/controllers/piyo_companion_controller.js
- app/javascript/controllers/term_tooltip_controller.js
komagata
force-pushed
the
feature/textbook
branch
from
79b421f to
66e2535
Compare
- textbooks - textbook_chapters - textbook_sections - reading_progresses - term_explanations - Enable pg_bigm extension (with rescue for environments without it)
- Textbook: has_many chapters, optional practice association - Textbook::Chapter: acts_as_list within textbook - Textbook::Section: Markdown body, goals/key_terms arrays - ReadingProgress: tracks user reading position per section - TermExplanation: caches AI-generated term definitions per section
- TextbooksController: index, show (table of contents) - Textbooks::ChaptersController / SectionsController: reading views - API::Textbooks::ReadingProgressesController: PATCH reading position - API::Textbooks::TermExplanationsController: GET/create term tooltips - TextbookFeatureGuard concern: Switchlet feature flag gate
- Textbook list, table of contents, chapter, section reading views - TextbookHelper: Markdown rendering with block indexing and term highlighting - Piyo companion partial: mascot UI (bottom-right)
- reading_progress_controller: Intersection Observer block-level tracking - piyo_companion_controller: mascot reactions on section completion - term_tooltip_controller: click-to-explain term tooltips via API
- CRUD for textbooks, chapters, and sections - Nested under /mentor namespace
20 chapters covering Linux basics for bootcamp curriculum
- Use find_by instead of find to avoid RecordNotFound - Check @Pjord mention exists before responding - Extract reply_as_comment to reduce method length
ruby/rubocop-check orb truncates stdout to 34 bytes on failure, making it impossible to identify offenses from CI logs.
RuboCop autocorrect corrupts Unicode escape sequences in seed files.
Enables textbook feature flag on review app deployments.
komagata
force-pushed
the
feature/textbook
branch
from
66e2535 to
653bb8c
Compare
Member
Author
|
@CodeRabbit レビューして。 |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/views/textbooks/sections/show.html.slim (1)
20-23: ビュー内の計算ロジックはDecorator/ヘルパー側へ寄せると保守しやすいです。Line 20-23 と Line 54-58 の算出処理は、今後の仕様追加で肥大化しやすいため、
progressとprev/nextの計算をDecorator（または専用ヘルパー）へ移譲するのを推奨します。As per coding guidelines "ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。"
Also applies to: 54-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/textbooks/sections/show.html.slim` around lines 20 - 23, The view currently calculates progress and prev/next using `@chapter.sections` and `@section` (the blocks computing progress_percent at lines 20-23 and the prev/next logic at lines 54-58); move these responsibilities into a decorator or helper (e.g., SectionDecorator or Textbook::SectionDecorator) by adding methods like progress_percent(section_list) or progress_for(section) and prev_section/next_section (or adjacent_sections) that accept the chapter or section and return the computed percent and neighboring sections, then update show.html.slim to call decorator.progress_for(`@section`) and decorator.prev_section(`@section`)/decorator.next_section(`@section`) (or a single decorator.adjacent_sections(`@section`)) instead of doing the math in the view.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/jobs/pjord_respond_job.rb`:
- Around line 10-12: The current check uses raw string matching on
mentionable.body which can falsely detect removed or altered mentions; change
the guard to re-check against the post's parsed mentions list instead (use the
mentionable.mentions collection or the same mention-parsing helper used at save
time) so you verify presence by login name (e.g. compare pjord.login_name
against mention objects' login_name) and preserve the early-return behavior when
pjord or mentions are nil/empty; update the logic around Pjord.user and the
mention check to use mentionable.mentions (or the shared parser) rather than
mentionable.body.include?.
- Around line 26-35: The case currently drops mentions when mentionable is
PairWork or MicroReport; add branches to handle those types by invoking
reply_as_comment with the original mentionable so a reply record is created.
Update the case in pjord_respond_job (the when Comment / Answer / Question /
Report / Product block) to include "when PairWork, MicroReport then
reply_as_comment(pjord, mentionable, body)" (or equivalent branches), keeping
the existing behavior for `@pjord` parsing and ensuring reply_as_comment is used
for PairWork and MicroReport just like Report/Product.
In `@app/models/concerns/mentioner.rb`:
- Around line 4-8: after_save_mention currently returns early for Report which
skips the PJORD reply path; change the early-return so Report still triggers the
PJORD enqueue when `@pjord` is newly mentioned: instead of unconditional return in
after_save_mention, check new_mentions for the PJORD mention and if present call
the same routine that enqueues PjordRespondJob (or directly enqueue
PjordRespondJob) while still skipping notify_users_found_by_mentions; refer to
after_save_mention, notify_users_found_by_mentions, notify_mentions (or the code
that enqueues PjordRespondJob) and ensure PjordRespondJob is invoked when
new_mentions contains PJORD even for Report.
In `@app/views/mentor/textbooks/edit.html.slim`:
- Around line 60-64: The two icon-only action links using
edit_mentor_textbook_chapter_section_path and
mentor_textbook_chapter_section_path need accessible names: add an aria-label
attribute to each link_to (the edit link and the delete link) with a descriptive
Japanese label (e.g. "章を編集" / "章を削除" or include chapter/section title
dynamically) so screen readers can announce their purpose, keeping the existing
classes and data-confirm/method attributes intact.
---
Nitpick comments:
In `@app/views/textbooks/sections/show.html.slim`:
- Around line 20-23: The view currently calculates progress and prev/next using
`@chapter.sections` and `@section` (the blocks computing progress_percent at lines
20-23 and the prev/next logic at lines 54-58); move these responsibilities into
a decorator or helper (e.g., SectionDecorator or Textbook::SectionDecorator) by
adding methods like progress_percent(section_list) or progress_for(section) and
prev_section/next_section (or adjacent_sections) that accept the chapter or
section and return the computed percent and neighboring sections, then update
show.html.slim to call decorator.progress_for(`@section`) and
decorator.prev_section(`@section`)/decorator.next_section(`@section`) (or a single
decorator.adjacent_sections(`@section`)) instead of doing the math in the view.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c1390207-bebd-4207-bee8-ee881ef6ad1f
📒 Files selected for processing (61)
.circleci/config.yml.cloudbuild/cloudbuild-review.yaml.rubocop.ymlapp/controllers/api/textbooks/reading_progresses_controller.rbapp/controllers/api/textbooks/term_explanations_controller.rbapp/controllers/concerns/textbook_feature_guard.rbapp/controllers/mentor/textbooks/chapters_controller.rbapp/controllers/mentor/textbooks/sections_controller.rbapp/controllers/mentor/textbooks_controller.rbapp/controllers/textbooks/chapters_controller.rbapp/controllers/textbooks/sections_controller.rbapp/controllers/textbooks_controller.rbapp/helpers/textbook_helper.rbapp/javascript/controllers/piyo_companion_controller.jsapp/javascript/controllers/reading_progress_controller.jsapp/javascript/controllers/term_tooltip_controller.jsapp/jobs/pjord_respond_job.rbapp/models/concerns/mentioner.rbapp/models/pjord.rbapp/models/reading_progress.rbapp/models/term_explanation.rbapp/models/textbook.rbapp/models/textbook/chapter.rbapp/models/textbook/section.rbapp/models/user.rbapp/views/mentor/textbooks/_form.html.slimapp/views/mentor/textbooks/chapters/_form.html.slimapp/views/mentor/textbooks/chapters/edit.html.slimapp/views/mentor/textbooks/chapters/new.html.slimapp/views/mentor/textbooks/edit.html.slimapp/views/mentor/textbooks/index.html.slimapp/views/mentor/textbooks/new.html.slimapp/views/mentor/textbooks/sections/_form.html.slimapp/views/mentor/textbooks/sections/edit.html.slimapp/views/mentor/textbooks/sections/new.html.slimapp/views/shared/_piyo_companion.html.slimapp/views/textbooks/chapters/show.html.slimapp/views/textbooks/index.html.slimapp/views/textbooks/sections/show.html.slimapp/views/textbooks/show.html.slimconfig/initializers/ruby_llm.rbconfig/routes.rbconfig/routes/mentor.rbconfig/routes/textbook.rbdb/migrate/20260322000000_enable_pg_bigm_extension.rbdb/migrate/20260326000000_create_textbook_tables.rbdb/migrate/20260328000000_add_unique_index_to_textbook_chapters_on_textbook_id_and_position.rbdb/schema.rbdb/seeds.rbdb/seeds/textbook_linux.rbtest/fixtures/reading_progresses.ymltest/fixtures/term_explanations.ymltest/fixtures/textbook_chapters.ymltest/fixtures/textbook_sections.ymltest/fixtures/textbooks.ymltest/models/reading_progress_test.rbtest/models/term_explanation_test.rbtest/models/textbook/chapter_test.rbtest/models/textbook/section_test.rbtest/models/textbook_test.rbtest/test_helper.rb
✅ Files skipped from review due to trivial changes (29)
- .rubocop.yml
- config/initializers/ruby_llm.rb
- app/models/pjord.rb
- app/controllers/api/textbooks/term_explanations_controller.rb
- db/seeds.rb
- app/views/mentor/textbooks/chapters/edit.html.slim
- app/views/mentor/textbooks/sections/edit.html.slim
- app/models/textbook/section.rb
- app/views/mentor/textbooks/sections/new.html.slim
- app/views/textbooks/chapters/show.html.slim
- app/views/mentor/textbooks/new.html.slim
- app/controllers/textbooks_controller.rb
- app/views/mentor/textbooks/chapters/new.html.slim
- test/fixtures/textbook_sections.yml
- app/views/mentor/textbooks/chapters/_form.html.slim
- app/views/shared/_piyo_companion.html.slim
- test/fixtures/textbook_chapters.yml
- config/routes/textbook.rb
- app/models/textbook.rb
- app/controllers/textbooks/chapters_controller.rb
- app/controllers/concerns/textbook_feature_guard.rb
- test/fixtures/term_explanations.yml
- app/views/mentor/textbooks/index.html.slim
- app/views/textbooks/index.html.slim
- app/views/mentor/textbooks/sections/_form.html.slim
- app/models/reading_progress.rb
- test/models/textbook_test.rb
- test/fixtures/reading_progresses.yml
- db/seeds/textbook_linux.rb
🚧 Files skipped from review as they are similar to previous changes (17)
- .cloudbuild/cloudbuild-review.yaml
- config/routes.rb
- app/models/user.rb
- test/fixtures/textbooks.yml
- app/controllers/textbooks/sections_controller.rb
- app/models/term_explanation.rb
- app/views/mentor/textbooks/_form.html.slim
- app/controllers/api/textbooks/reading_progresses_controller.rb
- app/views/textbooks/show.html.slim
- test/models/textbook/chapter_test.rb
- app/javascript/controllers/piyo_companion_controller.js
- test/models/term_explanation_test.rb
- app/helpers/textbook_helper.rb
- test/models/textbook/section_test.rb
- db/migrate/20260326000000_create_textbook_tables.rb
- app/javascript/controllers/term_tooltip_controller.js
- config/routes/mentor.rb
Comment on lines
10
to
+12
| pjord = Pjord.user | ||
| return if pjord.nil? | ||
| return unless mentions_pjord?(mentionable) | ||
| return unless mentionable.body&.include?("@#{pjord.login_name}") |
There was a problem hiding this comment.
include? だと「メンション削除済み」を正しく判定できません。
保存時のメンション判定はコードブロックや引用を除外していますが、ここだけ生文字列の部分一致にすると、編集後に @pjord をコード例へ移したケースや @pjord_admin のような別トークンへ変えたケースでも応答してしまいます。ジョブ実行時の再確認も、mentions ベースなど保存時と同じルールに揃えたいです。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/jobs/pjord_respond_job.rb` around lines 10 - 12, The current check uses
raw string matching on mentionable.body which can falsely detect removed or
altered mentions; change the guard to re-check against the post's parsed
mentions list instead (use the mentionable.mentions collection or the same
mention-parsing helper used at save time) so you verify presence by login name
(e.g. compare pjord.login_name against mention objects' login_name) and preserve
the early-return behavior when pjord or mentions are nil/empty; update the logic
around Pjord.user and the mention check to use mentionable.mentions (or the
shared parser) rather than mentionable.body.include?.
Comment on lines
26
to
35
| case mentionable | ||
| when Comment | ||
| reply_as_comment(mentionable.commentable, pjord, body) | ||
| reply_as_comment(pjord, mentionable.commentable, body) | ||
| when Answer | ||
| reply_as_answer(mentionable.question, pjord, body) | ||
| Answer.create!(user: pjord, question: mentionable.question, description: body) | ||
| when Question | ||
| reply_as_answer(mentionable, pjord, body) | ||
| when Report, Product, PairWork, MicroReport | ||
| reply_as_comment(mentionable, pjord, body) | ||
| else | ||
| Rails.logger.warn("[Pjord] Unsupported mentionable type: #{mentionable.class.name}") | ||
| Answer.create!(user: pjord, question: mentionable, description: body) | ||
| when Report, Product | ||
| reply_as_comment(pjord, mentionable, body) | ||
| end |
There was a problem hiding this comment.
PairWork / MicroReport では返信が無言で捨てられます。
Mentioner 側では両方ともメンション対象のままなのでジョブ自体は起動しますが、この case では返信レコードが作られないまま終了します。本文からの @pjord を引き続きサポートするなら、この分岐は残しておきたいです。
💡 修正イメージ
- when Report, Product
+ when Report, Product, PairWork, MicroReport
reply_as_comment(pjord, mentionable, body)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/jobs/pjord_respond_job.rb` around lines 26 - 35, The case currently drops
mentions when mentionable is PairWork or MicroReport; add branches to handle
those types by invoking reply_as_comment with the original mentionable so a
reply record is created. Update the case in pjord_respond_job (the when Comment
/ Answer / Question / Report / Product block) to include "when PairWork,
MicroReport then reply_as_comment(pjord, mentionable, body)" (or equivalent
branches), keeping the existing behavior for `@pjord` parsing and ensuring
reply_as_comment is used for PairWork and MicroReport just like Report/Product.
Comment on lines
4
to
8
| def after_save_mention(new_mentions) | ||
| if instance_of?(Report) | ||
| notify_pjord_if_mentioned(new_mentions) | ||
| return | ||
| end | ||
| return if instance_of?(Report) | ||
|
|
||
| notify_users_found_by_mentions(new_mentions) | ||
| end |
There was a problem hiding this comment.
日報更新時の @pjord がここで落ちます。
Report で早期 return すると notify_mentions まで到達しないので、現状の経路だと PjordRespondJob が enqueue されません。通常のメンション通知を止める意図でも、日報の編集で @pjord を追加したケース向けの PJORD 返信だけはここで維持したいです。
💡 修正イメージ
def after_save_mention(new_mentions)
- return if instance_of?(Report)
+ if instance_of?(Report)
+ if extract_login_names_from_mentions(new_mentions).include?(Pjord::LOGIN_NAME)
+ PjordRespondJob.perform_later(mentionable_type: self.class.name, mentionable_id: id)
+ end
+ return
+ end
notify_users_found_by_mentions(new_mentions)
end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/models/concerns/mentioner.rb` around lines 4 - 8, after_save_mention
currently returns early for Report which skips the PJORD reply path; change the
early-return so Report still triggers the PJORD enqueue when `@pjord` is newly
mentioned: instead of unconditional return in after_save_mention, check
new_mentions for the PJORD mention and if present call the same routine that
enqueues PjordRespondJob (or directly enqueue PjordRespondJob) while still
skipping notify_users_found_by_mentions; refer to after_save_mention,
notify_users_found_by_mentions, notify_mentions (or the code that enqueues
PjordRespondJob) and ensure PjordRespondJob is invoked when new_mentions
contains PJORD even for Report.
Comment on lines
+60
to
+64
| = link_to edit_mentor_textbook_chapter_section_path(@textbook, chapter, section), class: 'a-button is-sm is-secondary is-icon is-block' do | ||
| i.fa-solid.fa-pen | ||
| li | ||
| = link_to mentor_textbook_chapter_section_path(@textbook, chapter, section), data: { confirm: '本当に削除しますか？' }, method: :delete, class: 'a-button is-sm is-danger is-icon is-block' do | ||
| i.fa-solid.fa-trash |
There was a problem hiding this comment.
アイコンのみ操作リンクにアクセシブル名を付けてください。
Line 60 と Line 63 は視覚的には理解できますが、スクリーンリーダーでは用途が判別しづらいです。aria-label を追加してください。
🔧 修正例
- = link_to edit_mentor_textbook_chapter_section_path(`@textbook`, chapter, section), class: 'a-button is-sm is-secondary is-icon is-block' do
+ = link_to edit_mentor_textbook_chapter_section_path(`@textbook`, chapter, section), aria: { label: "「#{section.title}」を編集" }, class: 'a-button is-sm is-secondary is-icon is-block' do
i.fa-solid.fa-pen
li
- = link_to mentor_textbook_chapter_section_path(`@textbook`, chapter, section), data: { confirm: '本当に削除しますか？' }, method: :delete, class: 'a-button is-sm is-danger is-icon is-block' do
+ = link_to mentor_textbook_chapter_section_path(`@textbook`, chapter, section), aria: { label: "「#{section.title}」を削除" }, data: { confirm: '本当に削除しますか？' }, method: :delete, class: 'a-button is-sm is-danger is-icon is-block' do
i.fa-solid.fa-trash🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/views/mentor/textbooks/edit.html.slim` around lines 60 - 64, The two
icon-only action links using edit_mentor_textbook_chapter_section_path and
mentor_textbook_chapter_section_path need accessible names: add an aria-label
attribute to each link_to (the edit link and the delete link) with a descriptive
Japanese label (e.g. "章を編集" / "章を削除" or include chapter/section title
dynamically) so screen readers can announce their purpose, keeping the existing
classes and data-confirm/method attributes intact.
## Changes ### Textbook List (index) - Book icon + structured layout with icon, title, meta, progress - Chapter count and section count display - Progress bar using existing completed-practices-progress component - Hover effect on card links ### Table of Contents (show) - Summary card with stats (chapters, sections, completed count) - Progress bar with percentage - 'Continue reading' button with play icon - Chapter cards with per-chapter progress counts - Section links with completion checkmarks and estimated time - Completion celebration message when all done ### Section Reading (sections/show) - Breadcrumb navigation (Textbook > Chapter) - Compact progress bar replacing raw text - Goals section with subtle background styling - Cleaner layout with proper spacing ### CSS Architecture - _textbook-list.css: textbook card list styles - _textbook-toc.css: table of contents styles - _textbook-section.css: reading view navigation and goals - _piyo-companion.css: extracted from inline styles, hover effect, animation - All using CSS custom properties (var(--main), var(--border), etc.)
Replaced custom CSS with existing bootcamp components: - card-list for section listings (same as Docs/practices) - completed-practices-progress for progress bars - a-side-nav for chapter navigation in section view - page-content-prev-next for prev/next navigation - card-header with metas for chapter progress counts - textbook-breadcrumb for section reading context Key changes: - Index: uses card-list pattern with progress percentage - TOC: chapters as card-list with check icons per section - Section: added side-nav with chapter sections (like Docs) - Removed 250+ lines of custom CSS (now using existing components)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
bootcamp に「教科書」機能を追加します（Phase 1a）。
コンセプト: 「だれでも最後まで読みきれる教科書」
Wiki: https://github.com/fjordllc/bootcamp/wiki/textbook機能のコンセプト
設計: https://github.com/fjordllc/bootcamp/wiki/textbook機能の設計
feature flag
Switchlet.enabled?(:textbook)でガードされています。/switchletで:textbookを有効にするまで 404追加内容
DB（5テーブル）
textbooks— 教科書textbook_chapters— 章textbook_sections— 節（Markdown本文、学習目標、重要用語）reading_progresses— 読了追跡（ユーザー×セクション×ブロック位置）term_explanations— 単語説明キャッシュモデル
Textbook/Textbook::Chapter/Textbook::SectionReadingProgress/TermExplanation画面
/textbooks/textbooks/:id— 章・節一覧、進捗表示、「続きを読む」/textbooks/:id/chapters/:id/sections/:id— Markdown全幅表示、前後ナビ/mentor/textbooks— 教科書・章・節のCRUDフロントエンド（Stimulus）
reading_progress_controller— Intersection Observerでブロック単位の読了追跡piyo_companion_controller— ピヨルド🐥常駐、セクション完了で褒める、3日ぶりで「おかえり」term_tooltip_controller— 単語クリック → tooltip表示ヘルパー
textbook_section_body— Markdown描画 + ブロックindex付与 + 重要用語ハイライトseedデータ
スクリーンショット
（feature flag ON後に確認予定）
今後の予定（Phase 1b〜）
Summary by CodeRabbit
New Features
Chores
Tests