Optimize: admin tools #15
Conversation
- add `features` for UserApi - add `reset password` and `Features` for user info update
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1496065092
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (typeof value !== 'string' || value.length < 8) { | ||
| return HttpErrorHandler.unprocessableEntity(req, res, 'Password must be at least 8 characters long') | ||
| } |
There was a problem hiding this comment.
This branch accepts any non-empty string of length >= 8 and immediately hashes it, which bypasses the centralized password validation logic used elsewhere (e.g., max length/character policy and other AuthenticationManager.validatePassword checks). In deployments with stricter passwordStrengthOptions, an admin update here can persist passwords that violate configured policy (including bcrypt-truncated overlong passwords), creating inconsistent and weaker credential handling.
Useful? React with 👍 / 👎.
| const numberValue = parseInt(value, 10) | ||
| if (!isNaN(numberValue)) { | ||
| setUserData(prev => ({ |
There was a problem hiding this comment.
handleFeatureNumChange only updates state when parseInt succeeds, but this is a controlled number input: typing the first - for -1 produces NaN, so state is not updated and the field snaps back, making the advertised unlimited value hard/impossible to type via keyboard. The same condition also prevents clearing an existing numeric value because an empty input is ignored.
Useful? React with 👍 / 👎.
- add search router and frontend function - limit frontend users to 1000(normal)+1000(del) - fix i18n translations
- add search router and frontend function for project search - limit frontend projects to 1000(normal)+1000(del)
|
@codex, please check my new feature, remote search. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6a6eb4824
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| } | ||
|
|
||
| let users = searchResults !== null ? searchResults : loadedUsers |
There was a problem hiding this comment.
Switching processedUsers to searchResults means the table can now show users that are not in loadedUsers (the initial list is capped to 1000), but selection/bulk actions still rely on selectedUsers built from loadedUsers; in this state, selecting a remotely found user does not persist in the UI and bulk tools receive an empty selection. This breaks admin actions for users returned only by /admin/users/search.
Useful? React with 👍 / 👎.
| searchUsers(searchText) | ||
| .then(data => { | ||
| setSearchResults(data.users) |
There was a problem hiding this comment.
The debounced effect writes setSearchResults(data.users) for every completed request without checking whether that response matches the latest query, so a slower response for an older term can overwrite newer results when typing quickly. This causes stale/incorrect user lists and can lead admins to act on the wrong result set unless requests are cancelled or versioned.
Useful? React with 👍 / 👎.
The debounced effect writes setSearchResults(data.users) for every completed request without checking whether that response matches the latest query, so a slower response for an older term can overwrite newer results when typing quickly. This causes stale/incorrect user lists and can lead admins to act on the wrong result set unless requests are cancelled or versioned.
|
@codex, what about now |
Summary
Testing
Committed on branch
I also created a follow-up PR via the
|
|
@copilot What about now? |
|
@Musicminion I've opened a new pull request, #16, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@codex, I need you to review the code, I think there is no need to add subversion for even a search feature |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1fb661966
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| const activeUsers = await UserGetter.promises.getUsers({}, projection) | ||
| const activeUsers = await User.find({}, projection) | ||
| .limit(1000) |
There was a problem hiding this comment.
Applying .limit(1000) at query time truncates the admin dataset before _applyFilters/sorting run, so instances with more than 1000 users will silently hide accounts and under-report totalSize; this makes some users impossible to reach through normal list/filter workflows and breaks bulk-admin expectations in larger deployments.
Useful? React with 👍 / 👎.
| userId == null ? {} : { owner_ref: userId }, | ||
| projection, | ||
| ).lean().exec() | ||
| ).limit(1000).lean().exec() |
There was a problem hiding this comment.
This cap is applied before project filters and sorting, so when there are more than 1000 projects the list only reflects an arbitrary prefix of records; admins can miss matching deleted/trashed/inactive projects and totalSize no longer represents the real result set.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
No, filter in backend is deprecated.
| ) | ||
| } | ||
|
|
||
| let users = searchResults !== null ? searchResults : loadedUsers |
There was a problem hiding this comment.
processedUsers now prioritizes searchResults, but mutation callbacks in this context still update loadedUsers; during an active search, update/delete/restore actions can succeed server-side while the table remains stale because rendering ignores the mutated source, leading to repeated actions against already-changed users until the query is rerun.
Useful? React with 👍 / 👎.
Description
I test with 8000 users, since overleaf-cep get all users and projects in one time, the browser get stuck immediately. We need to optimize this.
Related issues / Pull Requests
Contributor Agreement