Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

複数キーワードで検索したい #1510

Closed
sanfrecce-osaka opened this issue Apr 4, 2020 · 2 comments
Closed

複数キーワードで検索したい #1510

sanfrecce-osaka opened this issue Apr 4, 2020 · 2 comments
Projects

Comments

@sanfrecce-osaka
Copy link
Contributor

概要

現行の検索機能の仕様では、複数キーワードでの絞り込みに対応しておらず、キーワード全体と一致するものを検索結果として返している
そのため、単一のキーワードで検索した後に別のキーワードを追加して検索結果を絞り込むという使い方ができない
そのため Google の検索のように、複数のキーワードをスペースで区切って検索できるようにしたい

現行の仕様

スペースを含めたキーワード全体と完全一致しているものを検索結果として返す

期待される振る舞い

スペースで区切られたキーワードは別のキーワードみなし、全てのキーワードを含むものを検索結果として返す(== AND検索)

e.g.

検索対象

  1. "メンター 就職の話をします。"
  2. "メンターと就職の話をします。"

検索キーワード

"メンター 就職"

検索結果

現行の仕様

  • 検索対象1 は検索結果に 含まれる
  • 検索対象2 は検索結果に 含まない

期待される振る舞い

  • 検索対象1 は検索結果に 含まれる
  • 検索対象2 は検索結果に 含まれる
@sanfrecce-osaka sanfrecce-osaka added this to やる in 開発 via automation Apr 4, 2020
@komagata
Copy link
Member

komagata commented Apr 5, 2020

@sanfrecce-osaka これはPRをいただけるという前振りですね?

@sanfrecce-osaka
Copy link
Contributor Author

@komagata

はい、後日PR出す予定です〜:pray:(現役生の課題にもいいかなと思ったのですがやり始めてしまったので自分で出しちゃいます:bow:)

関連: #1487

sanfrecce-osaka added a commit that referenced this issue Apr 11, 2020
refs #1510

### 概要
スペースで区切られたキーワードは別のキーワードみなし、全てのキーワードを含むものを検索結果として返す(== AND検索)ように Searcher.search_for を修正
半角スペースも全角スペースも区切り文字として許可するため、キーワードは [:blank] で split している
sanfrecce-osaka added a commit that referenced this issue Apr 11, 2020
refs #1510

### 概要
検索可能なリソースごとに半角・全角スペース区切りで複数キーワード検索をした際に検索結果が絞り込めているかどうかを観点としてテストを追加

### 修正内容
#### 複数キーワードでの検索のテストを追加
全ての検索可能なリソースに対して3つのケースでの検証を行っている

- 単一キーワードで検索した場合
  - 絞り込み前の比較対象
- 半角スペース区切りの複数キーワードで検索した場合
  - AND検索 になっているかどうか(想定通り絞り込まれているかどうか)
- 全角スペース区切りの複数キーワードで検索した場合
  - AND検索 になっているかどうか(想定通り絞り込まれているかどうか)

### その他の修正
- test/fixtures/pages.yml で typo があり、 expected を書く際にややこしかったため正しいスペルに修正
  - Bootcmap => Bootcamp
sanfrecce-osaka added a commit that referenced this issue Apr 11, 2020
refs #1510

### 概要
Searcher.result_for が DRY でなかったため、インターフェースの統一やパラメータ生成処理の各検索対象の model への移動等のリファクタリングを行った

### 修正内容
#### 検索の際に呼び出し側から呼び出すメソッドを search_by_keywords に統一
Searcher から呼び出す際のインターフェースを統一するため、検索の際に呼び出すメソッドを search_by_keywords に統一した

##### Searchable に実装したことについて
search_by_keywords は既存で Searchable が既にあり責務的にも適切であると判断したため、 Searchable に持たせた
一方、Searchable に description というメソッドが既にあること、 Searchable が include されていない検索対象の model があったということから、当初 Searchable に持たせるかどうか迷ったが、
include されていなかった model も含めて Page 以外の全ての model で description を持っていたため、問題ないと判断した

#### パラメータの生成処理を各検索対象の model に移動
キーワード検索の際のパラメータ生成への関心は、 Searcher ではなく それぞれの model が知っているべきと考えたため、 パラメータ生成処理は各検索対象の model に移動した
何のためのメソッドであるかを明示するため、 Module#concerning を使用している

#### result_for を private メソッドに修正
Searcher.result_for は外部から呼ばれることは想定されていないはずなので、 Searcher.result_for を private メソッド に修正した
sanfrecce-osaka added a commit that referenced this issue Apr 11, 2020
refs #1510

