Closed ras0q closed 1 month ago
Here are some key observations to aid the review process:
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪ |
🧪 No relevant tests |
🔒 No security concerns identified |
⚡ Recommended focus areas for review Hardcoded Value `includeSuspended` パラメータが常に `true` に設定されています。これにより、APIが常に停止されたユーザーを含めるようになります。この値を動的に設定できるようにするか、設定可能なオプションを提供することを検討してください。 |
Explore these optional code suggestions:
Category | Suggestion | Score |
Enhancement |
関数の柔軟性とテスト容易性を向上させるために、ハードコードされた変数を引数に置き換えます。___ **includeSuspended 変数を fetchUsers 関数の引数として渡すことを検討してください。これにより、関数の再利用性とテストのしやすさが向上します。**
[src/store/user.ts [14-15]](https://github.com/traPtitech/traPortfolio-Dashboard/pull/371/files#diff-588beb468db11465e8542876bbd8c2126fafe1ec770cfe5d90a3827b8175b356R14-R15)
```diff
-const includeSuspended = true
const res = await apis.getUsers(includeSuspended)
```
Suggestion importance[1-10]: 8Why: The suggestion to pass `includeSuspended` as an argument to the `fetchUsers` function enhances the function's reusability and testability by allowing different values to be passed in different contexts. This change improves the flexibility of the code without altering its current functionality. | 8 |
apiのレスポンスには凍結情報が含まれてないのでそこは注意ですね 現役だけの取得が必要になれば適宜includeSuspendedを変更できるようにすると良さそうです
User description
fix #339
PR Type
bug_fix
Description
getUsers
API 呼び出しにincludeSuspended
パラメータを追加しました。Changes walkthrough 📝
user.ts
Include suspended users in user fetch API call
src/store/user.ts
includeSuspended
to thegetUsers
API call.