Refine admin navigation and user group list#9
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48c4c92b3c
ℹ️ 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".
| for _, userID := range uniqueIDs(userIDs) { | ||
| session, ok := sessionByUser[userID] | ||
| if ok { | ||
| if err := s.store.DeleteExamSession(ctx, session.ID); err != nil { | ||
| return err |
There was a problem hiding this comment.
Preserve sessions still covered by other assignments
When removing a single assignment, this loop deletes every matching exam_sessions row without checking whether the same user is still assigned through another direct assignment or another user group. In that case, removing one assignment silently revokes access for users who should remain eligible, because their pre-created session is removed even though another assignment still exists for the same exam.
Useful? React with 👍 / 👎.
| if err := tx.Where("user_group_id = ?", id).Delete(&database.UserGroupMemberModel{}).Error; err != nil { | ||
| return err | ||
| } | ||
| return tx.Delete(&database.UserGroupModel{}, id).Error |
There was a problem hiding this comment.
Delete group-linked exam assignments with user group cleanup
Deleting a user group only removes memberships and the group row, but leaves exam_assignments entries that target that group. Because those rows keep referencing a now-missing group, later assignment operations can break (for example, removing that assignment resolves target users via ListUserGroupStudentIDs and will fail with user-group-not-found), leaving dangling assignment data in production.
Useful? React with 👍 / 👎.
| if roleFilter != "" || hasExcludeGroup { | ||
| queryPage = 1 | ||
| querySize = 10000 | ||
| } |
There was a problem hiding this comment.
Avoid truncating filtered user lists at 10,000 rows
For role or exclude_user_group_id filters, the handler forces a single fetch capped at 10,000 users and then applies filtering/pagination in memory. Any matching users beyond that cap are never returned and total becomes incorrect, so admins in larger deployments cannot page to or assign users outside the first 10,000 records.
Useful? React with 👍 / 👎.
Summary
Validation