### 概要
Searcher.rb で検索対象の model を表す symbol から model の名前や model のオブジェクトを動的に生成する処理が DRY でなかったかつパッと見て何をしているか分かりづらかったため、対象の処理をメソッドに切り出した
sanfrecce-osaka added a commit that referenced this issue Apr 11, 2020
refs #1510

### 概要
document_type による 処理の振り分け で if文 にベタ書きされていてリーダブルではなかったため、条件判定の処理に名前付けする目的でメソッドに切り出し
また、全ての条件式に document_type を引数として渡すのは DRY ではないため、条件判定のメソッドは lambda を返すようにして if を case に置き換えた
sanfrecce-osaka added a commit that referenced this issue Apr 27, 2020
refs #1510
refs #1521 (comment)

### 概要
検索可能なタイプかどうかの判定処理を可読性のためにメソッドに切り出していたが、 Symbol#in で済むためそちらを使うようにした
sanfrecce-osaka added a commit that referenced this issue Apr 27, 2020
refs #1510
refs #1521 (comment)

### 概要
214e427 で case文 に変更していて単純比較で済むものはメソッドに切り出す必要がなくなっていた
そのため、 all や questions を判定する when節 は単純にシンボルを書くように修正した
sanfrecce-osaka added a commit that referenced this issue Apr 27, 2020
refs #1510
refs #1521 (comment)

### 概要
下記コミットで修正漏れがあったため、該当箇所を修正した

title: model の名前の動的生成及び model への変換処理をリファクタリング
revision: 931559b
sanfrecce-osaka added a commit that referenced this issue Apr 27, 2020
refs #1510
refs #1521 (comment)

### 概要
各モデル毎に params_for_keyword_search 及び groupings を定義していたが、重複部分が多かったため共通部分は Searchable に定義して差分がある場合に各クラスでオーバーライドするよう修正した
また、 キーワードにより検索するカラムは DSL で定義できるようにした

### 修正内容
#### 共通部分を Searchable に定義して差分がある場合に各クラスでオーバーライドするよう修正
params_for_keyword_search 、groupings を各モデルで定義していたが、重複部分がかなり多かったため重複部分は Searchable に移動した
Comment のみ commentable_type が検索条件として必要になるため、オーバーライドして差分プログラミングで対応した

#### キーワード検索で使用する条件をDSLで設定できるように修正
キーワード検索の際に キーワードで検索されるカラムを DSL で設定できるよう Serachable.target_column_of_keyword を追加した
Serachable.target_column_of_keyword で定義されるメソッドを private化 する方法は Object#__send__ で Module#private を実行する方法と tap の際に Module$private を実行する方法を検討したが、 複数のプログラマーの意見、可読性を考慮して後者を選択した

refs #1521 (comment)

#### concerning を使用しないよう修正
bootcamp では他では concerning が使用されておらず統一感・可読性の観点から使用しないことに決定した

refs #1521 (comment)
refs #1521 (comment)
refs #1521 (comment)
refs #1521 (comment)
refs #1521 (comment)
refs #1521 (comment)
sanfrecce-osaka added a commit that referenced this issue Apr 27, 2020
refs #1510
refs #1521 (comment)

### 概要
commentable_type_eq での絞込が機能しているかどうかを検証するテストを追加した

### 修正内容
#### commentable_type_eq が機能しているかどうかを検証するテストを追加
コメントを検索する際は条件として commentable_type_eq を設定するが、 既存ではそれが機能しているかを検証するテストがなかった
そのため、下記の2件のテストを追加し commentable_type_eq が機能しているかを検証した

- :all 以外の document_type を指定して検索した結果が、指定した document_type のコメントのみであるかの検証
- document_type に何も指定せずに検索した結果に、全ての document_type が含まれているかの検証

refs #1521 (comment)
sanfrecce-osaka added a commit that referenced this issue Apr 27, 2020
refs #1510
refs #1521 (comment)
refs #1521 (comment)

### 概要
既存では想定外の type が Searcher#result_for に渡ってきた場合は空配列を返していたが、そもそも想定外の type が渡されるのは異常事態であるため、例外を発生させるように修正した

#### 補足
現状、修正行の例外は Unreachable であるが、下記の理由からそのままにしている

- /searchables?document_type=comments で検索した際の挙動をそのまま裏仕様として残しておくため
- 今後の修正で reachable になったときのための防御的プログラミング

想定外の type に対する本質的な対応は #1528 (comment) への対応で行う予定

refs #1528 (comment)
@komagata komagata moved this from やる to 今のイテレーション in 開発 May 13, 2020
sanfrecce-osaka added a commit that referenced this issue May 16, 2020
開発 automation moved this from 今のイテレーション to 完成 Jul 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
開発
  
完成
Archived in project
Development

No branches or pull requests

2 participants