Conversation
Walkthrough未チェック専用コントローラ・ビュー・ルート・関連システムテストを削除し、 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant ReportsController
participant ReportModel
participant View
User->>Browser: GET /reports or /reports?unchecked=1
Browser->>ReportsController: request (params)
alt params[:unchecked] present and admin_or_mentor_login?
ReportsController->>ReportModel: query Report.unchecked.not_wip.list.page(...).per(...)
else
ReportsController->>ReportModel: query Report.list (optional practice filter) + pagination
end
ReportModel-->>ReportsController: records
ReportsController->>View: render reports/index (single "日報" タブ with badge if allowed)
View-->>Browser: HTML
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 分 Possibly related issues
Possibly related PRs
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/system/reports_practice_filter_test.rbrubocop-minitest extension supports plugin, specify app/controllers/reports_controller.rbrubocop-minitest extension supports plugin, specify config/routes/reports.rbrubocop-minitest extension supports plugin, specify
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
sharoa119
changed the title
|
@machida さん |
|
@sharoa119 デザイン入れましたー |
|
@machida さん |
|
@su-su-su-su さん |
|
@sharoa119 |
|
@sharoa119 要望画像のような |
|
@su-su-su-su さん そして、自分のところで確認したら確かにデザインが入っていませんでした💦 お願いがあるのですが、デザインのことなので最初のコメント時に、一緒に町田さんをメンションしていただけると助かります〜🙏 それではしばしお待ちください。 |
|
@machida さん こちらのissueですが、デザインを入れてくださったとあったのですが レビュアーの @su-su-su-su さんから
とコメントいただき、確認したところ、デザインが入っていませんでした💦 タイトルが左に寄っていたり、「+日報作成」が真ん中にあったりなど、CSSがきいていないようです。 ご確認をお願いいたします🙇♀️ |
|
@sharoa119 あれ？すいません、確認します🙇♂️ |
|
すいません、コミット忘れがあったのか、デザインが抜けていました🙇♂️ 
|
|
@machida さん @su-su-su-su さん |
su-su-su-su
left a comment
su-su-su-su
left a comment
There was a problem hiding this comment.
@sharoa119
お疲れ様です！お待たせしました🙏
問題なさそうでしたのでapproveさせていただきます！
|
@su-su-su-su さん @komagata さん |
|
@sharoa119 conflictの修正をお願いします〜 |
sharoa119
force-pushed
the
feature/change_place_of_report_and_blog_tabs
branch
from
b025803 to
f37b09d
Compare
|
@komagata さん
とあったのですが、これはどのように対応したら良いのでしょうか。 ちょうどディスコードでsekitoさんがおっしゃっている内容が当てはまるのかな、とも思ったりです。 解決方法をご教示いただければと思います🙇♀️ |
sharoa119
force-pushed
the
feature/change_place_of_report_and_blog_tabs
branch
2 times, most recently
from
7e4627a to
92becec
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/views/external_entries/index.html.slim (1)
21-23: 空の .page-header-actions を条件付きで出力するか削除を検討現在中身がなく出力されるだけなので、DOMノイズになります。将来の拡張予定がなければ削除、あるいはコンテンツがある場合のみ描画するのが望ましいです。
提案（用途がなければ削除）:
- .page-main-header__end - .page-header-actions + / .page-main-header__end はこのタブでは未使用のため削除コンポーネント再利用の都合で残す場合は、content_for? 等で条件出力をご検討ください。
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
app/controllers/reports/unchecked_controller.rb(0 hunks)app/controllers/reports_controller.rb(1 hunks)app/helpers/page_tabs/reports_helper.rb(1 hunks)app/views/application/_global_nav.slim(1 hunks)app/views/external_entries/index.html.slim(1 hunks)app/views/reports/index.html.slim(1 hunks)app/views/reports/unchecked/index.html.slim(0 hunks)config/routes/reports.rb(1 hunks)test/system/report/unchecked_test.rb(0 hunks)test/system/reports_test.rb(0 hunks)test/system/searchables_test.rb(1 hunks)
💤 Files with no reviewable changes (4)
- test/system/reports_test.rb
- app/controllers/reports/unchecked_controller.rb
- app/views/reports/unchecked/index.html.slim
- test/system/report/unchecked_test.rb
🚧 Files skipped from review as they are similar to previous changes (4)
- app/controllers/reports_controller.rb
- app/helpers/page_tabs/reports_helper.rb
- config/routes/reports.rb
- app/views/reports/index.html.slim
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rb
⚙️ CodeRabbit Configuration File
**/*.rb: # refactoring
- まずFat Controllerを避け、次にFat Modelを避ける。
- Serviceクラスの乱用を避ける。
- controller concernを作ろうとしたらPORO(Plain Old Ruby Object)やActiveRecordモデルでの実装で代替できないか検討する。
Rails Patterns
- ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）
- 複雑なActiveRecordクエリがあり、再利用できそうな場合はQueryObjectパターンを検討する。（rails-patterns gemを導入しているのでそれのQuery機能を使う）
- Viewにpartialを作る場合はViewComponentを使うことを検討する。
- 複数のActiveRecordモデルを操作する１つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。（interactor gemを導入しているのでそれを使う）
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。（interactor gemを導入しており、その中にOrganizerの機能があるので使う）
Files:
test/system/searchables_test.rb
test/**/*
⚙️ CodeRabbit Configuration File
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/system/searchables_test.rb
🧬 Code Graph Analysis (1)
test/system/searchables_test.rb (1)
test/supports/avatar_helper.rb (1)
reset_avatar(6-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
🔇 Additional comments (2)
test/system/searchables_test.rb (1)
189-191: fixture の明示的な変数化と reload の追加は妥当（テスト安定性の向上）
users(:komagata)をローカル変数に入れてreset_avatar(user)を呼び、直後にuser.reloadしているのは、ActiveStorage の関連キャッシュを確実に最新化する意図として適切です。この変更は安全で、システムテストのフレーク低減に寄与します。app/views/external_entries/index.html.slim (1)
14-35: レイアウトの刷新は要件を満たしており妥当です（+日報作成ボタンの非表示もOK）
- main.page-main 配下にページタイトル「みんなのブログ」を持つ h1 を設け、ヘッダの右側から new_report を外した点は要件と一致しています。
- 未登録フィードの注意喚起も維持されており、視覚的な区切り(hr)も良いです。
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/system/external_entries_test.rb (1)
26-30: 冗長/不正確なアサーションを整理する。
assert_no_link '日報作成'とassert_no_selector 'a.btn', text: '日報作成'は実質的に同じ意図で、後者はこのプロジェクトのBEM風クラス（例:a-button）とも整合しないため将来のメンテに不利です。- ボタンの非表示確認はリンクテキストの不在で十分です（クラス名変更の影響を受けにくい）。
a.btnの行を削除することを推奨します。提案差分:
test 'does not show "New Report" button on External Entries page' do visit_with_auth external_entries_url, 'komagata' assert_no_link '日報作成' - assert_no_selector 'a.btn', text: '日報作成' end
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/views/application/_global_nav.slim(1 hunks)test/system/external_entries_test.rb(1 hunks)test/system/searchables_test.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/views/application/_global_nav.slim
- test/system/searchables_test.rb
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rb
⚙️ CodeRabbit Configuration File
**/*.rb: # refactoring
- まずFat Controllerを避け、次にFat Modelを避ける。
- Serviceクラスの乱用を避ける。
- controller concernを作ろうとしたらPORO(Plain Old Ruby Object)やActiveRecordモデルでの実装で代替できないか検討する。
Rails Patterns
- ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）
- 複雑なActiveRecordクエリがあり、再利用できそうな場合はQueryObjectパターンを検討する。（rails-patterns gemを導入しているのでそれのQuery機能を使う）
- Viewにpartialを作る場合はViewComponentを使うことを検討する。
- 複数のActiveRecordモデルを操作する１つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。（interactor gemを導入しているのでそれを使う）
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。（interactor gemを導入しており、その中にOrganizerの機能があるので使う）
Files:
test/system/external_entries_test.rb
test/**/*
⚙️ CodeRabbit Configuration File
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/system/external_entries_test.rb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
sharoa119
force-pushed
the
feature/change_place_of_report_and_blog_tabs
branch
from
eed2821 to
e290951
Compare
sharoa119
force-pushed
the
feature/change_place_of_report_and_blog_tabs
branch
from
0063c61 to
a3a6886
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/system/external_entries_test.rb (1)
32-36: 重複アサーションの共通化＋厳密一致で誤検知を抑制2つのテストで同一アサーションが繰り返されているため、ヘルパーに切り出すとDRYになります。あわせて
assert_no_linkにexact: trueを付けると部分一致の誤検知を避けられます（任意）。以下をテストクラス内に追加（例: 最後に private メソッドとして）:
private def assert_no_new_report_button assert_no_link '日報作成', exact: true assert_no_selector 'a.btn', text: '日報作成' end該当箇所をこのヘルパー呼び出しに置き換え:
- assert_no_link '日報作成' - assert_no_selector 'a.btn', text: '日報作成' + assert_no_new_report_button同様に先のテスト（Line 28-29）も以下のように置き換え可能です:
- assert_no_link '日報作成' - assert_no_selector 'a.btn', text: '日報作成' + assert_no_new_report_button
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
app/controllers/reports/unchecked_controller.rb(0 hunks)app/controllers/reports_controller.rb(1 hunks)app/helpers/page_tabs/reports_helper.rb(1 hunks)app/views/application/_global_nav.slim(1 hunks)app/views/external_entries/index.html.slim(1 hunks)app/views/reports/index.html.slim(1 hunks)app/views/reports/unchecked/index.html.slim(0 hunks)config/routes/reports.rb(1 hunks)test/system/external_entries_test.rb(1 hunks)test/system/report/unchecked_test.rb(0 hunks)test/system/reports_practice_filter_test.rb(1 hunks)test/system/reports_test.rb(0 hunks)test/system/searchables_test.rb(1 hunks)
💤 Files with no reviewable changes (4)
- test/system/reports_test.rb
- app/controllers/reports/unchecked_controller.rb
- test/system/report/unchecked_test.rb
- app/views/reports/unchecked/index.html.slim
🚧 Files skipped from review as they are similar to previous changes (8)
- app/helpers/page_tabs/reports_helper.rb
- app/views/application/_global_nav.slim
- app/views/external_entries/index.html.slim
- app/controllers/reports_controller.rb
- test/system/reports_practice_filter_test.rb
- app/views/reports/index.html.slim
- test/system/searchables_test.rb
- config/routes/reports.rb
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rb
⚙️ CodeRabbit configuration file
**/*.rb: # refactoring
- まずFat Controllerを避け、次にFat Modelを避ける。
- Serviceクラスの乱用を避ける。
- controller concernを作ろうとしたらPORO(Plain Old Ruby Object)やActiveRecordモデルでの実装で代替できないか検討する。
Rails Patterns
- ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）
- 複雑なActiveRecordクエリがあり、再利用できそうな場合はQueryObjectパターンを検討する。（rails-patterns gemを導入しているのでそれのQuery機能を使う）
- Viewにpartialを作る場合はViewComponentを使うことを検討する。
- 複数のActiveRecordモデルを操作する１つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。（interactor gemを導入しているのでそれを使う）
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。（interactor gemを導入しており、その中にOrganizerの機能があるので使う）
Files:
test/system/external_entries_test.rb
test/**/*
⚙️ CodeRabbit configuration file
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/system/external_entries_test.rb
🧠 Learnings (1)
📓 Common learnings
Learnt from: jun-kondo
PR: fjordllc/bootcamp#8977
File: app/controllers/reports_controller.rb:63-63
Timestamp: 2025-07-23T20:31:13.856Z
Learning: fjordllc/bootcampプロジェクトの`app/controllers/reports_controller.rb`において、`create`と`update`アクションは両方とも`report.save_uniquely`を使用し、同じ`:report_save`イベントと`'report.save'`イベントを発行する。これは両方とも本質的に「レポートの保存」操作であり、作成と更新を区別する必要がないためである。
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
🔇 Additional comments (1)
test/system/external_entries_test.rb (1)
26-30: 「+日報作成」非表示の回帰テスト追加は妥当（LGTM）External Entries 上で「日報作成」を出さない仕様に対する回帰テストとして適切です。テスト名も英語化されており、ガイドライン準拠です。
|
@komagata さん |
| namespace :reports do | ||
| resources :unchecked, only: %i(index) | ||
| end |
There was a problem hiding this comment.
未チェックの日報のページ自体なくなってしまって良かったんでしたっけ？
There was a problem hiding this comment.
コメントありがとうございます。
確認します！！
|
@machida さん
とコメントをいただきました。 私の理解では、未チェックタブ自体はなくなって、日報タブの中に「全て」と「未チェック」のサブタブがあるイメージです。 この方向で大丈夫だったのでしょうか。ご教示いただければと思います🙇♀️ |
|
@sharoa119 はい！それで大丈夫なはずなのですが、念のため再度このブランチを僕も確認したいと思います。 確認するので、少々お待ちください🙏 |
|
なるほど、わかりました。サブタブなので未チェック一覧をパラメータで表示するように変更してくれたのですね！ 元々未チェックの日報一覧のURLが、 このPRで URLが変わっただけで、機能や動作には問題ありませんでした。 |
|
@machida さん
いえいえ、私が確認すればよかったという話です😫 駒形さんからの返信を待ちたいと思います。 |
なるほどです。ご説明ありがとうございます〜 |
4 participants
Issue
概要
現在、このようになっていて、未チェックの日報タブは一部のユーザーにだけ表示されている。
それを、次のように変更。未チェックの日報タブ、未チェックの日報数は一部のユーザーにだけ表示されるようにする。
（のちに、「全て」と「未チェック」の下に、「プラクティスで絞り込む」を配置することを確認しました。）
変更確認方法
feature/change_place_of_report_and_blog_tabsをローカルに取り込むforeman start -f Procfile.devでローカル環境を立ち上げるkomagata（管理者かメンター）のアカウントでログインをする全てと未チェックのタブがあり、未チェックがある場合はその数値が日報タブの右上に表示されていることを確認プラクティスで絞り込むは非表示になっていることを確認（これは町田さんからの要望の画像です。）
+日報作成は無いことを確認komagataをログアウトしてkimura（管理者・メンター以外）でログインする全てと未チェックのタブが非表示になっていることを確認Screenshot
変更前
変更後
2025-07-22.15.11.33.mov
Summary by CodeRabbit
新機能
改善
削除
テスト