Iindexable test2 #19
Iindexable test2 #19
Conversation
Walkthroughこのプルリクエストでは、OpenSearchのインデックス機能を強化するための新しい設定ファイルやマイグレーション、サービスの変更が行われました。ユーザーのインデックス可能性を制御する新しいプロパティが追加され、関連するAPIエンドポイントやフロントエンドコンポーネントも更新されています。これにより、ユーザーのプライバシー設定や検索機能が改善され、より柔軟なデータ管理が可能になります。 Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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
CodeRabbit Configuration File (
|
|
이 PR에 의한 api.json 차이 차이점은 여기에서 볼 수 있음--- base
+++ head
@@ -13122,6 +13122,29 @@
"bearerAuth": []
}
],
+ "requestBody": {
+ "required": true,
+ "content": {
+ "application/json": {
+ "schema": {
+ "type": "object",
+ "properties": {
+ "index": {
+ "type": "string",
+ "enum": [
+ "notes",
+ "reaction",
+ "pollVote"
+ ]
+ }
+ },
+ "required": [
+ "index"
+ ]
+ }
+ }
+ }
+ },
"responses": {
"204": {
"description": "OK (without any results)"
@@ -50014,6 +50037,9 @@
"isExplorable": {
"type": "boolean"
},
+ "isIndexable": {
+ "type": "boolean"
+ },
"hideOnlineStatus": {
"type": "boolean"
}, |
…rrypick into iindexable-test2
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
Outside diff range and nitpick comments (22)
.github/cherrypick/test-opensearch.yml (1)
1-4: URLとポート設定が適切です。コメントの改善を提案します。ローカルテスト用のURL設定と、競合を避けるためのポート設定が適切に行われています。
コメントの改善案：
-# ローカルでテストするときにポートを被らないようにするためデフォルトのものとは変える(以下同じ) +# ローカルテスト時のポート競合を避けるため、デフォルト値から変更しています（以下の設定も同様）この変更により、コメントがより簡潔で明確になります。
packages/backend/test/compose.yml (2)
15-16: ビルド設定にバージョンを指定することを検討してください。再現性を確保するために、OpenSearchのビルドにバージョンを指定することをお勧めします。これにより、異なる環境間での一貫性が向上します。
例えば、以下のように
Dockerfileを使用してバージョンを指定できます：- build: ../../../opensearch + build: + context: ../../../opensearch + dockerfile: Dockerfile + args: + OPENSEARCH_VERSION: 2.x.x # 適切なバージョンに置き換えてくださいこの変更により、特定のバージョンのOpenSearchを使用することが保証されます。
29-30: ポートマッピングの設定が適切です。ドキュメントの追加を検討してください。localhostインターフェースへのマッピングは、セキュリティの観点から適切です。また、非標準のポート番号（59200）を使用することで、潜在的な競合を回避できます。
この非標準のポート設定について、プロジェクトのドキュメントに記載することをお勧めします。他の開発者がこの設定を理解し、適切に使用できるようになります。
packages/backend/src/server/api/endpoints/notes/favorites/delete.ts (1)
72-72: お気に入りのunindex処理が追加されていますが、改善の余地があります。お気に入りの削除後にunindex処理を追加したのは良い改善です。しかし、以下の点について検討が必要です：
- エラーハンドリング：unindex処理が失敗した場合の対応が必要です。
- トランザクション管理：削除とunindex処理を1つのトランザクションで行うことで、データの整合性を保つことができます。
以下のような実装を検討してください：
try { await this.db.transaction(async (transactionalEntityManager) => { await transactionalEntityManager.delete(NoteFavorite, exist.id); await this.advancedSearchService.unindexFavorite(exist.id, note.id, undefined, me.id); }); } catch (error) { this.logger.error('Failed to delete and unindex favorite', error); throw new ApiError('Failed to delete favorite'); }この実装により、削除とunindex処理が一つのトランザクションで行われ、エラーが発生した場合も適切に処理されます。
packages/backend/src/core/DeleteAccountService.ts (1)
76-76: 新しいメソッド呼び出しが追加されていますが、エラーハンドリングを検討してください。
advancedSearchService.unindexUserFavorites(user.id)の呼び出しが適切に追加されています。ローカルユーザーの削除プロセスに統合されており、ユーザーのお気に入りをインデックスから削除する目的に合致しています。しかし、このメソッド呼び出しにエラーハンドリングが含まれていません。エラーが発生した場合にアカウント削除プロセス全体が中断されないよう、try-catchブロックでこの呼び出しを囲むことを検討してください。
以下のようなエラーハンドリングの追加を提案します：
+ try { this.advancedSearchService.unindexUserFavorites(user.id); + } catch (error) { + console.error('Failed to unindex user favorites:', error); + // 必要に応じて、エラーをログに記録するか、適切に処理してください + }packages/frontend/src/pages/admin/other-settings.vue (1)
99-113: インデックス作成機能の改善を評価します。さらなる改善の提案があります。この変更は、管理者により細かな制御を提供し、特定のデータタイプのインデックス作成を可能にするため、良い改善です。しかし、以下の点について検討することをお勧めします：
すべてのデータタイプを一度にインデックス作成するための「すべて」オプションを追加することを検討してください。これにより、包括的なインデックス作成が必要な場合に便利です。
デフォルト選択を「reaction」から、より一般的なユースケースに変更することを検討してください。例えば、「notes」や新しく追加する「すべて」オプションなどが考えられます。
以下のような変更を提案します：
items: [{ + value: 'all', text: i18n.ts.all, +}, { value: 'notes', text: i18n.ts.note, }, { value: 'reaction', text: i18n.ts.reaction, }, { value: 'pollVote', text: i18n.ts.poll, }], -default: 'reaction', +default: 'all',この変更により、より柔軟で直感的なユーザー体験が提供できます。
packages/backend/src/server/api/endpoints/notes/polls/vote.ts (1)
149-151: 投票作成とID生成のロジックが改善されています。IDの生成が投票の挿入前に行われるようになり、より効率的になっています。これは良い変更です。
可読性向上のため、以下のように変数名を明確にすることを検討してください：
-const id = this.idService.gen(createdAt.getTime()); +const voteId = this.idService.gen(createdAt.getTime()); const vote = await this.pollVotesRepository.insertOne({ - id: id, + id: voteId, noteId: note.id, userId: me.id, choice: ps.choice, });packages/frontend/src/pages/settings/privacy.vue (2)
46-50: 新しいisIndexableスイッチの実装が適切です。新しいスイッチの実装は既存のパターンに従っており、適切に配置されています。ただし、ベータタグの扱いについて1点提案があります。
ベータタグ "yojo-art" の扱いを改善するために、以下の変更を検討してください：
- <span class="_beta">yojo-art</span> + <span class="_beta">{{ i18n.ts.yojoArt }}</span>これにより、ベータタグのテキストも他の部分と同様に国際化され、一貫性が向上します。
i18n.tsファイルにyojoArtの翻訳を追加することを忘れないでください。
46-50: 全体的な変更の要約と影響この PR では、ユーザーのプロファイルのインデックス可能性を制御する新しい
isIndexable設定が追加されました。これは以下の変更を含みます：
- テンプレート部分に新しい
MkSwitchコンポーネントが追加されました。- スクリプト部分に
isIndexableリアクティブ参照が追加されました。save()関数にisIndexableの値が含まれるようになりました。これらの変更により、ユーザーはより細かくプライバシー設定を制御できるようになります。実装は既存のコードパターンに従っており、一貫性があります。
この新機能の追加に伴い、以下の点を考慮することをお勧めします：
- この設定の影響を受ける可能性のある他のコンポーネントやAPIエンドポイントを確認し、必要に応じて更新してください。
- この新しい設定のユーザーへの影響と、それがどのように機能するかを説明するドキュメントを更新または作成することを検討してください。
- この機能のテストケースを追加し、期待通りに動作することを確認してください。
Also applies to: 98-98, 116-116
packages/backend/src/server/api/endpoints/i/update.ts (3)
37-37: インポートの追加を承認します。IdServiceのインポートが追加されました。これは新しい機能に関連していると思われます。
可読性を向上させるため、関連するインポートをグループ化することを検討してください。例えば、他のサービスのインポートと一緒に配置することができます。
183-183: 新しいプロパティの追加を承認します。
isIndexableプロパティがparamDefオブジェクトに追加されました。これはユーザープロファイルのインデックス可能性を制御するためのものと思われます。このプロパティの目的を明確にするために、簡単なコメントを追加することを推奨します。例：
isIndexable: { type: 'boolean' }, // ユーザープロファイルが検索エンジンによってインデックス可能かどうかを制御
Line range hint
1-696: 全体的な変更の確認と提案新しい
isIndexableプロパティの追加は一貫して実装されており、既存のコードパターンに従っています。しかし、この機能追加に伴ういくつかの考慮事項があります：
ドキュメンテーションの更新：この新機能についてのドキュメントを更新し、その使用方法と影響について説明することを推奨します。
セキュリティとプライバシーの影響：この設定がユーザーのプライバシーにどのような影響を与えるか、十分に検討してください。
パフォーマンスへの影響：インデックス作成の変更が検索機能やデータベースのパフォーマンスにどのような影響を与えるか評価してください。
既存のユーザーへの影響：この新しい設定のデフォルト値と、既存のユーザーアカウントへの適用方法を検討してください。
これらの点について、チームで議論し、必要に応じて追加の変更や文書化を行うことを推奨します。
packages/backend/src/core/activitypub/ApRendererService.ts (1)
542-542: LGTM: indexableプロパティの追加ユーザーのActivityPub表現に
indexableプロパティを追加したのは良い改善です。これにより、ユーザープロファイルの検索エンジンなどによるインデックス可能性を制御できるようになります。実装は簡潔で、他のブーリアンプロパティと一貫性があります。
一貫性をさらに高めるために、
indexableプロパティをdiscoverableプロパティの直後に配置することを検討してください。これにより、関連するプロパティがグループ化され、コードの可読性が向上します。discoverable: user.isExplorable, +indexable: user.isIndexable, publicKey: this.renderKey(user, keypair, '#main-key'), isCat: user.isCat, -indexable: user.isIndexable,packages/backend/src/core/entities/UserEntityService.ts (1)
569-569: 変更は適切です。ドキュメンテーションの追加を検討してください。
isIndexableプロパティの追加は適切に実装されています。このプロパティはユーザーのプロファイルが検索エンジンによってインデックス可能かどうかを制御するために使用されると思われます。セキュリティとプライバシーに関連する重要な機能であるため、このプロパティの目的と使用方法についてコードコメントを追加することを推奨します。例えば：
// isIndexable: ユーザープロファイルが検索エンジンによってインデックス可能かどうかを示す isIndexable: user.isIndexable,これにより、他の開発者がこのプロパティの重要性をより理解しやすくなります。
packages/backend/src/core/activitypub/models/ApPersonService.ts (2)
411-411: 新しいプロパティisIndexableの追加を承認します。この変更は、ユーザーの検索エンジンインデックス可能性を制御する新機能を導入しています。デフォルトで
trueに設定されているのは適切です。この新しいプロパティの目的と使用方法についてのドキュメントを追加することをお勧めします。
652-655:updatesオブジェクトへのisIndexableプロパティの追加を承認します。この変更は、先程のコード変更と一貫性があり、適切です。
コードの一貫性と保守性を向上させるために、
isIndexableの値を設定する共通の関数を作成することを検討してください。例えば：function getIsIndexable(person: any): boolean { return person.indexable ?? true; } // 使用例 isIndexable: getIsIndexable(person),この関数を両方の場所で使用することで、将来的な変更が容易になります。
packages/backend/src/core/NoteCreateService.ts (1)
146-146: 不要な変更の可能性があります。この行の変更は、
visibilityプロパティの型をstring | nullからstring | nullに変更していますが、これらは同じ型です。この変更は意図的なものではない可能性があります。変更が意図的でない場合は、この行を元に戻すことをお勧めします。
packages/backend/test/e2e/users.ts (2)
70-70: ユーザーオブジェクトにisIndexableプロパティを追加しました。この変更は適切です。新しい
isIndexableプロパティをユーザーオブジェクトに追加することで、このプロパティに関するテストが可能になります。このプロパティの目的を説明するコメントを追加することをお勧めします。例えば：
+ // ユーザーのプロフィールが検索エンジンにインデックスされるかどうかを示す isIndexable: user.isIndexable,
435-436:isIndexableプロパティのテストケースを追加しました。この変更は適切です。新しい
isIndexableプロパティのtrueとfalseの両方のケースをテストしており、包括的なテストになっています。より具体的なテストケースを追加することを検討してください。例えば：
isIndexableの値が正しくAPIレスポンスに反映されているか確認するテストisIndexableの値を変更した後、その変更が保存され、再取得時に正しく反映されているか確認するテストisIndexableの値によって、ユーザーの検索結果への表示/非表示が適切に制御されているか確認するテストこれらの追加テストにより、
isIndexableプロパティの機能がより完全にカバーされます。packages/cherrypick-js/etc/cherrypick-js.api.md (1)
244-246: 新しい型宣言が適切に追加されています。
AdminFullIndexRequest型の追加は適切に行われており、他の管理者レベルのリクエスト型と一貫性があります。ただし、この新しいエンドポイントの目的や使用方法について、開発者向けのドキュメントを追加することをお勧めします。
+/** + * 管理者用の完全インデックス作成リクエスト + * @description このエンドポイントの目的や使用方法についての説明を追加してください + */ type AdminFullIndexRequest = operations['admin___full-index']['requestBody']['content']['application/json'];packages/cherrypick-js/src/autogen/types.ts (1)
9570-9577: リクエストボディの追加は適切です。
admin___full-index操作にリクエストボディを追加することで、APIの定義が明確になりました。indexフィールドに対する列挙型の使用は、型安全性を向上させる良い実践です。可読性を向上させるために、
indexの各値に対してJSDocコメントを追加することを検討してください。例：/** @enum {string} */ index: | 'notes' /** ノートのインデックス */ | 'reaction' /** リアクションのインデックス */ | 'pollVote' /** 投票のインデックス */;packages/backend/src/core/ReactionService.ts (1)
104-104: 依存性注入の一貫性を確認してください。他のサービスは
@Injectデコレータを使用して依存性を注入していますが、AdvancedSearchServiceでは使用していません。一貫性のため、@Injectデコレータを追加することを検討してください。
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (34)
- .github/cherrypick/test-opensearch.yml (1 hunks)
- .github/workflows/test-backend.yml (2 hunks)
- CHANGELOG_YOJO.md (1 hunks)
- locales/index.d.ts (2 hunks)
- locales/ja-JP.yml (2 hunks)
- packages/backend/migration/1726205819617-AddIsIndexable.js (1 hunks)
- packages/backend/src/core/AdvancedSearchService.ts (16 hunks)
- packages/backend/src/core/ClipService.ts (5 hunks)
- packages/backend/src/core/DeleteAccountService.ts (3 hunks)
- packages/backend/src/core/NoteCreateService.ts (2 hunks)
- packages/backend/src/core/QueryService.ts (1 hunks)
- packages/backend/src/core/ReactionService.ts (4 hunks)
- packages/backend/src/core/SearchService.ts (1 hunks)
- packages/backend/src/core/activitypub/ApRendererService.ts (1 hunks)
- packages/backend/src/core/activitypub/misc/contexts.ts (1 hunks)
- packages/backend/src/core/activitypub/models/ApPersonService.ts (2 hunks)
- packages/backend/src/core/activitypub/type.ts (1 hunks)
- packages/backend/src/core/entities/UserEntityService.ts (2 hunks)
- packages/backend/src/models/User.ts (1 hunks)
- packages/backend/src/server/api/endpoints/admin/full-index.ts (2 hunks)
- packages/backend/src/server/api/endpoints/i/update.ts (3 hunks)
- packages/backend/src/server/api/endpoints/notes/favorites/create.ts (3 hunks)
- packages/backend/src/server/api/endpoints/notes/favorites/delete.ts (3 hunks)
- packages/backend/src/server/api/endpoints/notes/polls/vote.ts (3 hunks)
- packages/backend/test/compose.yml (1 hunks)
- packages/backend/test/e2e/search-notes.ts (1 hunks)
- packages/backend/test/e2e/users.ts (3 hunks)
- packages/cherrypick-js/etc/cherrypick-js.api.md (2 hunks)
- packages/cherrypick-js/src/autogen/endpoint.ts (2 hunks)
- packages/cherrypick-js/src/autogen/entities.ts (1 hunks)
- packages/cherrypick-js/src/autogen/types.ts (2 hunks)
- packages/frontend/src/components/MkUserSetupDialog.Privacy.vue (2 hunks)
- packages/frontend/src/pages/admin/other-settings.vue (1 hunks)
- packages/frontend/src/pages/settings/privacy.vue (3 hunks)
Additional context used
Gitleaks
.github/workflows/test-backend.yml
147-148: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
packages/backend/test/compose.yml
20-20: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Markdownlint
CHANGELOG_YOJO.md
18-18: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
19-19: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
20-20: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
21-21: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
22-22: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
23-23: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
17-17: Expected: 4; Actual: 1
Unordered list indentation(MD007, ul-indent)
18-18: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
19-19: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
20-20: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
21-21: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
22-22: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
23-23: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
17-17: Column: 1
Hard tabs(MD010, no-hard-tabs)
18-18: Column: 3
Hard tabs(MD010, no-hard-tabs)
19-19: Column: 3
Hard tabs(MD010, no-hard-tabs)
20-20: Column: 3
Hard tabs(MD010, no-hard-tabs)
21-21: Column: 3
Hard tabs(MD010, no-hard-tabs)
22-22: Column: 3
Hard tabs(MD010, no-hard-tabs)
23-23: Column: 3
Hard tabs(MD010, no-hard-tabs)
Additional comments not posted (46)
.github/cherrypick/test-opensearch.yml (2)
12-14: Redis設定が適切です。ローカルテスト環境用のRedis設定として問題ありません。ローカルホストの使用と非標準ポートの選択は、他の設定と一貫性があり適切です。
15-15: 'id'フィールドの目的を明確にしてください。'id: aidx'の設定が含まれていますが、この設定の目的や重要性が不明確です。
以下の点について説明をお願いします：
- このIDフィールドの具体的な用途は何ですか？
- 'aidx'という値にはどのような意味がありますか？
- この設定はアプリケーションのどの部分で使用されていますか？
この情報を追加することで、将来のメンテナンスや他の開発者の理解が容易になります。可能であれば、この設定の目的を説明するコメントを追加することをお勧めします。
packages/backend/migration/1726205819617-AddIsIndexable.js (3)
1-4: 著作権表示とライセンス情報が適切に記載されています。ファイルヘッダーには、適切な著作権表示とライセンス情報が含まれています。これはオープンソースプロジェクトにとって重要な要素です。
6-8: マイグレーションクラスの命名が適切です。クラス名
AddIsIndexable1726205819617にタイムスタンプが含まれており、マイグレーションの順序を保証するのに役立ちます。
9-11: 新しいカラム追加の影響を考慮してください。
isIndexableカラムをデフォルト値trueで追加していますが、既存のユーザーへの影響を考慮する必要があります。以下の点について検討してください：
- この変更が既存のユーザーデータにどのような影響を与えるか。
- デフォルト値を
trueにする理由と、それが適切かどうか。- この変更に関連して、アプリケーションコードの更新が必要かどうか。
Verification successful
isIndexableカラムの追加に関する確認デフォルト値
trueの設定およびアプリケーションコードへの影響について問題は確認されませんでした。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # アプリケーションコード内で isIndexable フィールドの使用を確認 rg --type typescript 'isIndexable'Length of output: 69
Script:
# 新しいシェルスクリプトを実行して `isIndexable` フィールドの使用を確認します rg 'isIndexable'Length of output: 2542
Script:
#!/bin/bash # 'isIndexable' フィールドの使用を確認するために、正しいファイルタイプでrgを実行します rg --type ts 'isIndexable'Length of output: 1428
packages/backend/test/compose.yml (1)
22-28: リソース制限の設定が適切です。メモリロックの制限を無制限に設定し、ファイルディスクリプタの制限を65536に設定しているのは、OpenSearchインスタンスに適しています。これらの設定により、パフォーマンスが向上し、スワッピングが防止されます。
packages/backend/src/server/api/endpoints/notes/favorites/delete.ts (2)
11-11: 新しいインポートが適切に追加されています。AdvancedSearchServiceのインポートが正しく追加されており、プロジェクトの規約に従っています。
51-51: コンストラクタの更新が適切に行われています。advancedSearchServiceが正しく追加され、型付けされています。既存の依存性注入のパターンに従っています。
packages/frontend/src/components/MkUserSetupDialog.Privacy.vue (3)
18-25: テンプレートの変更が適切に実装されています。新しい「インデックス可能」設定が既存のUIパターンに沿って追加されており、i18nを使用して適切にローカライズされています。ユーザーエクスペリエンスの一貫性が保たれています。
Line range hint
1-78: 全体的な変更の評価新しい「インデックス可能」機能の実装は全体的に良好です。以下の点が特に評価できます：
- 既存のコンポーネント構造に適切に統合されています。
- 新しい機能に対して一貫してi18nによるローカライゼーションが使用されています。
- テンプレートとスクリプトの両方で機能が一貫して実装されています。
ただし、
isIndexableの初期値の設定については再考の余地があります。また、この新機能がユーザーのプライバシーに与える影響を考慮し、ユーザーガイドやドキュメントの更新も検討してください。全体として、この変更はユーザーに新しい制御オプションを提供し、プライバシー設定の柔軟性を向上させています。
63-63:isIndexableの初期値について再考してください。
isIndexableの初期値がtrueに設定されていますが、これが適切かどうか確認してください。ユーザーのプライバシーを考慮すると、デフォルトでfalseに設定し、ユーザーが明示的に選択できるようにすることも検討に値します。また、この設定がユーザーの既存のアカウント設定と同期されていることを確認してください。新規ユーザーと既存ユーザーの両方に対して、この設定が正しく機能することを確認する必要があります。
以下のスクリプトを実行して、既存のユーザー設定との整合性を確認してください：
Also applies to: 71-71
packages/backend/src/core/DeleteAccountService.ts (3)
16-16: インポート文が適切に追加されています。
AdvancedSearchServiceのインポートが正しく追加されており、他のインポート文と一貫性のある相対パスを使用しています。
31-31: コンストラクタの変更が適切に行われています。
AdvancedSearchServiceが正しくコンストラクタに注入されています。他のサービスと同じパターンでprivate修飾子が使用されており、命名規則も一貫しています。
Line range hint
1-94: 全体的な変更の評価この変更は、アカウント削除プロセスに
AdvancedSearchServiceを統合することで、機能を向上させています。主な変更点は以下の通りです：
AdvancedSearchServiceのインポート- コンストラクタへの新しいサービスの注入
- アカウント削除プロセス中のユーザーお気に入りのインデックス解除
これらの変更は既存のコードスタイルと構造に一貫しており、アカウント削除プロセスの完全性を向上させています。唯一の改善点として、新しく追加されたメソッド呼び出しにエラーハンドリングを追加することを提案しました。
全体として、この変更は適切に実装されており、アプリケーションの機能を向上させるものと評価します。
.github/workflows/test-backend.yml (3)
13-13: トリガーパスの追加は適切です。
.github/cherrypick/test*.ymlファイルをトリガーパスに追加することで、テスト設定ファイルの変更時にワークフローが実行されるようになります。これは良い実践です。Also applies to: 20-20
118-170: 新しいジョブの構造は一貫性があり、適切です。
opensearch-e2eジョブは既存のジョブと同様の構造を持ち、同じNode.jsバージョンを使用しています。これは保守性と互換性の観点から良い実践です。Tools
Gitleaks
147-148: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
165-165: テストの範囲を確認してください。現在のテストコマンドは
search-notes.tsファイルのみを実行しているようです。これが意図的なものか、より広範囲のテストを実行する必要があるかを確認してください。より包括的なテストカバレッジを確保するために、以下のスクリプトを実行して、他のOpenSearch関連のテストファイルがないか確認してください：
packages/backend/src/server/api/endpoints/notes/polls/vote.ts (3)
18-18: LGTM: AdvancedSearchServiceのインポートが適切に追加されています。インポート文が正しく配置され、パスも適切であると思われます。
99-99: コンストラクタの更新が適切に行われています。AdvancedSearchServiceが正しく依存性注入のパターンに従って追加されています。これにより、新しい機能を適切に利用できるようになります。
156-159: 投票のインデックス化が適切に実装されています。AdvancedSearchServiceを使用して投票をインデックス化する新しいロジックが追加されており、検索機能が向上します。これは素晴らしい改善です。
インデックス化処理中にエラーが発生した場合の対応を確認してください。エラーハンドリングを追加することで、より堅牢な実装になります。以下のスクリプトを実行して、他の箇所でのエラーハンドリングの実装を確認できます：
また、インデックス化の失敗が投票プロセス全体に影響を与えないようにするため、以下のような実装を検討してください：
try { await this.advancedSearchService.indexVote(voteId, { noteId: note.id, userId: me.id, }); } catch (error) { this.logger.error('投票のインデックス化に失敗しました', error); // ここでエラーを記録するが、プロセスは続行する }packages/frontend/src/pages/settings/privacy.vue (1)
98-98: スクリプト部分の変更が適切に実装されています。
isIndexableプロパティの追加とsave()関数への組み込みが正しく行われています。これらの変更は既存のパターンに従っており、コンポーネントの他の部分と一貫性があります。Also applies to: 116-116
packages/backend/src/models/User.ts (1)
197-203: 新しいisIndexableプロパティの目的と影響を明確にしてください。新しく追加された
isIndexableプロパティについて、以下の点を確認・検討してください：
- このプロパティの具体的な目的と使用方法を説明してください。
- デフォルト値が
trueに設定されていますが、これが適切かどうか確認してください。- このプロパティの意味を明確にするために、コメントフィールドに説明を追加することを推奨します。
- この変更が既存のデータやクエリにどのような影響を与えるか検討してください。特に、既存のユーザーレコードに対する影響を考慮してください。
- この新しいプロパティを使用するための関連するビジネスロジックやAPIの変更が必要かどうか確認してください。
以下のスクリプトを実行して、
isIndexableプロパティの使用状況を確認してください：このスクリプトの結果を基に、新しいプロパティの影響範囲を評価し、必要な追加の変更を特定してください。
Verification successful
isIndexableプロパティの追加は適切に行われており、コード全体での使用も確認されました。ただし、このプロパティの目的と使用方法についてのコメントが不足しているため、他の開発者が理解しやすいようにコメントを追加することを推奨します。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: `isIndexable` プロパティの使用状況を確認する # Test: `isIndexable` プロパティが他のファイルで使用されているか検索する echo "Searching for usages of isIndexable property:" rg --type typescript 'isIndexable' # Test: `User` モデルを使用している箇所を検索する echo "\nSearching for usages of User model:" rg --type typescript 'MiUser'Length of output: 320
Script:
#!/bin/bash # Description: `isIndexable` プロパティの使用状況を確認する # Test: `isIndexable` プロパティが他のファイルで使用されているか検索する echo "Searching for usages of isIndexable property:" rg --type ts --type tsx 'isIndexable' # Test: `User` モデルを使用している箇所を検索する echo "\nSearching for usages of User model:" rg --type ts --type tsx 'MiUser'Length of output: 312
Script:
#!/bin/bash # Description: Ripgrep のサポートするファイルタイプを一覧表示し、TypeScriptファイルの検索方法を確認する # List all supported file types echo "Listing all supported file types in ripgrep:" rg --type-list # Alternatively, search TypeScript files by extension without using --type echo "\nSearching for usages of isIndexable property in .ts and .tsx files:" rg --glob '*.ts' --glob '*.tsx' 'isIndexable' echo "\nSearching for usages of MiUser in .ts and .tsx files:" rg --glob '*.ts' --glob '*.tsx' 'MiUser'Length of output: 123430
CHANGELOG_YOJO.md (1)
7-8: 重要な注意事項が追加されています。インデックス構造の変更に関する重要な情報が追加されています。ユーザーに再インデックスの必要性を明確に伝えています。
packages/backend/src/core/activitypub/type.ts (1)
195-195: LGTM: 新しいプロパティの追加が適切に行われています。
indexableプロパティの追加は適切に行われており、オプショナルな boolean 型として定義されています。これはアクターのインデックス可能性を指定するのに適しています。ただし、以下の改善を提案します：
- このプロパティの目的と使用方法を説明するコメントを追加することを検討してください。例えば：
/** アクターがインデックス可能かどうかを示します。trueの場合、検索エンジンなどでインデックス化できることを意味します。 */ indexable?: boolean;このようなドキュメンテーションは、将来のメンテナンスや他の開発者の理解を助けるのに役立ちます。
packages/backend/src/core/activitypub/misc/contexts.ts (1)
546-546: 変更は適切に実装されています。使用目的の明確化をお願いします。
indexableプロパティがextension_context_definitionオブジェクトに正しく追加されています。この実装は既存の構造と命名規則に一致しています。この新しいプロパティの目的と使用方法について、詳細な説明を追加していただけますか？また、この変更がシステムの他の部分にどのような影響を与えるかを確認するために、以下のスクリプトを実行してください：
Verification successful
変更が正しく適用されていることを確認しました。
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: 'indexable' プロパティの使用状況を確認します # Test: 'indexable' プロパティが使用されている箇所を検索します rg --type typescript --type javascript 'indexable' # Test: 'toot:indexable' の使用箇所を検索します rg --type typescript --type javascript 'toot:indexable'Length of output: 176
Script:
#!/bin/bash # Description: 'indexable' プロパティの使用状況を確認します # Test: 'indexable' プロパティが使用されている箇所を検索します rg --type ts --type js 'indexable' # Test: 'toot:indexable' の使用箇所を検索します rg --type ts --type js 'toot:indexable'Length of output: 2501
packages/backend/src/server/api/endpoints/i/update.ts (1)
357-357: ユーザー更新ロジックの変更を承認します。
isIndexableプロパティがupdatesオブジェクトに追加されました。これは、入力に応じてユーザーのインデックス可能性ステータスを更新するためのものです。この新しいプロパティの潜在的な誤用を防ぐため、追加のエラーハンドリングやバリデーションの実装を検討してください。例えば、特定の条件下でのみこの設定を許可するなどです。以下のスクリプトで、他の類似のプロパティの処理方法を確認できます：
Verification successful
ユーザー更新ロジックの変更を確認しました。
isIndexableプロパティの追加は、他のブール値プロパティと一貫した方法で処理されています。特別なエラーハンドリングは必要ありません。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 他のブール値プロパティの処理方法を確認 rg "typeof ps\.\w+ === 'boolean'" -A 3 packages/backend/src/server/api/endpoints/i/update.tsLength of output: 1776
packages/backend/src/core/NoteCreateService.ts (1)
755-756: 検索インデックスの追加が良好です。MeiliSearchとOpenSearchへのノートのインデックス追加は、検索機能を向上させる良い改善です。
packages/backend/test/e2e/users.ts (2)
12-12: DEFAULT_POLICIESのインポートを復活させました。この変更は適切です。DEFAULT_POLICIESは、新しく追加された
isIndexableプロパティのデフォルト設定を含んでいる可能性が高いため、このインポートは必要です。
Line range hint
1-1000: 全体的なレビューサマリー
- DEFAULT_POLICIESのインポートの復活
- ユーザーオブジェクトへの
isIndexableプロパティの追加isIndexableプロパティのテストケースの追加これらの変更は、新しい
isIndexable機能を実装するために必要で一貫性のあるものです。テストカバレッジを向上させるための提案をいくつか行いましたが、全体的に変更は適切に行われています。packages/cherrypick-js/src/autogen/endpoint.ts (1)
684-684: 変更を承認します。潜在的な互換性の問題に注意してください。この変更は正しく実装されていますが、以下の点に注意が必要です：
- この変更は、パラメータなしでこのエンドポイントを使用している既存のコードを破壊する可能性があります。
AdminFullIndexRequest型がプロジェクト内の適切な場所で定義されていることを確認してください。- API ドキュメントとクライアント側のコードを更新することをお勧めします。
AdminFullIndexRequest型の定義を確認するために、以下のスクリプトを実行してください：Verification successful
AdminFullIndexRequest型の存在が確認されました。
packages/cherrypick-js/src/autogen/entities.tsに定義されています。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # AdminFullIndexRequest 型の定義を検索 rg --type typescript "type AdminFullIndexRequest = " -A 10Length of output: 95
Script:
#!/bin/bash # AdminFullIndexRequest 型の定義を検索（--type オプションを削除） rg "type AdminFullIndexRequest =" -A 10Length of output: 2990
packages/cherrypick-js/src/autogen/entities.ts (1)
101-101: 新しい型AdminFullIndexRequestが正しく追加されています。この新しい型の追加は、管理者向けの全文インデックス作成操作が API に導入されたことを示唆しています。型の宣言自体に問題は見られません。
この新しい型の使用状況を確認するために、以下のスクリプトを実行してください：
Verification successful
AdminFullIndexRequest型の使用が確認されました。以下のファイルで
AdminFullIndexRequest型が使用されています:
- packages/cherrypick-js/src/autogen/entities.ts
- packages/cherrypick-js/src/autogen/endpoint.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 説明: `AdminFullIndexRequest` 型の使用箇所を検索します。 # テスト: 型の使用箇所を検索します。期待される結果: 型が適切に使用されている箇所が表示されます。 rg --type typescript "AdminFullIndexRequest"Length of output: 79
Script:
#!/bin/bash # 説明: `AdminFullIndexRequest` 型の使用箇所を `.ts` および `.tsx` ファイル内で検索します。 # テスト: 型の使用箇所を検索します。期待される結果: 型が適切に使用されている箇所が表示されます。 rg "AdminFullIndexRequest" --glob "*.ts" --glob "*.tsx"Length of output: 409
locales/ja-JP.yml (3)
880-880: 新しいエントリが適切に追加されています。
makeIndexableの翻訳が簡潔かつ適切に行われています。ノート検索のインデックス作成を許可するという意味が正確に伝わります。
881-883: 説明文が適切に追加されています。
makeIndexableDescriptionの翻訳が明確で情報量が豊富です。インデックス作成機能をオフにした場合の影響と例外について適切に説明されています。読みやすさを考慮して改行が適切に使用されています。
Line range hint
1-3184: 全体的に良好な変更が行われていますが、小さな修正が必要です。新機能の翻訳が適切に追加され、既存のコンテンツも改善されています。唯一の問題点は、
_reIndexOpenSearchセクションのキー名に軽微なタイプミスがあることです。このタイプミスを修正すれば、ファイル全体の品質が向上します。packages/cherrypick-js/etc/cherrypick-js.api.md (1)
1297-1297: entities名前空間への適切な追加
AdminFullIndexRequest型がentities名前空間に正しく追加されており、アルファベット順も維持されています。これはコードの一貫性と可読性を向上させます。locales/index.d.ts (2)
3534-3543: 新しい検索関連の設定が追加されました。
makeIndexableとmakeIndexableDescriptionプロパティがLocaleインターフェースに追加されました。これらは、ユーザーのノートを検索可能にするかどうかを制御する新しい設定に関連しているようです。プライバシーに配慮した重要な機能追加だと考えられます。実装は既存のコードスタイルと一貫性があり、適切に行われています。
Line range hint
1-11943: ファイル全体の確認完了ファイルの残りの部分を確認しましたが、他に大きな変更や問題は見つかりませんでした。新しく追加された
makeIndexableとmakeIndexableDescriptionプロパティは、既存の構造にうまく適合しており、ファイル全体の一貫性は保たれています。ローカライゼーション文字列の追加は適切に行われており、CherryPickアプリケーションの機能拡張をサポートしています。
packages/cherrypick-js/src/autogen/types.ts (1)
19942-19942:isIndexableプロパティの追加は適切です。新しい
isIndexableオプションプロパティの追加は、既存の構造と一貫性があり、適切に実装されています。この変更の目的と使用方法についてもう少し情報を提供していただけますか？以下のスクリプトを実行して、この新しいプロパティの使用状況を確認できます：
Verification successful
isIndexableプロパティの追加を確認しました。
isIndexableプロパティがコードベース内で適切に使用されており、他の関連部分との整合性も保たれています。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: 新しい isIndexable プロパティの使用状況を確認する # Test: isIndexable プロパティの使用を検索する rg --type typescript 'isIndexable'Length of output: 69
Script:
#!/bin/bash # Description: 新しい isIndexable プロパティの使用状況を確認する # Test: isIndexable プロパティの使用を検索する rg --type ts 'isIndexable'Length of output: 1428
packages/backend/src/server/api/endpoints/admin/full-index.ts (2)
20-26: 'index' パラメータの定義が正しく設定されています'index' パラメータが適切に定義され、必須項目として設定されています。
36-43: 条件分岐の実装が適切です'ps.index' の値に応じて、正しい再インデックスメソッドが呼び出されています。
packages/backend/src/core/ClipService.ts (3)
23-23: 変更内容は問題ありませんAdvancedSearchServiceのインポートが正しく追加されています。
186-186: unindexFavorite関数へのundefinedの渡し方を確認してくださいunindexFavorite関数の第一引数にundefinedを渡しています。この関数がundefinedを適切に処理できるか、または引数が必須でないか確認してください。
Verification successful
unindexFavorite関数の第一引数にundefinedを渡すことは適切です。
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 説明: unindexFavorite関数の定義を確認し、第一引数がundefinedを許容するか確認します。 # テスト: unindexFavorite関数の定義を検索 # 期待: 第一引数がオプショナルである、またはundefinedを処理できる rg 'unindexFavorite\(' --type ts -A 5Length of output: 1473
54-54: 依存関係の注入が適切に行われていることを確認してくださいコンストラクタにAdvancedSearchServiceが追加されました。ClipServiceをインスタンス化する全ての箇所で、この新しい依存関係が正しく渡されていることを確認してください。
packages/backend/src/core/ReactionService.ts (1)
33-33: AdvancedSearchServiceのインポートが正しく追加されています。packages/backend/src/server/api/endpoints/notes/favorites/create.ts (2)
62-62: AdvancedSearchServiceの依存性注入を確認してください。
AdvancedSearchServiceが正しく依存性注入されるよう、DIコンテナでの登録を確認してください。検証のために、以下のスクリプトを実行してください。
Verification successful
AdvancedSearchService の依存性注入が正しく設定されています。
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: AdvancedSearchServiceがDIコンテナに登録されていることを確認します。 # Test: AdvancedSearchServiceのプロバイダ登録を検索します。期待値: AdvancedSearchServiceがプロバイダとして登録されている。 rg --type typescript 'provide.*AdvancedSearchService'Length of output: 90
Script:
#!/bin/bash # Description: AdvancedSearchServiceがDIコンテナに登録されていることを確認します。 # Test: AdvancedSearchServiceのプロバイダ登録を検索します。期待値: AdvancedSearchServiceがプロバイダとして登録されている。 rg --type ts 'provide.*AdvancedSearchService'Length of output: 202
84-86: IDの手動生成による一貫性を確認してください。
idService.gen()を使用してIDを手動生成しています。この方法でIDの一意性や一貫性に問題がないか確認してください。他の部分でIDが自動生成されている場合、一貫性を保つために統一することを検討してください。検証のために、以下のスクリプトを実行してください。
| opensearch: | ||
| host: 127.0.0.1 | ||
| port: 59200 | ||
| user: 'admin' | ||
| pass: 'eF53xwF4NYjrcXXwZ2CHgpwFL' | ||
| ssl: false | ||
| index: 'instancename' #なんでもいい |
There was a problem hiding this comment.
OpenSearch設定にセキュリティ上の懸念があります。改善が必要です。
ローカルテスト環境用のOpenSearch設定として機能しますが、いくつかの問題点があります。
以下の改善を強く推奨します：
-
クレデンシャルのハードコーディング：
- 環境変数や安全な秘密管理システムを使用してください。
user: ${OPENSEARCH_USER} pass: ${OPENSEARCH_PASSWORD}
-
SSL設定：
- ローカルテストでもSSLを有効にすることを検討してください。本番環境との一貫性が保てます。
ssl: true
-
インデックス名：
- 「なんでもいい」というコメントは適切ではありません。具体的で一貫したインデックス名を使用してください。
index: 'test_cherrypick_index' # テスト環境用のインデックス名
これらの変更により、セキュリティが向上し、設定の一貫性と明確性が確保されます。
| if (note.hasPoll) { | ||
| this.pollsRepository.findOneBy({ noteId: note.id }).then( (poll) => { | ||
| this.indexNote(note, poll ? poll.choices : undefined); | ||
| }); | ||
| } | ||
| this.indexNote(note, undefined); |
There was a problem hiding this comment.
fullIndexNoteメソッドでのindexNoteの二重呼び出しに注意
note.hasPoll が true の場合でも、その後に this.indexNote(note, undefined); が呼び出されています。同じノートを二度インデックスすることになり、不要な処理となる可能性があります。
if (note.hasPoll) {
this.pollsRepository.findOneBy({ noteId: note.id }).then((poll) => {
this.indexNote(note, poll ? poll.choices : undefined);
});
-}
-this.indexNote(note, undefined);
+} else {
+ this.indexNote(note, undefined);
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (note.hasPoll) { | |
| this.pollsRepository.findOneBy({ noteId: note.id }).then( (poll) => { | |
| this.indexNote(note, poll ? poll.choices : undefined); | |
| }); | |
| } | |
| this.indexNote(note, undefined); | |
| if (note.hasPoll) { | |
| this.pollsRepository.findOneBy({ noteId: note.id }).then( (poll) => { | |
| this.indexNote(note, poll ? poll.choices : undefined); | |
| }); | |
| } else { | |
| this.indexNote(note, undefined); | |
| } |
| public async unindexFavorite(id?: string, noteId?: string, clipId?: string, userId?: string) { | ||
| if (id) { | ||
| this.unindexByQuery(this.favoriteIndex, { | ||
| bool: { | ||
| must: [ | ||
| { term: { userId: { value: userId } } }, | ||
| { term: { noteId: { value: noteId } } }, | ||
| ], | ||
| must_not: [ | ||
| { exists: { field: 'clipId' } }, | ||
| ], | ||
| }, | ||
| }); | ||
| } else { | ||
| this.unindexByQuery(this.favoriteIndex, { | ||
| bool: { | ||
| must: [ | ||
| { term: { noteId: { value: noteId } } }, | ||
| { term: { clipId: { value: clipId } } }, | ||
| { exists: { field: 'clipId' } }, | ||
| ], | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
unindexFavoriteメソッドでのidパラメータの使用方法を見直してください
id が指定されている場合でも、クエリを使用して削除を行っています。id を直接使用して削除する方が効率的です。
if (id) {
- this.unindexByQuery(this.favoriteIndex, {
- bool: {
- must: [
- { term: { userId: { value: userId } } },
- { term: { noteId: { value: noteId } } },
- ],
- must_not: [
- { exists: { field: 'clipId' } },
- ],
- },
- });
+ this.unindexById(this.favoriteIndex, id);
} else {
// クリップの削除処理
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public async unindexFavorite(id?: string, noteId?: string, clipId?: string, userId?: string) { | |
| if (id) { | |
| this.unindexByQuery(this.favoriteIndex, { | |
| bool: { | |
| must: [ | |
| { term: { userId: { value: userId } } }, | |
| { term: { noteId: { value: noteId } } }, | |
| ], | |
| must_not: [ | |
| { exists: { field: 'clipId' } }, | |
| ], | |
| }, | |
| }); | |
| } else { | |
| this.unindexByQuery(this.favoriteIndex, { | |
| bool: { | |
| must: [ | |
| { term: { noteId: { value: noteId } } }, | |
| { term: { clipId: { value: clipId } } }, | |
| { exists: { field: 'clipId' } }, | |
| ], | |
| }, | |
| }); | |
| } | |
| public async unindexFavorite(id?: string, noteId?: string, clipId?: string, userId?: string) { | |
| if (id) { | |
| this.unindexById(this.favoriteIndex, id); | |
| } else { | |
| this.unindexByQuery(this.favoriteIndex, { | |
| bool: { | |
| must: [ | |
| { term: { noteId: { value: noteId } } }, | |
| { term: { clipId: { value: clipId } } }, | |
| { exists: { field: 'clipId' } }, | |
| ], | |
| }, | |
| }); | |
| } |
packages/backend/src/server/api/endpoints/notes/favorites/create.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
.github/workflows/test-backend.yml (1)
167-170: Codecovトークンの扱いを改善してください。現在、CodecovトークンがGitHub Secretsを通じて参照されていますが、これは良い実践です。しかし、さらなるセキュリティ向上のため、以下の点を検討してください：
${{ secrets.CODECOV_TOKEN }}の使用を継続してください。- 可能であれば、このトークンに対して最小限の権限を設定してください。
- トークンを定期的にローテーションする仕組みを検討してください。
これらの実践により、セキュリティがさらに強化されます。
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .github/workflows/test-backend.yml (2 hunks)
- packages/backend/test/e2e/search-notes.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/backend/test/e2e/search-notes.ts
Additional context used
Gitleaks
.github/workflows/test-backend.yml
147-148: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Additional comments not posted (2)
.github/workflows/test-backend.yml (2)
13-13: トリガーパスの追加が適切です。
.github/cherrypick/test*.ymlファイルをトリガーパスに追加したのは良い変更です。これにより、テスト設定ファイルが変更された際にワークフローが実行されるため、テスト環境の一貫性が保たれます。Also applies to: 20-20
118-170: 新しいopensearch-e2eジョブの追加を承認します。全体的な構造が既存の
e2eジョブと一貫しており、OpenSearchを含むエンドツーエンドテストを実行する良い方法です。特に、OpenSearch用の特定のテスト設定ファイルを使用している点（161行目）は適切な実践です。Tools
Gitleaks
147-148: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (4)
packages/backend/src/core/AdvancedSearchService.ts (4)
Line range hint
143-170: 新しいインデックスとリポジトリの追加が適切に行われています。リアクション、投票、お気に入りの検索機能拡張のための変更が適切に実装されています。これにより、より包括的な検索機能が提供されることが期待できます。
可読性向上のため、新しく追加されたインデックスのプロパティにコメントを追加することを検討してください。例えば：
+ /** リノートのインデックス */ private renoteIndex: string; + /** リアクションのインデックス */ private reactionIndex: string; + /** 投票のインデックス */ private pollVoteIndex: string; + /** お気に入りとクリップのインデックス */ private favoriteIndex: string;
330-379: 新しいインデックス作成メソッドが適切に実装されています。リアクション、投票、お気に入りのインデックス作成メソッドが追加され、共通のインデックス作成ロジックを使用している点が良いです。これにより、コードの再利用性が向上しています。
エラーハンドリングを改善するため、
indexメソッドでのエラーをキャッチし、呼び出し元に伝播させることを検討してください。例えば：@bindThis -private async index(index: string, id: string, body: any ) { +private async index(index: string, id: string, body: any): Promise<void> { if (!this.opensearch) return; - await this.opensearch.index({ - index: index, - id: id, - body: body, - }).catch((error) => { - this.logger.error(error); - }); + try { + await this.opensearch.index({ + index: index, + id: id, + body: body, + }); + } catch (error) { + this.logger.error(error); + throw new Error(`Failed to index document: ${error.message}`); + } }この変更により、インデックス作成の失敗を呼び出し元で適切に処理できるようになります。
Line range hint
835-1000: 検索機能の改善が適切に実装されています。
searchメソッドにフィルタリングロジックが追加され、新しいfilterメソッドによってビジビリティやユーザー固有のフィルタリングが実装されています。これにより、より細かな検索結果の制御が可能になっています。パフォーマンス改善のため、以下の点を検討してください：
filterメソッド内でのデータベースクエリの最適化：
- ユーザー情報のキャッシュを活用し、データベースアクセスを減らすことができます。
非同期処理の並列化：
Promise.all()を使用して、複数の非同期操作を並列で実行することで、全体的な処理時間を短縮できる可能性があります。インデックスの活用：
- 頻繁に検索される項目に対してインデックスを作成し、クエリのパフォーマンスを向上させることができます。
これらの最適化により、大規模なデータセットに対しても効率的な検索が可能になると考えられます。
Line range hint
1-1000: 全体的に検索機能が大幅に改善されています。このファイルの変更により、ノート、リアクション、投票、お気に入りなど、さまざまなタイプのデータに対する検索機能が強化されています。新しいインデックスとメソッドの追加により、より柔軟で包括的な検索が可能になっています。
今後の改善のために、以下の点を検討することをお勧めします：
パフォーマンス最適化：
- 大規模データセットに対するクエリのパフォーマンスを継続的にモニタリングし、必要に応じてインデックスを追加または調整してください。
- 非同期処理の並列化をさらに活用し、全体的な応答時間を改善してください。
エラーハンドリングの強化：
- 各メソッドでのエラーハンドリングを統一し、エラーログの詳細度を向上させてください。
- 重要な操作に対してはリトライメカニズムの実装を検討してください。
コードの分割：
- ファイルが大きくなっているため、関連する機能ごとにクラスを分割することを検討してください。例えば、
IndexingServiceとSearchServiceのように分けることで、保守性が向上する可能性があります。テストカバレッジの向上：
- 新しく追加された機能に対する単体テストと統合テストを追加し、コードの信頼性を確保してください。
これらの改善により、コードの品質、保守性、およびパフォーマンスがさらに向上すると考えられます。
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- CHANGELOG_YOJO.md (1 hunks)
- packages/backend/src/core/AdvancedSearchService.ts (14 hunks)
- packages/backend/src/core/ClipService.ts (5 hunks)
- packages/backend/src/core/NoteCreateService.ts (2 hunks)
- packages/backend/src/server/api/endpoints/notes/favorites/create.ts (3 hunks)
- packages/backend/src/server/api/endpoints/notes/favorites/delete.ts (3 hunks)
- packages/backend/test/e2e/search-notes.ts (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- packages/backend/src/core/ClipService.ts
- packages/backend/src/core/NoteCreateService.ts
- packages/backend/src/server/api/endpoints/notes/favorites/create.ts
- packages/backend/src/server/api/endpoints/notes/favorites/delete.ts
- packages/backend/test/e2e/search-notes.ts
Additional context used
Markdownlint
CHANGELOG_YOJO.md
18-18: Expected: 1; Actual: 4
Inconsistent indentation for list items at the same level(MD005, list-indent)
19-19: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
20-20: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
21-21: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
22-22: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
23-23: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
17-17: Expected: 4; Actual: 1
Unordered list indentation(MD007, ul-indent)
19-19: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
20-20: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
21-21: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
22-22: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
23-23: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
17-17: Column: 1
Hard tabs(MD010, no-hard-tabs)
19-19: Column: 3
Hard tabs(MD010, no-hard-tabs)
20-20: Column: 3
Hard tabs(MD010, no-hard-tabs)
21-21: Column: 3
Hard tabs(MD010, no-hard-tabs)
22-22: Column: 3
Hard tabs(MD010, no-hard-tabs)
23-23: Column: 3
Hard tabs(MD010, no-hard-tabs)
Additional comments not posted (1)
CHANGELOG_YOJO.md (1)
7-23: 新機能の追加についての説明が明確です。インデックス構造の変更と Mastodon の indexable 機能の追加について、十分な情報が提供されています。ユーザーにとって重要な以下の点が明確に説明されています：
- インデックスの再構築が必要であること。
- リアクションと投票のインデックス化に関する変更。
- Mastodon の indexable 機能の詳細と、それがユーザーの検索可視性にどのように影響するか。
これらの説明は、ユーザーや管理者が新しい変更を理解し、必要なアクションを取るのに役立ちます。
Tools
Markdownlint
18-18: Expected: 1; Actual: 4
Inconsistent indentation for list items at the same level(MD005, list-indent)
19-19: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
20-20: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
21-21: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
22-22: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
23-23: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
17-17: Expected: 4; Actual: 1
Unordered list indentation(MD007, ul-indent)
19-19: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
20-20: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
21-21: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
22-22: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
23-23: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
17-17: Column: 1
Hard tabs(MD010, no-hard-tabs)
19-19: Column: 3
Hard tabs(MD010, no-hard-tabs)
20-20: Column: 3
Hard tabs(MD010, no-hard-tabs)
21-21: Column: 3
Hard tabs(MD010, no-hard-tabs)
22-22: Column: 3
Hard tabs(MD010, no-hard-tabs)
23-23: Column: 3
Hard tabs(MD010, no-hard-tabs)
CHANGELOG_YOJO.md
Outdated
| インデックス構造が変わったため破棄して再インデックスが必要です。 | ||
| リアクション、投票が途中からインデックスにされるので再インデックスをおすすめします。 | ||
|
|
||
| - Enhance: 連合一覧のソートにリバーシのバージョンを追加 | ||
| - Enhance: リモートのクリップをお気に入りに登録できるように | ||
| - Enhance: リモートのPlayを遊べるように | ||
| - Enhance: リモートのPlayをお気に入りに登録できるように | ||
| - Feat: mastodonのindexableに対応 | ||
| - 検索で表示される条件を制限できるようになります | ||
| - 設定→プライバシーより設定できます | ||
| - 設定されている場合対応しているサーバーでは、以下のことをしたユーザーのみ検索できます | ||
| - リアクション | ||
| - リノート | ||
| - クリップ | ||
| - お気に入り | ||
| - 返信 | ||
| - 投票 |
There was a problem hiding this comment.
フォーマットの一貫性と読みやすさを改善してください。
新しいエントリーの内容は良いですが、フォーマットに一貫性がありません。以下の修正を提案します：
- インデントを統一する。
- 箇条書きの記号を統一する（
-を使用）。 - Mastodonの indexable 機能の説明を改善する。
以下の修正を提案します：
## x.x.x
Cherrypick 4.11.1
### Release Date
### General
-インデックス構造が変わったため破棄して再インデックスが必要です。
-リアクション、投票が途中からインデックスにされるので再インデックスをおすすめします。
+- インデックス構造が変わったため破棄して再インデックスが必要です。
+- リアクション、投票が途中からインデックスにされるので再インデックスをおすすめします。
- Enhance: 連合一覧のソートにリバーシのバージョンを追加
- Enhance: リモートのクリップをお気に入りに登録できるように
- Enhance: リモートのPlayを遊べるように
- Enhance: リモートのPlayをお気に入りに登録できるように
-Feat: mastodonのindexableに対応
- - 検索で表示される条件を制限できるようになります
- - 設定→プライバシーより設定できます
- - 設定されている場合対応しているサーバーでは、以下のことをしたユーザーのみ検索できます
- - リアクション
- - リノート
- - クリップ
- - お気に入り
- - 返信
- - 投票
+- Feat: Mastodonのindexableに対応
+ - 検索で表示される条件を制限できるようになります
+ - 設定→プライバシーより設定できます
+ - 設定されている場合、対応しているサーバーでは以下のアクションを行ったユーザーのみ検索できます：
+ - リアクション
+ - リノート
+ - クリップ
+ - お気に入り
+ - 返信
+ - 投票これらの変更により、ドキュメントの一貫性と読みやすさが向上します。
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| インデックス構造が変わったため破棄して再インデックスが必要です。 | |
| リアクション、投票が途中からインデックスにされるので再インデックスをおすすめします。 | |
| - Enhance: 連合一覧のソートにリバーシのバージョンを追加 | |
| - Enhance: リモートのクリップをお気に入りに登録できるように | |
| - Enhance: リモートのPlayを遊べるように | |
| - Enhance: リモートのPlayをお気に入りに登録できるように | |
| - Feat: mastodonのindexableに対応 | |
| - 検索で表示される条件を制限できるようになります | |
| - 設定→プライバシーより設定できます | |
| - 設定されている場合対応しているサーバーでは、以下のことをしたユーザーのみ検索できます | |
| - リアクション | |
| - リノート | |
| - クリップ | |
| - お気に入り | |
| - 返信 | |
| - 投票 | |
| ## x.x.x | |
| Cherrypick 4.11.1 | |
| ### Release Date | |
| ### General | |
| - インデックス構造が変わったため破棄して再インデックスが必要です。 | |
| - リアクション、投票が途中からインデックスにされるので再インデックスをおすすめします。 | |
| - Enhance: 連合一覧のソートにリバーシのバージョンを追加 | |
| - Enhance: リモートのクリップをお気に入りに登録できるように | |
| - Enhance: リモートのPlayを遊べるように | |
| - Enhance: リモートのPlayをお気に入りに登録できるように | |
| - Feat: Mastodonのindexableに対応 | |
| - 検索で表示される条件を制限できるようになります | |
| - 設定→プライバシーより設定できます | |
| - 設定されている場合、対応しているサーバーでは以下のアクションを行ったユーザーのみ検索できます： | |
| - リアクション | |
| - リノート | |
| - クリップ | |
| - お気に入り | |
| - 返信 | |
| - 投票 |
Tools
Markdownlint
18-18: Expected: 1; Actual: 4
Inconsistent indentation for list items at the same level(MD005, list-indent)
19-19: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
20-20: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
21-21: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
22-22: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
23-23: Expected: 1; Actual: 3
Inconsistent indentation for list items at the same level(MD005, list-indent)
17-17: Expected: 4; Actual: 1
Unordered list indentation(MD007, ul-indent)
19-19: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
20-20: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
21-21: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
22-22: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
23-23: Expected: 4; Actual: 3
Unordered list indentation(MD007, ul-indent)
17-17: Column: 1
Hard tabs(MD010, no-hard-tabs)
19-19: Column: 3
Hard tabs(MD010, no-hard-tabs)
20-20: Column: 3
Hard tabs(MD010, no-hard-tabs)
21-21: Column: 3
Hard tabs(MD010, no-hard-tabs)
22-22: Column: 3
Hard tabs(MD010, no-hard-tabs)
23-23: Column: 3
Hard tabs(MD010, no-hard-tabs)
| public async indexNote(note: MiNote, choices?: string[]): Promise<void> { | ||
| if (!this.opensearch) return; | ||
| if (note.text == null && note.cw == null) { | ||
| await this.index(this.renoteIndex, note.id, { | ||
| renoteId: note.renoteId, | ||
| userId: note.userId, | ||
| userHost: note.userHost, | ||
| createdAt: this.idService.parse(note.id).date.getTime(), | ||
| tags: note.tags, | ||
| fileIds: note.fileIds, | ||
| visibility: note.visibility, | ||
| visibleUserIds: note.visibleUserIds, | ||
| isReply: IsReply, | ||
| isQuote: IsQuote, | ||
| referenceUserId: IsReply ? note.replyUserId : IsQuote ? note.renoteUserId : null, | ||
| sensitiveFileCount: sensitiveCount, | ||
| nonSensitiveFileCount: nonSensitiveCount, | ||
| }; | ||
| await this.opensearch.index({ | ||
| index: this.opensearchNoteIndex as string, | ||
| id: note.id, | ||
| body: body, | ||
| }).catch((error) => { | ||
| this.logger.error(error); | ||
| }); | ||
| return; | ||
| } | ||
| if (await this.redisClient.get('indexDeleted') !== null) { | ||
| return; | ||
| } | ||
| const IsQuote = isRenote(note) && isQuote(note); | ||
| const sensitiveCount = await this.driveService.getSensitiveFileCount(note.fileIds); | ||
| const nonSensitiveCount = note.fileIds.length - sensitiveCount; | ||
|
|
||
| const body = { | ||
| text: note.text, | ||
| cw: note.cw, | ||
| userId: note.userId, | ||
| userHost: note.userHost, | ||
| createdAt: this.idService.parse(note.id).date.getTime(), | ||
| tags: note.tags, | ||
| fileIds: note.fileIds, | ||
| visibility: note.visibility, | ||
| visibleUserIds: note.visibleUserIds, | ||
| replyId: note.replyId, | ||
| renoteId: note.renoteId, | ||
| pollChoices: choices, | ||
| referenceUserId: note.replyId ? note.replyUserId : IsQuote ? note.renoteUserId : null, | ||
| sensitiveFileCount: sensitiveCount, | ||
| nonSensitiveFileCount: nonSensitiveCount, | ||
| }; | ||
| this.index(this.opensearchNoteIndex as string, note.id, body); |
There was a problem hiding this comment.
indexNoteとunindexNoteメソッドの改善が適切に行われています。
リノートの処理が改善され、関連するエントリーの削除が他のインデックスにも適用されるようになりました。これにより、データの一貫性が向上しています。
ただし、fullIndexNoteメソッドに潜在的な問題があります：
fullIndexNoteメソッド内で、投票を含むノートの処理に非同期の問題がある可能性があります。以下のように修正することを検討してください：
-if (note.hasPoll) {
- this.pollsRepository.findOneBy({ noteId: note.id }).then((poll) => {
- this.indexNote(note, poll ? poll.choices : undefined);
- });
-} else {
- this.indexNote(note, undefined);
-}
+if (note.hasPoll) {
+ const poll = await this.pollsRepository.findOneBy({ noteId: note.id });
+ await this.indexNote(note, poll ? poll.choices : undefined);
+} else {
+ await this.indexNote(note, undefined);
+}この変更により、投票を含むノートのインデックス作成が確実に完了してから次のノートの処理に移ることができます。
Also applies to: 516-559
What
Why
Additional info (optional)
Checklist
Summary by CodeRabbit