fix: Exclude user page data from search response when user pages are disabled#10740
fix: Exclude user page data from search response when user pages are disabled#10740yuki-takei merged 21 commits intomasterfrom
Conversation
apps/app/src/server/routes/search.ts
Outdated
| if (!query.includes('-prefix:/user')) { | ||
| query = `${query.trim()} -prefix:/user`; | ||
| } | ||
| } |
There was a problem hiding this comment.
おそらく ElasticSearchClient 周りまで見る必要はなく Controller 層の修正のみで済む
ごめん、これは嘘で、normalizeQueryString の中でやった方が良さそうです。
growi/apps/app/src/server/service/search.ts
Lines 51 to 56 in db7a620
| const cleanQuery = query.replace(/prefix:\/user/g, '').trim(); | ||
|
|
||
| if (!cleanQuery.includes('-prefix:/user')) { | ||
| const queryWithUserPrefix = `${cleanQuery.trim()} -prefix:/user`; |
There was a problem hiding this comment.
cleanQuery 変数作成時 trim() 済みなので、ここで trim() をもう一回呼び出すのは冗長だとおもいます
| @@ -0,0 +1,18 @@ | |||
| export function removeUserPages( | |||
There was a problem hiding this comment.
- メソッド名から実装の内容が想像しにくいです
excludeUserPagesFromQueryとかはどうでしょうか？
| return query; | ||
| } | ||
|
|
||
| const cleanQuery = query.replace(/prefix:\/user/g, '').trim(); |
There was a problem hiding this comment.
文字列操作が甘い
prefix:/userouterがouterになってしまうapprefix:/userがapになってしまう
|
|
||
| const cleanQuery = query.replace(/prefix:\/user/g, '').trim(); | ||
|
|
||
| if (!cleanQuery.includes('-prefix:/user')) { |
| const normalizeQueryString = ( | ||
| _queryString: string, | ||
| disableUserPages: boolean, | ||
| ): string => { |
There was a problem hiding this comment.
normalizeQueryString はいじらず、const terms = this.parseQueryString(queryString); の結果である QueryTerms に対して prefix と not_prefix を操作すればいいんじゃないかね？
それから /user だけではなく、/user/ から始まるものも全て消す必要がある
There was a problem hiding this comment.
ちなみに、parseSearchQuery() の返り値である ParsedQuery の queryString には特に操作を加える必要がない可能性が高い。おそらく appendFunctionScore で文字列長を数えるためだけに使われている。(要調査)
| const normalizeQueryString = ( | ||
| _queryString: string, | ||
| disableUserPages: boolean, | ||
| ): string => { |
There was a problem hiding this comment.
ちなみに、parseSearchQuery() の返り値である ParsedQuery の queryString には特に操作を加える必要がない可能性が高い。おそらく appendFunctionScore で文字列長を数えるためだけに使われている。(要調査)
|
|
||
| export function excludeUserPagesFromQuery(terms: QueryTerms): void { | ||
| terms.prefix = terms.prefix.filter((p) => !p.startsWith('/user')); | ||
| terms.not_prefix = terms.not_prefix.filter((p) => !p.startsWith('/user')); |
There was a problem hiding this comment.
prefix:/userouter のような edge case を考えるようにしてください
…177549-block-user-page-searches
3 participants
https://redmine.weseek.co.jp/issues/177549
┗ https://redmine.weseek.co.jp/issues/177572