Conversation
|
お待たせしました🙇♂️ 
これでお願いしますー |
kitarou888
force-pushed
the
feature/searchable-filtering-login-user
branch
from
359ec49 to
0c5c13b
Compare
|
@machida ■ これまで（検索ワードあり） （検索ワードなし） ■ 改善案（検索ワードあり、自分の投稿のみにチェックあり） （検索ワードあり、自分の投稿のみにチェックなし） （検索ワードなし、自分の投稿のみにチェックあり） （検索ワードなし、自分の投稿のみにチェックなし） |
|
@kitarou888 一旦それでリリースにしようと思います！別PRで人やカテゴリーを線で囲って他の文字と区別するなどの見た目のデザインをしたいと思いますー |
kitarou888
force-pushed
the
feature/searchable-filtering-login-user
branch
from
6bf055f to
68a17e1
Compare
Walkthrough検索機能に「自分の投稿のみ」フィルタを追加し、関連するUI・テスト・スタイルを調整しました。検索フォームとモーダルの見た目・構造が刷新され、検索処理のメソッドシグネチャやテストも新仕様に対応しています。一部不要なスタイルも削除されました。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HeaderView
participant Modal
participant SearchablesController
participant Searcher
User->>HeaderView: 検索ボタンをクリック
HeaderView->>Modal: 検索モーダルを表示
User->>Modal: 検索条件入力・「自分の投稿のみ」チェック
Modal->>SearchablesController: 検索リクエスト送信 (word, only_me, ...)
SearchablesController->>Searcher: search(word:, only_me:, current_user:, ...)
Searcher->>Searcher: only_me条件で検索結果をフィルタ
Searcher-->>SearchablesController: 検索結果返却
SearchablesController-->>Modal: 結果を表示
Modal-->>User: 検索結果閲覧
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/helpers/search_helper_test.rbrubocop-minitest extension supports plugin, specify 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.rbInstructions used from: Sources: test/**/*Instructions used from: Sources: 🧠 Learnings (2)📓 Common learningstest/helpers/search_helper_test.rb (1)⏰ 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)
🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
kitarou888
force-pushed
the
feature/searchable-filtering-login-user
branch
from
68a17e1 to
824e06a
Compare
|
@machida 
理由
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/controllers/api/searchables_controller.rb(1 hunks)app/javascript/stylesheets/application.sass(0 hunks)app/javascript/stylesheets/application/blocks/header/_header-search.sass(0 hunks)app/javascript/stylesheets/atoms/_a-text-input.sass(0 hunks)app/models/searcher.rb(2 hunks)app/views/application/header/_header_links.html.slim(2 hunks)app/views/application/header/_header_search.html.slim(1 hunks)app/views/searchables/index.html.slim(1 hunks)test/models/searcher_test.rb(2 hunks)
💤 Files with no reviewable changes (3)
- app/javascript/stylesheets/application.sass
- app/javascript/stylesheets/atoms/_a-text-input.sass
- app/javascript/stylesheets/application/blocks/header/_header-search.sass
🧰 Additional context used
🧠 Learnings (1)
app/views/searchables/index.html.slim (1)
Learnt from: tyrrell-IH
PR: fjordllc/bootcamp#8807
File: app/views/welcome/job_support.html.slim:391-392
Timestamp: 2025-06-21T22:30:20.116Z
Learning: このプロジェクトにはRailsの組み込みmarkdownヘルパが利用できないため、markdownのHTML変換には自前でヘルパを実装する必要がある。
🧬 Code Graph Analysis (1)
app/controllers/api/searchables_controller.rb (1)
app/models/searcher.rb (1)
search(20-36)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_and_test
🔇 Additional comments (10)
app/models/searcher.rb (1)
20-20: メソッドシグネチャの変更は適切です位置引数からキーワード引数への変更により、メソッドの使いやすさと可読性が向上しています。デフォルト値の設定も適切です。
app/views/searchables/index.html.slim (1)
1-1: タイトルの条件分岐は適切です
params[:only_me]が存在する場合に「自分の投稿のみ」を追加する実装は分かりやすく、ユーザー体験の向上に寄与します。app/controllers/api/searchables_controller.rb (1)
7-7: コントローラーの実装は適切です
Searcher.searchメソッドの新しいシグネチャに対応した実装で、パラメータの受け渡しも正しく行われています。test/models/searcher_test.rb (1)
20-20: 既存テストの更新は適切ですすべてのテストが新しいキーワード引数形式に正しく更新されています。
Also applies to: 25-25, 30-30, 35-35, 40-40, 45-45, 50-50, 55-55, 60-60, 65-65, 70-70, 76-76, 80-80, 84-84, 90-90, 94-94, 98-98, 104-104, 108-108, 112-112, 118-118, 122-122, 126-126, 132-132, 136-136, 140-140, 146-146, 150-150, 154-154, 160-160, 164-164, 168-168, 174-174, 179-179, 191-191, 196-196, 201-201
app/views/application/header/_header_links.html.slim (1)
3-3: UIの変更は適切です検索モーダルの実装とヘッダーリンクの表示調整が適切に行われています。新しい検索機能に対応したUI構造の変更が反映されています。
Also applies to: 8-8, 45-47, 53-53
app/views/application/header/_header_search.html.slim (5)
1-3: フォーム構造の改善が適切に実装されています
form_withヘルパーを使用した適切なフォーム宣言と、新しいCSSクラス構造により、より意味的で整理されたマークアップになっています。.form__itemsと.form__items-innerのネストされたコンテナ構造は、スタイリングとレスポンシブデザインに適しています。
4-8: カテゴリー選択部分のアクセシビリティが向上しています適切なラベル付けとセマンティックな
.form-itemコンテナの使用により、フォームのアクセシビリティが向上しています。Searcher::DOCUMENT_TYPESの使用は適切な関心の分離を示しています。
9-18: メイン機能の「自分の投稿のみ」フィルタチェックボックスが適切に実装されています新しい絞り込み条件セクションが以下の点で適切に実装されています：
only_meパラメータの適切な命名とハンドリングparams[:only_me].present?による状態管理- アクセシビリティを考慮したセマンティックなHTML構造
- 適切な日本語ラベル「自分の投稿のみに絞る」
- 他のフォーム要素と一貫性のあるスタイリング
この実装により、ユーザーは自分の投稿のみを検索結果に表示できるようになります。
19-22: 検索ワード入力部分のUXが向上しています適切なラベル付けと、より具体的なプレースホルダーテキスト「就職が心配な方へ」の使用により、ユーザーエクスペリエンスが向上しています。全体的なフォーム構造との一貫性も保たれています。
23-29: フォームアクション部分が適切に構造化されています
.form-actionsコンテナの使用により、フォームの送信部分が適切に構造化されています。検索アイコンと日本語テキストの組み合わせにより、ユーザーにとって分かりやすいUIになっています。
kitarou888
force-pushed
the
feature/searchable-filtering-login-user
branch
from
8d586cb to
ce2d165
Compare
|
@kitarou888 これでOKですー |
kitarou888
force-pushed
the
feature/searchable-filtering-login-user
branch
2 times, most recently
from
cde7a76 to
109314b
Compare
|
@masyuko0222 お忙しそうでしたら遠慮なくご連絡いただければと思います！ |
|
@kitarou888 |
|
@kitarou888 上記の場合でも効くようにするか、カテゴリーが「すべて」の場合は『自分の投稿のみ絞る』のチェックボックスを押せないようにした方がいいのかなーと思いました。 |
|
@masyuko0222 理由は「プラクティス」と「ユーザー」は自分のもののみでフィルタをかけられないため、一括で効かないように実装しています。
# only_me=true の場合、user_id カラムを持たない :practices および :users は除外してフィルタリング
searchables = searchables.select { |searchable| searchable.user_id == current_user.id } if only_me && %i[all practices users].exclude?(document_type)おっしゃる後者の案で修正しようと思います！ |
|
@masyuko0222 |
|
@kitarou888 「すべて + 自分の投稿のみに絞る」で検索をする人は、自分が投稿した日報や提出物・コメントはまとめて一覧で見たいけど、ユーザーやプラクティスは見えなくても良いんじゃないかなと思いました🤔 自分の投稿のみに絞る、というフィルタなのでユーザーやプラクティスも出てくるとちょっと違和感かもと初めて触る人は思うかもです。 |
|
@masyuko0222 修正してまた連絡しますね！ |
|
@kitarou888 |
kitarou888
force-pushed
the
feature/searchable-filtering-login-user
branch
from
5a7f333 to
a8c22e0
Compare
|
@masyuko0222 というわけで修正しましたので、確認お願いします！ |
masyuko0222
left a comment
masyuko0222
left a comment
There was a problem hiding this comment.
@kitarou888
動作確認OKです〇
気になる点を少しコメントしましたので、ご確認お願いいたしますー。
app/models/searcher.rb
Outdated
| # only_me=true の場合、user_id カラムを持たないプラクティスとユーザーは対象外とする | ||
| if only_me | ||
| searchables = searchables.select do |searchable| | ||
| %w[User Practice].include?(searchable.class.name) ? false : searchable.user_id == current_user.id | ||
| end | ||
| end |
There was a problem hiding this comment.
コメントよりも説明的な変数名を使った方が、コードの意図がより伝わりやすくなるのかなと思いました。
また、searchables から User と Practice クラスを除外し、その後に current_user.id と一致するものを絞り込む処理で書いた方が、流れがシンプルで読みやすくなるかなと思いました。
以下のイメージです。
if only_me
not_posted_classes = %w[User Practice]
searchables = searchables.reject { |s| s.class.name.in?(not_posted_classes) }
.select { |s| s.user_id == current_user.id }
end
test/models/searcher_test.rb
Outdated
|
|
||
| # 他のユーザーの投稿が除外されていることを確認 | ||
| all_results = Searcher.search(word: 'テスト', only_me: false, current_user:, document_type: :reports) | ||
| other_user_results = all_results.select { |searchable| searchable.user_id == other_user.id } |
There was a problem hiding this comment.
assert(result.all? { |searchable| searchable.user_id == current_user.id })で「結果の全てがcurrent_userの投稿である」ことが確認できているので、
あらためて「他ユーザーの投稿が含まれていない」ことを別でチェックする必要はないと思います。
その代わりにUserやPracticeクラスのオブジェクトが検索結果に混ざっていないことは別途確認してもいいのではないでしょうか？
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/models/searcher_test.rb (1)
208-208: 未使用変数を削除してください。
other_user変数が宣言されていますが、テスト内で使用されていません。test 'search only myself' do current_user = users(:komagata) - other_user = users(:kimura)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/models/searcher.rb(2 hunks)test/models/searcher_test.rb(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/models/searcher.rb
🧰 Additional context used
🧠 Learnings (1)
test/models/searcher_test.rb (7)
Learnt from: masyuko0222
PR: fjordllc/bootcamp#8277
File: test/models/searcher_test.rb:0-0
Timestamp: 2025-07-07T12:46:01.650Z
Learning: テストにおいて、`result.all? { |condition| }` でコレクションの全要素が条件を満たすことを確認できれば、その条件を満たさない要素が含まれていないことも論理的に保証されるため、追加のアサーションは冗長となる。
Learnt from: mousu-a
PR: fjordllc/bootcamp#8566
File: app/views/pair_works/_body.html.slim:3-4
Timestamp: 2025-07-01T04:26:24.636Z
Learning: PairWorkモデルにおいて、pair_work.solved?がtrueを返す場合、buddy_idは必ず存在することがビジネスロジックで保証されている。そのため、solved?の条件分岐内でのUser.find(pair_work.buddy_id)は安全に使用できる。
Learnt from: mousu-a
PR: fjordllc/bootcamp#8566
File: app/notifiers/activity_notifier.rb:357-373
Timestamp: 2025-07-01T04:30:19.565Z
Learning: matching_pair_workメソッドは、ペア確定時に送信される通知のため、pair_work.buddy_idは必ず存在することがビジネスロジックで保証されている。そのため、User.find(pair_work.buddy_id)でのエラーハンドリングは不要。
Learnt from: e-yanagita-gs
PR: fjordllc/bootcamp#8771
File: app/controllers/welcome_controller.rb:57-57
Timestamp: 2025-06-30T01:07:46.192Z
Learning: プロジェクトでは、仕様上データの存在が保証されている場合、nil処理を省略することがある。特にFAQ表示のような特定機能に特化したPRでは、データの存在を前提とした実装が採用される。
Learnt from: hirokiej
PR: fjordllc/bootcamp#8740
File: app/helpers/reports_helper.rb:56-64
Timestamp: 2025-06-29T03:44:15.179Z
Learning: このプロジェクト（fjordllc/bootcamp）では、ja.ymlファイルで一部の単語や文章のみI18n対応されているが、多くのテキストは日本語でハードコーディングされており、完全な国際化対応は行われていない。新しい機能でもI18n対応は不要と判断される。
Learnt from: tyrrell-IH
PR: fjordllc/bootcamp#8807
File: app/views/welcome/job_support.html.slim:391-392
Timestamp: 2025-06-21T22:30:20.116Z
Learning: このプロジェクトにはRailsの組み込みmarkdownヘルパが利用できないため、markdownのHTML変換には自前でヘルパを実装する必要がある。
Learnt from: su-su-su-su
PR: fjordllc/bootcamp#0
File: :0-0
Timestamp: 2025-07-07T05:28:03.676Z
Learning: fjordllc/bootcampプロジェクトでは、fixtureファイル内のERB構文で改行制御文字(-)は使用せず、標準的な<% %>と<%= %>のみを使用する方針が統一されている。<% -%>や<%- %>を使用するとSyntaxErrorが発生する。
⏰ 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/models/searcher_test.rb (1)
20-201: メソッドシグネチャの更新が適切に実装されています。全てのテストケースで
Searcher.searchの呼び出しが位置引数からキーワード引数（word:）に正しく更新されています。変更が一貫して適用されており、問題ありません。
kitarou888
force-pushed
the
feature/searchable-filtering-login-user
branch
from
c893f92 to
8650734
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/controllers/searchables_controller.rb (1)
10-10: キーワード引数の使用により可読性が向上しましたSearcherクラスのメソッドシグネチャに合わせたキーワード引数の使用は明確で良い変更です。only_meパラメータの追加も適切に実装されています。
ただし、より厳密にするなら、チェックボックスのパラメータを明示的にbooleanに変換することを検討してください。
- searchables = Searcher.search(word: @word, only_me: params[:only_me], current_user:, document_type: @document_type) + searchables = Searcher.search(word: @word, only_me: !!params[:only_me], current_user:, document_type: @document_type)または、より明示的に：
- searchables = Searcher.search(word: @word, only_me: params[:only_me], current_user:, document_type: @document_type) + searchables = Searcher.search(word: @word, only_me: params[:only_me] == '1', current_user:, document_type: @document_type)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/controllers/searchables_controller.rb(1 hunks)app/javascript/stylesheets/application.sass(0 hunks)app/javascript/stylesheets/application/blocks/header/_header-search.sass(0 hunks)app/javascript/stylesheets/atoms/_a-text-input.sass(0 hunks)app/models/searcher.rb(1 hunks)app/views/application/header/_header_links.html.slim(2 hunks)app/views/application/header/_header_search.html.slim(1 hunks)app/views/searchables/index.html.slim(1 hunks)test/models/searcher_test.rb(12 hunks)test/models/user_test.rb(1 hunks)test/system/admin/users/password_test.rb(1 hunks)test/system/current_user/password_test.rb(1 hunks)test/system/searchables_test.rb(6 hunks)
💤 Files with no reviewable changes (3)
- app/javascript/stylesheets/application.sass
- app/javascript/stylesheets/atoms/_a-text-input.sass
- app/javascript/stylesheets/application/blocks/header/_header-search.sass
🚧 Files skipped from review as they are similar to previous changes (9)
- test/system/admin/users/password_test.rb
- test/system/current_user/password_test.rb
- test/system/searchables_test.rb
- test/models/user_test.rb
- app/views/searchables/index.html.slim
- app/views/application/header/_header_links.html.slim
- app/models/searcher.rb
- test/models/searcher_test.rb
- app/views/application/header/_header_search.html.slim
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.rb`: # refactoring - まずFat Controllerを避け、次にFat Modelを避ける。 - Serviceクラスの乱用を...
**/*.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の機能があるので使う）
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
app/controllers/searchables_controller.rb
🧬 Code Graph Analysis (1)
app/controllers/searchables_controller.rb (1)
app/models/searcher.rb (1)
search(16-32)
⏰ 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
kitarou888
force-pushed
the
feature/searchable-filtering-login-user
branch
from
77fca61 to
04d2fcd
Compare
kitarou888
force-pushed
the
feature/searchable-filtering-login-user
branch
from
04d2fcd to
31f2eaf
Compare
|
@masyuko0222 |
|
@masyuko0222 @komagata |
4 participants
Issue
概要
ヘッダーにある検索フォームに「自分のみ」のチェックボックスを追加し、自分が書いた日報その他のみをフィルタリングできるようにした。
変更確認方法
feature/searchable-filtering-login-userをローカルに取り込むgit fetch origin pull/8277/head:pr-8277-reviewgit switch pr-8277-reviewbin/setupforeman start -f Procfile.devmachida、パスワードtesttest）Screenshot
変更前
変更後
Summary by CodeRabbit
新機能
改善
バグ修正
テスト