feat: add usage leaderboard with podium UI and daily check-in ranking - #6524
feat: add usage leaderboard with podium UI and daily check-in ranking#6524HongShi2333 wants to merge 4 commits into
Conversation
Co-authored-by: traeagent <traeagent@users.noreply.github.com>
Co-authored-by: traeagent <traeagent@users.noreply.github.com>
Co-authored-by: traeagent <traeagent@users.noreply.github.com>
Reuses the global `animate-appear` CSS utility (which already honors prefers-reduced-motion) with position-keyed delays so the silver → gold → bronze blocks cascade in on first paint. Zero new CSS, zero runtime cost — the animation fires once on mount. Also drops a redundant ternary in the medal label.
WalkthroughThis PR adds invitation-code registration and administration, canonical OAuth identity storage and migration, usage/check-in leaderboards, updated authentication flows, SQLite concurrency handling, API documentation, localized UI text, and frontend pages/routes for invitation codes and usage rankings. ChangesInvitation Registration and Administration
Canonical Auth Identity
Usage Leaderboards
Sequence Diagram(s)sequenceDiagram
participant Browser
participant RegistrationAPI
participant SettingsStore
participant InvitationCode
participant UserStore
Browser->>RegistrationAPI: submit registration with invitation_code
RegistrationAPI->>SettingsStore: read locked invitation settings
RegistrationAPI->>UserStore: create user in transaction
RegistrationAPI->>InvitationCode: consume invitation reference
InvitationCode-->>RegistrationAPI: used invitation state
RegistrationAPI-->>Browser: registration result
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
controller/custom_oauth.go (2)
214-220: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winWhitespace-only slug bypasses
requiredvalidation after normalization.
binding:"required"only rejects the empty string, so a slug of" "passes validation, thenNormalizeCustomOAuthProviderSlugtrims it to""before the uniqueness/conflict checks and persistence — silently creating a provider with an empty slug.🐛 Proposed fix
req.Slug = model.NormalizeCustomOAuthProviderSlug(req.Slug) + if req.Slug == "" { + common.ApiErrorMsg(c, "Slug 不能为空") + return + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/custom_oauth.go` around lines 214 - 220, Update CreateCustomOAuthProvider so the slug is normalized before request validation, ensuring whitespace-only values become empty and are rejected by the existing required validation. Preserve the current error response and continue using the normalized slug for conflict checks and persistence.
293-336: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve existing custom OAuth provider slug casing until a backfill runs.
UpdateCustomOAuthProvidernormalizesreq.Slugimmediately, so resubmitting a current mixed-case slug now triggersreq.Slug != provider.Slugand saves the lowercase slug without existing rows being migrated. Either include a backfill that case-normalizescustom_oauth_providers.slug, or skip updatingprovider.Slugwhen the normalized slug is unchanged but the existing slug casing should remain untouched.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/custom_oauth.go` around lines 293 - 336, Update UpdateCustomOAuthProvider to preserve the existing provider.Slug casing when the normalized request slug differs only by case, avoiding an implicit lowercase migration; only assign provider.Slug when the requested slug represents an actual slug change, or add and run an explicit backfill for existing custom_oauth_providers rows before normalization.model/option.go.rej (1)
1-338: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDelete this
.rejartifact — it should not be committed.
model/option.go.rejis a rejected-hunk file left over from a failed patch application. Its contents are already present (in a newer form) inmodel/option.go, so keeping it only adds a stale, misleading copy of the invitation-settings logic to the repo. Consider adding*.rej/*.origto.gitignore.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/option.go.rej` around lines 1 - 338, Delete the model/option.go.rej rejected-patch artifact from the repository; its invitation-settings logic is already represented by the corresponding symbols in model/option.go. Ensure no .rej or .orig patch artifacts remain committed, and optionally add matching patterns to .gitignore to prevent recurrence.
🧹 Nitpick comments (20)
model/auth_identity.go (2)
54-60: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueSubject is validated trimmed but stored untrimmed.
normalizeAuthIdentityrejects whitespace-only subjects and length-checks the raw value, yet returnsproviderSubjectunmodified, so" abc"and"abc"hash to different identities. Trim for consistency withisMalformedLegacyAuthIdentityInputinmodel/auth_identity_migration.goLines 219-226.♻️ Proposed normalization
func normalizeAuthIdentity(providerKey string, providerSubject string) (string, string, error) { providerKey = strings.ToLower(strings.TrimSpace(providerKey)) - if providerKey == "" || strings.TrimSpace(providerSubject) == "" || len(providerKey) > 64 || len(providerSubject) > 256 { + providerSubject = strings.TrimSpace(providerSubject) + if providerKey == "" || providerSubject == "" || len(providerKey) > 64 || len(providerSubject) > 256 { return "", "", errors.New("OAuth identity provider and subject are required") } return providerKey, providerSubject, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/auth_identity.go` around lines 54 - 60, Update normalizeAuthIdentity to trim whitespace from providerSubject before validating its emptiness and length, then return the trimmed value so normalized identities are stored and hashed consistently. Keep the existing providerKey normalization and validation behavior unchanged.
162-176: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueScope the user lookup or document the soft-delete contract.
GetUserByAuthIdentityreturns soft-deleted users because it callsDB.Unscoped().First(...)for the owner. Existing login flows checkDeletedAt.Valid, but the public test intentionally exposes this behavior, andGetUserByOAuthBindingreturns it unchanged, which can affect generic OIDC login paths that rely on this resolver.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/auth_identity.go` around lines 162 - 176, Update GetUserByAuthIdentity to avoid returning soft-deleted users by removing Unscoped from the owner lookup, or otherwise enforce the existing DeletedAt contract before returning the user. Preserve the identity lookup and active-user behavior, and ensure callers such as GetUserByOAuthBinding cannot receive deleted users through this resolver.model/main.go.rej (1)
1-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove leftover patch-reject artifact.
This
.rejfile is generated when a patch fails to apply cleanly and should not be committed. The corresponding change already appears to have landed inmodel/main.go, so this file is stray debris.🧹 Suggested fix
Delete
model/main.go.rejfrom the repository.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/main.go.rej` around lines 1 - 20, Delete the stray model/main.go.rej patch-reject artifact from the repository; the corresponding normalizeSQLiteDSN change is already present in model/main.go.model/invitation_code_test.go (1)
17-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer GORM deletes over raw
DELETE FROM users.The invitation-code cleanup already uses the GORM API; the user cleanup drops to raw SQL for no dialect-specific reason. Using
DB.Session(&gorm.Session{AllowGlobalUpdate: true}).Unscoped().Delete(&User{})keeps it portable and consistent.As per coding guidelines: "Prefer GORM methods over raw SQL".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/invitation_code_test.go` around lines 17 - 29, Replace both raw “DELETE FROM users” calls in the invitation-code test setup and cleanup with the GORM Unscoped Delete pattern using DB.Session, AllowGlobalUpdate, and Delete(&User{}), matching the existing InvitationCode cleanup.Source: Coding guidelines
controller/custom_oauth_test.go (1)
126-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider asserting on stable identifiers instead of localized message substrings.
assert.Contains(t, response.Message, "冲突")(and"已被使用"at Line 162, Line 197) couples these tests to Chinese UI copy; any wording/i18n change breaks them. If the handler exposes i18n message keys or sentinel errors, prefer those (ascontroller/option_invitation_test.godoes withi18n.Translate).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/custom_oauth_test.go` around lines 126 - 135, Replace localized substring assertions in the custom OAuth tests, including the checks near the conflict cases and the “已被使用” cases, with assertions against stable i18n message keys or sentinel errors exposed by the handler. Follow the existing pattern in option_invitation_test.go using i18n.Translate, while preserving the current failure and database/provider state assertions.controller/user_leaderboard.go (1)
19-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffAll service errors are reported as HTTP 400, even likely infra failures.
service.GetUsageLeaderboard/GetCheckinLeaderboarderrors (e.g. wrapped"aggregate usage logs: %w"DB failures from the model layer) are mapped tohttp.StatusBadRequestalongside genuine validation errors like"invalid time range". Client-caused vs server-caused failures should generally map to different status codes for correct monitoring/alerting semantics.Also applies to: 41-47
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/user_leaderboard.go` around lines 19 - 25, Update the error handling around GetUsageLeaderboard and GetCheckinLeaderboard so validation errors such as “invalid time range” remain HTTP 400, while service or infrastructure failures such as wrapped database errors return an appropriate 5xx status. Use the existing error types or classification mechanism to distinguish client-caused errors from internal failures, and preserve the current error response structure.model/user_leaderboard.go (1)
73-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate "load users + filter admin + rank" logic across both functions.
The user-lookup, admin-filter, and ranking block is duplicated almost verbatim between
GetUsageLeaderboardandGetCheckinLeaderboard. Consider extracting a shared helper (e.g.,loadEligibleUsers(ids []int) (map[int]userInfo, error)plus a small ranking helper) — it has two call sites, which satisfies the "package-level helper" exception for shared business logic.Also applies to: 154-192
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/user_leaderboard.go` around lines 73 - 112, Extract the duplicated user lookup and admin-filtering logic from GetUsageLeaderboard and GetCheckinLeaderboard into a shared package-level helper such as loadEligibleUsers, preserving the existing database error context and Role filtering. Reuse the helper at both call sites and centralize the repeated rank/result assembly in a small shared helper where practical, while preserving each leaderboard’s existing output and limit behavior.web/scripts/gen-routes.mjs (1)
22-39: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPin
@tanstack/router-generatorbefore using this on CI/reproducible builds.
web/scripts/gen-routes.mjsimportsGeneratordirectly from@tanstack/router-generator. Even if that is a documented entry point, the generated route tree is not version-locked for this script, so installing a newerlatestversion can silently change the route-tree output across environments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/scripts/gen-routes.mjs` around lines 22 - 39, Pin the `@tanstack/router-generator` dependency used by the Generator import to an explicit, reproducible version in the project’s dependency configuration. Ensure CI and local installs resolve the same version while leaving the route-generation configuration and generator invocation unchanged.web/src/features/usage/hooks/use-usage-leaderboard.ts (1)
29-45: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider adding an
enabledoption (see companion comment onweb/src/features/usage/index.tsx).The hooks always fetch on mount; the call site currently invokes both hooks unconditionally regardless of the active tab. See the paired comment in
index.tsxfor the fix that needs this hook to acceptenabled.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/features/usage/hooks/use-usage-leaderboard.ts` around lines 29 - 45, The useUsageLeaderboard and useCheckinLeaderboard hooks always fetch regardless of the active tab. Add an enabled option to each hook and pass it through to its useQuery configuration, preserving the default enabled behavior when callers omit it so the index.tsx call site can disable inactive-tab queries.model/invitation_code.go (1)
137-142: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse of the same query builder across
CountandFind.
queryis reused after a finisher (Count), which relies on GORM's condition-reuse behavior and can silently accumulate clauses if this code grows. Snapshotting the conditions makes the intent explicit.♻️ Suggested change
- if err = query.Count(&total).Error; err != nil { + query = query.Session(&gorm.Session{}) + if err = query.Count(&total).Error; err != nil { return nil, 0, err }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/invitation_code.go` around lines 137 - 142, Update the query flow around Count and Find to snapshot or clone the query conditions before the Count finisher, then use that preserved query for the paginated Find. Keep the existing count, ordering, limit, offset, and error behavior unchanged while avoiding reuse of the same builder after Count.service/registration_test.go (1)
306-349: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest can hang instead of failing, and the 50 ms window is a weak assertion.
registrationEnteredis only closed from insideCreateRelated, which never runs ifRegisterNewUserfails before admission (e.g., settings load error). In that case Line 324 blocks until the package-levelgo testtimeout rather than reporting a failure. Guarding the handshake with a timeout and surfacing the registration error keeps the failure mode diagnosable.The
time.After(50 * time.Millisecond)check can also silently pass on a loaded runner without proving the update actually blocked; consider asserting the ordering via observable state (e.g., the update result relative to the registration commit) rather than a wall-clock gap.♻️ Suggested hardening of the handshake
- <-registrationEntered + select { + case <-registrationEntered: + case err := <-registrationDone: + t.Fatalf("registration finished before entering the admitted window: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("registration never reached the admitted window") + }As per coding guidelines: "prefer deterministic table tests with explicit inputs and exact outputs, and avoid coverage-only, fake stress, timing, or implementation-detail tests."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@service/registration_test.go` around lines 306 - 349, Harden TestInvitationSettingsUpdateWaitsForRegistrationAdmittedUnderPreviousSnapshot so registrationEntered cannot block indefinitely: report RegisterNewUser errors through the existing registrationDone channel and await it with a bounded timeout before proceeding. Replace the time.After(50*time.Millisecond) assertion with deterministic synchronization that verifies updateDone remains pending until releaseRegistration is closed, then assert registration and update completion ordering through their results.Source: Coding guidelines
service/registration.go (1)
49-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
baseUseris vestigial.It's only used to seed
attemptUserand never re-read (there's no retry loop that resets state), so the two-variable dance just obscures intent.♻️ Proposed simplification
- baseUser := *registration.User - attemptUser := baseUser + attemptUser := *registration.User🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@service/registration.go` around lines 49 - 50, Remove the vestigial baseUser variable in the registration flow and initialize attemptUser directly from registration.User. Keep the existing value-copy semantics and all subsequent attemptUser behavior unchanged.model/sqlite_dsn_test.go (1)
30-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate assertion.
Line 33 repeats the
_txlockcheck from line 31.♻️ Proposed cleanup
assert.Equal(t, "shared", query.Get("cache")) assert.Equal(t, "immediate", query.Get("_txlock")) assert.NotContains(t, query, "_busy_timeout") - assert.Equal(t, "immediate", query.Get("_txlock")) assert.ElementsMatch(t, []string{🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/sqlite_dsn_test.go` around lines 30 - 37, Remove the redundant duplicate assertion for query.Get("_txlock") in the sqlite DSN test, preserving the original assertion and all other query validations.model/sqlite_retry.go (1)
17-33: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider gating on the active dialect to avoid cross-dialect false positives.
The structural
Code() intassertion plus substring matching runs for every dialect (this is called fromcontroller/option.goandservice/registration.go). Any wrapped error type exposingCode() intthat returns 5/6, or a message coincidentally containingdatabase is locked, would be reported to users as a transient "retry later". Guarding withcommon.UsingMainDatabase/the SQLite database type would keep the heuristic scoped to where it applies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/sqlite_retry.go` around lines 17 - 33, Update IsSQLiteBusyError to only apply its sqlite error-code and message heuristics when common.UsingMainDatabase identifies the active database as SQLite; return false for other dialects before evaluating codedError or error text, while preserving the existing busy/locked detection for SQLite.web/src/features/auth/lib/oauth-create-flow.test.ts (2)
163-173: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis block asserts a property of a literal defined two lines above.
It can't fail regardless of production behavior; either assert against the real Telegram login param builder or drop it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/features/auth/lib/oauth-create-flow.test.ts` around lines 163 - 173, Replace the self-referential literal check in “telegram and bind never carry invitation” with an assertion against the real Telegram login parameter builder, verifying its produced params omit invitation_code; otherwise remove this test block rather than testing a locally defined object.
34-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTests exercise a copy of the logic, not the production helpers.
buildCreateOAuthFlowBody/buildWeChatRequestre-implement the rules inweb/src/features/auth/api.ts(createOAuthFlow,wechatLoginByCode), so a regression there won't fail these tests. Drift already exists: production reads the affiliate code viagetAffiliateCode()while this copy takes it as an option. Extracting the pure body-building rules fromapi.tsinto an exported helper and importing it here would make these invariants actually enforceable without pulling in axios.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/features/auth/lib/oauth-create-flow.test.ts` around lines 34 - 73, Replace the test-local buildCreateOAuthFlowBody and buildWeChatRequest implementations with imports of exported pure helpers extracted from api.ts alongside createOAuthFlow and wechatLoginByCode. Ensure createOAuthFlow’s helper uses the production getAffiliateCode() behavior and preserves the existing login/bind, trimming, and invitation-code rules, while wechatLoginByCode reuses the extracted request-building helper without requiring axios.model/option_invitation_test.go (1)
164-176: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTiming-based negative assertion may flake on loaded CI.
Asserting "the update did not finish within 100 ms" proves the lock held, but a slow/stalled scheduler on the other side (the update never even reaching the DB) also passes, and a fast lock-timeout config could fail it. Since this only runs with
TEST_MYSQL_DSN/TEST_POSTGRES_DSN, impact is limited — consider making the wait configurable or asserting on the DB-visible pair ordering instead.As per coding guidelines: "avoid coverage-only, fake stress, timing, or implementation-detail tests."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/option_invitation_test.go` around lines 164 - 176, Replace the fixed 100 millisecond timing assertion in the goroutine-based UpdateInvitationCodeSettings test with a deterministic database-visible assertion of registration/update ordering. Verify that the update cannot complete before the admitted registration boundary is committed, while preserving the existing error reporting and database-specific test setup; avoid relying on scheduler timing or lock-duration assumptions.Source: Coding guidelines
controller/invitation_test.go (1)
68-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing controller test coverage for
UpdateInvitationCode.Tests cover
AddInvitationCodesandDeleteUsedInvitationCodes, but the branchiest new handler —UpdateInvitationCode(status-only vs. full update, and theErrInvitationCodeUsed→i18n.MsgInvitationUsedCannotUpdatemapping) — has no direct controller test here.GetInvitationCode/SearchInvitationCodes/GetAllInvitationCodesare also untested at this layer.Consider adding table-driven tests exercising: status-only update to
Disabled, full update (name/expiry) success, and update-attempt on an already-usedcode (expectsuccess:falsewith no state change), mirroring the model-level assertions inmodel/invitation_code_test.go'sTestSearchAndDeleteUsedInvitationCodes.
As per path instructions, "Backend tests must protect real behavior, API contracts, billing/accounting invariants, compatibility, or regression paths; prefer deterministic table tests with explicit inputs and exact outputs."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/invitation_test.go` around lines 68 - 161, Add deterministic controller-level table tests for UpdateInvitationCode covering status-only updates to Disabled, full name/expiry updates, and attempts to update an already-used code. Assert exact success responses, verify persisted state for successful cases, and confirm used-code updates return success:false with unchanged state while mapping ErrInvitationCodeUsed to i18n.MsgInvitationUsedCannotUpdate.Source: Path instructions
controller/telegram_registration_boundary_test.go (1)
93-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the existing package-level signing helpers instead of a third copy.
controller/telegram_test.goalready providessignedTelegramAuthorizationplussignTelegramAuthorization, and the same file overrides theidfield that way (e.g.disabledParams.Set("id", ...)then re-sign). Reusing them keeps a single source of truth if the data-check-string rules change.♻️ Suggested replacement
- query := signedTelegramLoginBoundaryQuery( - common.TelegramBotToken, - "987654321", - time.Now(), - ) + query := signedTelegramAuthorization(common.TelegramBotToken, time.Now()) + query.Set("id", "987654321") + signTelegramAuthorization(common.TelegramBotToken, query)Then drop
signedTelegramLoginBoundaryQueryand the now-unusedcrypto/*,encoding/hex,sort,strconvimports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/telegram_registration_boundary_test.go` around lines 93 - 113, Replace the duplicate signedTelegramLoginBoundaryQuery helper with the existing signedTelegramAuthorization and signTelegramAuthorization helpers from telegram_test.go. Build the boundary parameters through the shared helper, override the id field as needed, and re-sign using signTelegramAuthorization; then remove imports only used by the deleted helper, including crypto, encoding/hex, sort, and strconv.controller/user.go.rej (1)
1-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the committed reject artifact.
This is an unapplied patch, not source code. Delete it and apply any intended changes to
controller/user.go.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/user.go.rej` around lines 1 - 22, Delete the committed reject artifact `controller/user.go.rej`, then apply its intended import changes directly to `controller/user.go`: add the `constant` import in the normal import block and remove the obsolete `setting` import if still unused, preserving valid import ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/option_invitation_test.go`:
- Around line 258-274: Move the invitationControllerContext call out of the
goroutine and create each test context in the submission loop before launching
the goroutine. Capture the prepared ctx and recorder alongside the submission,
then have the goroutine wait on start and invoke UpdateInvitationCode using
those values, preserving the existing response handling and synchronization.
In `@controller/registration_matrix_test.go`:
- Line 32: Update the OptionMap snapshot and restoration logic in the affected
tests to clone the map while holding common.OptionMapRWMutex, following
setupInvitationOptionControllerTest. Restore the cloned contents under the same
mutex so in-place handler mutations cannot leak between tests, and remove the
manual key-specific workaround in
TestRootSetupBypassesInvitationAndDefaultToken.
In `@model/user_leaderboard.go`:
- Around line 51-112: Update GetUsageLeaderboard and GetCheckinLeaderboard to
exclude admin users in their aggregation queries via the users table, rather
than fetching a limit*2 heuristic and filtering afterward. Use the requested
limit directly, remove the post-query admin checks and obsolete rank/break
bookkeeping, and assign ranks from the remaining eligible results while
preserving ordering and missing-user handling.
In `@model/user_oauth_binding.go`:
- Around line 164-179: Update userOAuthBindingFromAuthIdentity to skip custom
identities with an empty ProviderSubject instead of returning an error, logging
the skipped legacy row with common.SysLog while preserving the existing
non-custom result. Also update GetUserOAuthBinding to check the helper’s ok
result and return an appropriate not-found/error outcome when the identity is
not custom rather than returning a nil binding with no error.
In `@service/user_leaderboard.go`:
- Around line 163-197: Validate the caller-provided date before constructing the
cache key, accepting only the expected YYYY-MM-DD format and rejecting invalid
values. Update the check-in cache insertion flow around checkinLeaderboardCache
to evict expired entries and enforce a bounded maximum capacity before adding a
new entry, while preserving existing cache-hit behavior and response generation.
In `@web/src/features/invitation-codes/components/invitation-code-actions.tsx`:
- Around line 84-102: Mark the decorative Ban, CheckCircle2, and Trash2 icon
components inside IconAction as aria-hidden="true", while keeping the accessible
names supplied by the wrapping button labels unchanged.
In `@web/src/features/invitation-codes/components/invitation-codes-table.tsx`:
- Around line 112-127: Clamp the current page to the updated totalPages whenever
query data changes, so deletions or status mutations cannot leave page beyond
the final page. Update the pagination state near the pageInfo/totalPages
calculations, preserving a minimum page of 1 and the existing range calculations
and navigation behavior.
In `@web/src/features/system-settings/auth/basic-auth-section.tsx`:
- Around line 113-124: Wrap the update loop in the form submission handler with
try/catch so rejections from updateOption.mutateAsync are contained. Preserve
the existing early return for unsuccessful results, and return from the catch
without resetting the form or setting save confirmation after a failed request.
In `@web/src/features/usage/components/podium.tsx`:
- Around line 197-201: Add aria-hidden="true" to the decorative rank numeral
container or span in the podium rank rendering near the rank expression,
matching the medal avatar behavior so assistive technologies do not announce the
rank twice.
- Around line 191-196: Replace the nested ternary in the podium rank label
rendering with a lookup map, mirroring the existing PLACE_STYLES pattern. Define
a clearly named camelCase mapping for ranks 1–3 to their translation keys or
labels, then use the rank value to select the result while preserving the
existing t('1st'), t('2nd'), and t('3rd') behavior.
In `@web/src/features/usage/index.tsx`:
- Around line 81-88: Update the error message logic in the usage component’s
<code>UsageError</code> rendering to route server errors through the existing
<code>handleServerError</code> helper instead of displaying
<code>error.message</code> directly. Preserve the fallback i18n message for
non-<code>Error</code> values and ensure the resulting user-facing text follows
the project’s localized server-error handling path.
- Around line 79-100: Replace the nested ternary in the board-rendering JSX with
a clear branch-based helper such as renderBoard, using early returns for
isLoading and error, then separate isCheckin and UsageBoard branches. Preserve
the existing error-message fallback and all board props while calling the helper
from the component render.
- Around line 45-46: Update the usage and check-in leaderboard hooks to accept
an optional enabled option, then in the component containing usageQuery and
checkinQuery pass enabled based on the active tab so only the visible
leaderboard query runs. Preserve default enabled behavior for other callers.
In `@web/src/i18n/locales/fr.json`:
- Around line 1772-1774: Align the French translations for the “Expiration time”
and “Expiration Time” keys in the locale file by using the same wording, such as
“Heure d'expiration,” while leaving the future-expiration message unchanged.
In `@web/src/i18n/locales/vi.json`:
- Around line 799-800: The Vietnamese translations in
web/src/i18n/locales/vi.json lines 799-800, 4085, and 4966-4967 use “đăng nhập”
instead of the intended “điểm danh” terminology; update the leaderboard and
reward entries at lines 799-800, the daily check-in reward at line 4085 to “phần
thưởng điểm danh hàng ngày”, and the daily check-in ranking entries at lines
4966-4967 to “xếp hạng điểm danh hàng ngày”.
In `@web/src/i18n/locales/zh-TW.json`:
- Line 4085: Update the zh-TW translation for the leaderboard description key
“See who is leading the platform by usage and daily check-in rewards.
Administrators are excluded.” so it conveys who leads on the platform in usage
and daily check-in rewards, rather than implying someone leads the platform
itself.
---
Outside diff comments:
In `@controller/custom_oauth.go`:
- Around line 214-220: Update CreateCustomOAuthProvider so the slug is
normalized before request validation, ensuring whitespace-only values become
empty and are rejected by the existing required validation. Preserve the current
error response and continue using the normalized slug for conflict checks and
persistence.
- Around line 293-336: Update UpdateCustomOAuthProvider to preserve the existing
provider.Slug casing when the normalized request slug differs only by case,
avoiding an implicit lowercase migration; only assign provider.Slug when the
requested slug represents an actual slug change, or add and run an explicit
backfill for existing custom_oauth_providers rows before normalization.
In `@model/option.go.rej`:
- Around line 1-338: Delete the model/option.go.rej rejected-patch artifact from
the repository; its invitation-settings logic is already represented by the
corresponding symbols in model/option.go. Ensure no .rej or .orig patch
artifacts remain committed, and optionally add matching patterns to .gitignore
to prevent recurrence.
---
Nitpick comments:
In `@controller/custom_oauth_test.go`:
- Around line 126-135: Replace localized substring assertions in the custom
OAuth tests, including the checks near the conflict cases and the “已被使用” cases,
with assertions against stable i18n message keys or sentinel errors exposed by
the handler. Follow the existing pattern in option_invitation_test.go using
i18n.Translate, while preserving the current failure and database/provider state
assertions.
In `@controller/invitation_test.go`:
- Around line 68-161: Add deterministic controller-level table tests for
UpdateInvitationCode covering status-only updates to Disabled, full name/expiry
updates, and attempts to update an already-used code. Assert exact success
responses, verify persisted state for successful cases, and confirm used-code
updates return success:false with unchanged state while mapping
ErrInvitationCodeUsed to i18n.MsgInvitationUsedCannotUpdate.
In `@controller/telegram_registration_boundary_test.go`:
- Around line 93-113: Replace the duplicate signedTelegramLoginBoundaryQuery
helper with the existing signedTelegramAuthorization and
signTelegramAuthorization helpers from telegram_test.go. Build the boundary
parameters through the shared helper, override the id field as needed, and
re-sign using signTelegramAuthorization; then remove imports only used by the
deleted helper, including crypto, encoding/hex, sort, and strconv.
In `@controller/user_leaderboard.go`:
- Around line 19-25: Update the error handling around GetUsageLeaderboard and
GetCheckinLeaderboard so validation errors such as “invalid time range” remain
HTTP 400, while service or infrastructure failures such as wrapped database
errors return an appropriate 5xx status. Use the existing error types or
classification mechanism to distinguish client-caused errors from internal
failures, and preserve the current error response structure.
In `@controller/user.go.rej`:
- Around line 1-22: Delete the committed reject artifact
`controller/user.go.rej`, then apply its intended import changes directly to
`controller/user.go`: add the `constant` import in the normal import block and
remove the obsolete `setting` import if still unused, preserving valid import
ordering.
In `@model/auth_identity.go`:
- Around line 54-60: Update normalizeAuthIdentity to trim whitespace from
providerSubject before validating its emptiness and length, then return the
trimmed value so normalized identities are stored and hashed consistently. Keep
the existing providerKey normalization and validation behavior unchanged.
- Around line 162-176: Update GetUserByAuthIdentity to avoid returning
soft-deleted users by removing Unscoped from the owner lookup, or otherwise
enforce the existing DeletedAt contract before returning the user. Preserve the
identity lookup and active-user behavior, and ensure callers such as
GetUserByOAuthBinding cannot receive deleted users through this resolver.
In `@model/invitation_code_test.go`:
- Around line 17-29: Replace both raw “DELETE FROM users” calls in the
invitation-code test setup and cleanup with the GORM Unscoped Delete pattern
using DB.Session, AllowGlobalUpdate, and Delete(&User{}), matching the existing
InvitationCode cleanup.
In `@model/invitation_code.go`:
- Around line 137-142: Update the query flow around Count and Find to snapshot
or clone the query conditions before the Count finisher, then use that preserved
query for the paginated Find. Keep the existing count, ordering, limit, offset,
and error behavior unchanged while avoiding reuse of the same builder after
Count.
In `@model/main.go.rej`:
- Around line 1-20: Delete the stray model/main.go.rej patch-reject artifact
from the repository; the corresponding normalizeSQLiteDSN change is already
present in model/main.go.
In `@model/option_invitation_test.go`:
- Around line 164-176: Replace the fixed 100 millisecond timing assertion in the
goroutine-based UpdateInvitationCodeSettings test with a deterministic
database-visible assertion of registration/update ordering. Verify that the
update cannot complete before the admitted registration boundary is committed,
while preserving the existing error reporting and database-specific test setup;
avoid relying on scheduler timing or lock-duration assumptions.
In `@model/sqlite_dsn_test.go`:
- Around line 30-37: Remove the redundant duplicate assertion for
query.Get("_txlock") in the sqlite DSN test, preserving the original assertion
and all other query validations.
In `@model/sqlite_retry.go`:
- Around line 17-33: Update IsSQLiteBusyError to only apply its sqlite
error-code and message heuristics when common.UsingMainDatabase identifies the
active database as SQLite; return false for other dialects before evaluating
codedError or error text, while preserving the existing busy/locked detection
for SQLite.
In `@model/user_leaderboard.go`:
- Around line 73-112: Extract the duplicated user lookup and admin-filtering
logic from GetUsageLeaderboard and GetCheckinLeaderboard into a shared
package-level helper such as loadEligibleUsers, preserving the existing database
error context and Role filtering. Reuse the helper at both call sites and
centralize the repeated rank/result assembly in a small shared helper where
practical, while preserving each leaderboard’s existing output and limit
behavior.
In `@service/registration_test.go`:
- Around line 306-349: Harden
TestInvitationSettingsUpdateWaitsForRegistrationAdmittedUnderPreviousSnapshot so
registrationEntered cannot block indefinitely: report RegisterNewUser errors
through the existing registrationDone channel and await it with a bounded
timeout before proceeding. Replace the time.After(50*time.Millisecond) assertion
with deterministic synchronization that verifies updateDone remains pending
until releaseRegistration is closed, then assert registration and update
completion ordering through their results.
In `@service/registration.go`:
- Around line 49-50: Remove the vestigial baseUser variable in the registration
flow and initialize attemptUser directly from registration.User. Keep the
existing value-copy semantics and all subsequent attemptUser behavior unchanged.
In `@web/scripts/gen-routes.mjs`:
- Around line 22-39: Pin the `@tanstack/router-generator` dependency used by the
Generator import to an explicit, reproducible version in the project’s
dependency configuration. Ensure CI and local installs resolve the same version
while leaving the route-generation configuration and generator invocation
unchanged.
In `@web/src/features/auth/lib/oauth-create-flow.test.ts`:
- Around line 163-173: Replace the self-referential literal check in “telegram
and bind never carry invitation” with an assertion against the real Telegram
login parameter builder, verifying its produced params omit invitation_code;
otherwise remove this test block rather than testing a locally defined object.
- Around line 34-73: Replace the test-local buildCreateOAuthFlowBody and
buildWeChatRequest implementations with imports of exported pure helpers
extracted from api.ts alongside createOAuthFlow and wechatLoginByCode. Ensure
createOAuthFlow’s helper uses the production getAffiliateCode() behavior and
preserves the existing login/bind, trimming, and invitation-code rules, while
wechatLoginByCode reuses the extracted request-building helper without requiring
axios.
In `@web/src/features/usage/hooks/use-usage-leaderboard.ts`:
- Around line 29-45: The useUsageLeaderboard and useCheckinLeaderboard hooks
always fetch regardless of the active tab. Add an enabled option to each hook
and pass it through to its useQuery configuration, preserving the default
enabled behavior when callers omit it so the index.tsx call site can disable
inactive-tab queries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2aa97c3e-a907-4b0a-a952-8443133316a0
📒 Files selected for processing (124)
common/constants.gocommon/database.gocommon/invitation.gocommon/invitation_test.gocontroller/audit.gocontroller/auth_flow_test.gocontroller/custom_oauth.gocontroller/custom_oauth_test.gocontroller/invitation.gocontroller/invitation_test.gocontroller/misc.gocontroller/oauth.gocontroller/oauth_invitation_test.gocontroller/option.gocontroller/option_invitation_test.gocontroller/registration_matrix_test.gocontroller/telegram.gocontroller/telegram_registration_boundary_test.gocontroller/telegram_test.gocontroller/user.gocontroller/user.go.rejcontroller/user_leaderboard.gocontroller/wechat.gocontroller/wechat_invitation_test.godocs/openapi/api.jsondto/registration.goi18n/keys.goi18n/locales/en.yamli18n/locales/zh-CN.yamli18n/locales/zh-TW.yamlmiddleware/audit.gomiddleware/header_nav.gomiddleware/logger.gomiddleware/logger_test.gomodel/auth_identity.gomodel/auth_identity_migration.gomodel/auth_identity_migration_test.gomodel/auth_identity_test.gomodel/custom_oauth_provider.gomodel/external_identity_claim.gomodel/external_identity_claim_test.gomodel/invitation_code.gomodel/invitation_code_concurrency_test.gomodel/invitation_code_test.gomodel/locking.gomodel/main.gomodel/main.go.rejmodel/option.gomodel/option.go.rejmodel/option_invitation_test.gomodel/sqlite_dsn_test.gomodel/sqlite_retry.gomodel/task_cas_test.gomodel/user.gomodel/user_authentication_test.gomodel/user_leaderboard.gomodel/user_oauth_binding.gorouter/api-router.gorouter/invitation_permission_test.goservice/registration.goservice/registration_test.goservice/user_leaderboard.goweb/scripts/add-missing-keys.mjsweb/scripts/gen-routes.mjsweb/src/components/layout/components/public-header.tsxweb/src/features/auth/api.tsweb/src/features/auth/auth-layout.tsxweb/src/features/auth/components/oauth-providers.tsxweb/src/features/auth/constants.tsweb/src/features/auth/hooks/use-oauth-login.tsweb/src/features/auth/lib/invitation.test.tsweb/src/features/auth/lib/invitation.tsweb/src/features/auth/lib/oauth-create-flow.test.tsweb/src/features/auth/lib/registration.test.tsweb/src/features/auth/lib/registration.tsweb/src/features/auth/lib/storage.test.tsweb/src/features/auth/lib/storage.tsweb/src/features/auth/sign-in/index.tsxweb/src/features/auth/sign-up/components/sign-up-form.tsxweb/src/features/auth/types.tsweb/src/features/invitation-codes/api.tsweb/src/features/invitation-codes/components/create-invitation-codes-dialog.tsxweb/src/features/invitation-codes/components/generated-invitation-codes-dialog.tsxweb/src/features/invitation-codes/components/invitation-code-actions.tsxweb/src/features/invitation-codes/components/invitation-codes-data-view.tsxweb/src/features/invitation-codes/components/invitation-codes-table.tsxweb/src/features/invitation-codes/constants.tsweb/src/features/invitation-codes/index.tsxweb/src/features/invitation-codes/types.tsweb/src/features/system-settings/api.tsweb/src/features/system-settings/auth/basic-auth-section.tsxweb/src/features/system-settings/auth/index.tsxweb/src/features/system-settings/auth/invitation-code-section.tsxweb/src/features/system-settings/auth/section-registry.tsxweb/src/features/system-settings/hooks/use-update-invitation-code-config.tsweb/src/features/system-settings/hooks/use-update-option.tsweb/src/features/system-settings/maintenance/config.tsweb/src/features/system-settings/maintenance/header-navigation-section.tsxweb/src/features/system-settings/maintenance/sidebar-modules-section.tsxweb/src/features/system-settings/types.tsweb/src/features/usage/api.tsweb/src/features/usage/components/index.tsweb/src/features/usage/components/podium.tsxweb/src/features/usage/components/usage-hero.tsxweb/src/features/usage/components/usage-list.tsxweb/src/features/usage/hooks/use-usage-leaderboard.tsweb/src/features/usage/index.tsxweb/src/features/usage/types.tsweb/src/hooks/use-sidebar-config.tsweb/src/hooks/use-sidebar-data.tsweb/src/hooks/use-top-nav-links.tsweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-TW.jsonweb/src/i18n/locales/zh.jsonweb/src/i18n/static-keys.tsweb/src/lib/nav-modules.tsweb/src/routeTree.gen.tsweb/src/routes/(auth)/oauth.tsxweb/src/routes/_authenticated/invitation-codes/index.tsxweb/src/routes/usage/index.tsx
| for _, submission := range submissions { | ||
| submission := submission | ||
| go func() { | ||
| defer waitGroup.Done() | ||
| ctx, recorder := invitationControllerContext(t, http.MethodPut, "/api/option/invitation-code", submission.body, submission.adminID) | ||
| <-start | ||
| UpdateInvitationCodeOption(ctx) | ||
| var response struct { | ||
| Success bool `json:"success"` | ||
| } | ||
| if err := common.Unmarshal(recorder.Body.Bytes(), &response); err != nil { | ||
| responses <- false | ||
| return | ||
| } | ||
| responses <- response.Success | ||
| }() | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Building the test context inside the goroutine risks t.FailNow from a non-test goroutine.
invitationControllerContext(t, ...) performs require-style assertions; if it fails inside a spawned goroutine, t.FailNow() is called off the test goroutine, which Go explicitly does not support (the failure may be lost or hang). Build both contexts in the loop before launching, then only run the handler concurrently.
♻️ Proposed adjustment
for _, submission := range submissions {
- submission := submission
+ ctx, recorder := invitationControllerContext(t, http.MethodPut, "/api/option/invitation-code", submission.body, submission.adminID)
go func() {
defer waitGroup.Done()
- ctx, recorder := invitationControllerContext(t, http.MethodPut, "/api/option/invitation-code", submission.body, submission.adminID)
<-start📝 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.
| for _, submission := range submissions { | |
| submission := submission | |
| go func() { | |
| defer waitGroup.Done() | |
| ctx, recorder := invitationControllerContext(t, http.MethodPut, "/api/option/invitation-code", submission.body, submission.adminID) | |
| <-start | |
| UpdateInvitationCodeOption(ctx) | |
| var response struct { | |
| Success bool `json:"success"` | |
| } | |
| if err := common.Unmarshal(recorder.Body.Bytes(), &response); err != nil { | |
| responses <- false | |
| return | |
| } | |
| responses <- response.Success | |
| }() | |
| } | |
| for _, submission := range submissions { | |
| ctx, recorder := invitationControllerContext(t, http.MethodPut, "/api/option/invitation-code", submission.body, submission.adminID) | |
| go func() { | |
| defer waitGroup.Done() | |
| <-start | |
| UpdateInvitationCodeOption(ctx) | |
| var response struct { | |
| Success bool `json:"success"` | |
| } | |
| if err := common.Unmarshal(recorder.Body.Bytes(), &response); err != nil { | |
| responses <- false | |
| return | |
| } | |
| responses <- response.Success | |
| }() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/option_invitation_test.go` around lines 258 - 274, Move the
invitationControllerContext call out of the goroutine and create each test
context in the submission loop before launching the goroutine. Capture the
prepared ctx and recorder alongside the submission, then have the goroutine wait
on start and invoke UpdateInvitationCode using those values, preserving the
existing response handling and synchronization.
| oldQuotaForNewUser := common.QuotaForNewUser | ||
| oldSettings := common.GetInvitationCodeSettings() | ||
| oldMainDatabaseType, oldLogDatabaseType := common.MainDatabaseType(), common.LogDatabaseType() | ||
| oldOptionMap := common.OptionMap |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
common.OptionMap snapshot is shallow, so option writes leak across tests.
oldOptionMap := common.OptionMap stores the same map reference; handlers exercised here (e.g. PostSetup) mutate entries in place, and restoring the reference does not undo them. TestRootSetupBypassesInvitationAndDefaultToken works around this by manually saving/deleting two keys. Copy the map (and take common.OptionMapRWMutex, as setupInvitationOptionControllerTest in controller/option_invitation_test.go does) so cleanup truly restores state.
🛡️ Proposed fix
- oldOptionMap := common.OptionMap
+ common.OptionMapRWMutex.RLock()
+ var oldOptionMap map[string]string
+ if common.OptionMap != nil {
+ oldOptionMap = make(map[string]string, len(common.OptionMap))
+ for key, value := range common.OptionMap {
+ oldOptionMap[key] = value
+ }
+ }
+ common.OptionMapRWMutex.RUnlock()Also applies to: 60-62, 72-72
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/registration_matrix_test.go` at line 32, Update the OptionMap
snapshot and restoration logic in the affected tests to clone the map while
holding common.OptionMapRWMutex, following setupInvitationOptionControllerTest.
Restore the cloned contents under the same mutex so in-place handler mutations
cannot leak between tests, and remove the manual key-specific workaround in
TestRootSetupBypassesInvitationAndDefaultToken.
| var rows []aggregatedRow | ||
| tx := LOG_DB.Table("logs"). | ||
| Select("user_id, COALESCE(SUM(quota), 0) as total_quota, COUNT(*) as total_requests"). | ||
| Where("type = ? AND created_at >= ? AND created_at <= ? AND user_id > 0", | ||
| LogTypeConsume, startTimestamp, endTimestamp). | ||
| Group("user_id"). | ||
| Order("total_quota DESC"). | ||
| Limit(limit * 2) | ||
|
|
||
| if err := tx.Find(&rows).Error; err != nil { | ||
| return nil, fmt.Errorf("aggregate usage logs: %w", err) | ||
| } | ||
|
|
||
| if len(rows) == 0 { | ||
| return []UserUsageRankEntry{}, nil | ||
| } | ||
|
|
||
| userIds := make([]int, 0, len(rows)) | ||
| for _, r := range rows { | ||
| userIds = append(userIds, r.UserId) | ||
| } | ||
|
|
||
| type userInfo struct { | ||
| Id int `gorm:"column:id"` | ||
| Username string `gorm:"column:username"` | ||
| Role int `gorm:"column:role"` | ||
| } | ||
| var users []userInfo | ||
| if err := DB.Table("users"). | ||
| Select("id, username, role"). | ||
| Where("id IN ?", userIds). | ||
| Find(&users).Error; err != nil { | ||
| return nil, fmt.Errorf("load user info for leaderboard: %w", err) | ||
| } | ||
|
|
||
| userMap := make(map[int]userInfo, len(users)) | ||
| for _, u := range users { | ||
| userMap[u.Id] = u | ||
| } | ||
|
|
||
| result := make([]UserUsageRankEntry, 0, limit) | ||
| rank := 0 | ||
| for _, r := range rows { | ||
| if len(result) >= limit { | ||
| break | ||
| } | ||
| info, ok := userMap[r.UserId] | ||
| if !ok { | ||
| continue | ||
| } | ||
| if info.Role >= common.RoleAdminUser { | ||
| continue | ||
| } | ||
| rank++ | ||
| result = append(result, UserUsageRankEntry{ | ||
| Rank: rank, | ||
| UserId: r.UserId, | ||
| Username: info.Username, | ||
| Quota: r.Quota, | ||
| Requests: r.Requests, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
limit * 2 buffer can under-fill the leaderboard when admins/deleted users dominate the top rows.
Both GetUsageLeaderboard and GetCheckinLeaderboard fetch only limit * 2 aggregated rows, then filter out admins/missing users, and break once len(result) >= limit. If more than half of those limit*2 rows are admins (or reference deleted users), the function returns fewer than limit entries even though enough eligible non-admin users exist further down the true ranking — this can silently short the public-facing leaderboard.
Filtering admins at the SQL layer removes the need for a heuristic buffer and guarantees the requested count when available.
🐛 Proposed fix: exclude admin ids at the query layer
+ var adminIds []int
+ if err := DB.Table("users").
+ Where("role >= ?", common.RoleAdminUser).
+ Pluck("id", &adminIds).Error; err != nil {
+ return nil, fmt.Errorf("load admin ids: %w", err)
+ }
+
var rows []aggregatedRow
- tx := LOG_DB.Table("logs").
+ tx := LOG_DB.Table("logs").
Select("user_id, COALESCE(SUM(quota), 0) as total_quota, COUNT(*) as total_requests").
Where("type = ? AND created_at >= ? AND created_at <= ? AND user_id > 0",
- LogTypeConsume, startTimestamp, endTimestamp).
- Group("user_id").
- Order("total_quota DESC").
- Limit(limit * 2)
+ LogTypeConsume, startTimestamp, endTimestamp)
+ if len(adminIds) > 0 {
+ tx = tx.Where("user_id NOT IN ?", adminIds)
+ }
+ tx = tx.Group("user_id").Order("total_quota DESC").Limit(limit)The same pattern applies to GetCheckinLeaderboard's checkins query. This also removes the need for the post-hoc if info.Role >= common.RoleAdminUser { continue } checks and the rank++/break bookkeeping, since every returned row is already eligible.
Also applies to: 133-192
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@model/user_leaderboard.go` around lines 51 - 112, Update GetUsageLeaderboard
and GetCheckinLeaderboard to exclude admin users in their aggregation queries
via the users table, rather than fetching a limit*2 heuristic and filtering
afterward. Use the requested limit directly, remove the post-query admin checks
and obsolete rank/break bookkeeping, and assign ranks from the remaining
eligible results while preserving ordering and missing-user handling.
| func userOAuthBindingFromAuthIdentity(identity *AuthIdentity) (*UserOAuthBinding, bool, error) { | ||
| providerId, custom := customOAuthProviderIdFromAuthIdentityKey(identity.ProviderKey) | ||
| if !custom { | ||
| return nil, false, nil | ||
| } | ||
| if identity.ProviderSubject == "" { | ||
| return nil, false, fmt.Errorf("custom OAuth identity %d has no migrated subject value", identity.Id) | ||
| } | ||
| return &UserOAuthBinding{ | ||
| Id: int(identity.Id), | ||
| UserId: identity.UserId, | ||
| ProviderId: providerId, | ||
| ProviderUserId: identity.ProviderSubject, | ||
| CreatedAt: identity.CreatedAt, | ||
| }, true, nil | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Two rough edges in the projection helper.
- Returning an error when
ProviderSubjectis empty makesGetUserOAuthBindingsByUserId(Lines 38-41) fail the entire listing because of one unbackfilled row, so a single legacy/NULLprovider_subject_valuetakes down every binding view for that user. Skipping the row (with acommon.SysLognote) degrades gracefully and matches how the migration tolerates dirty rows. GetUserOAuthBinding(Line 59) discardsok, so a non-custom identity would yield(nil, nil)— a nil binding with no error. The query filters on the custom provider key today, so it is unreachable, but it is a cheap guard against future refactoring.
♻️ Suggested handling
var identity AuthIdentity
if err := DB.Where("user_id = ? AND provider_key = ?", userId, providerKey).First(&identity).Error; err != nil {
return nil, err
}
- binding, _, err := userOAuthBindingFromAuthIdentity(&identity)
- return binding, err
+ binding, ok, err := userOAuthBindingFromAuthIdentity(&identity)
+ if err != nil {
+ return nil, err
+ }
+ if !ok {
+ return nil, gorm.ErrRecordNotFound
+ }
+ return binding, nil🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@model/user_oauth_binding.go` around lines 164 - 179, Update
userOAuthBindingFromAuthIdentity to skip custom identities with an empty
ProviderSubject instead of returning an error, logging the skipped legacy row
with common.SysLog while preserving the existing non-custom result. Also update
GetUserOAuthBinding to check the helper’s ok result and return an appropriate
not-found/error outcome when the identity is not custom rather than returning a
nil binding with no error.
| if date == "" { | ||
| date = time.Now().Format("2006-01-02") | ||
| } | ||
|
|
||
| cacheKey := fmt.Sprintf("checkin:%s:%d", date, limit) | ||
| now := time.Now() | ||
|
|
||
| checkinLeaderboardCacheMu.Lock() | ||
| if item, ok := checkinLeaderboardCache[cacheKey]; ok && now.Before(item.expiresAt) { | ||
| copied := *item.data | ||
| copied.FromCache = true | ||
| copied.Entries = cloneCheckinEntries(item.data.Entries) | ||
| markSelfCheckin(copied.Entries, currentUserId) | ||
| checkinLeaderboardCacheMu.Unlock() | ||
| return &copied, nil | ||
| } | ||
| checkinLeaderboardCacheMu.Unlock() | ||
|
|
||
| entries, err := model.GetCheckinLeaderboard(date, limit) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| resp := &CheckinLeaderboardResponse{ | ||
| Date: date, | ||
| Entries: entries, | ||
| CachedAt: now.Unix(), | ||
| } | ||
|
|
||
| checkinLeaderboardCacheMu.Lock() | ||
| checkinLeaderboardCache[cacheKey] = checkinLeaderboardCacheItem{ | ||
| expiresAt: now.Add(userLeaderboardCacheTTL), | ||
| data: resp, | ||
| } | ||
| checkinLeaderboardCacheMu.Unlock() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound and evict the check-in cache.
date is caller-controlled, becomes part of the cache key, and expired entries are never deleted. Authenticated callers can accumulate unbounded cache entries with unique dates. Validate dates and enforce eviction/capacity before inserting new entries.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@service/user_leaderboard.go` around lines 163 - 197, Validate the
caller-provided date before constructing the cache key, accepting only the
expected YYYY-MM-DD format and rejecting invalid values. Update the check-in
cache insertion flow around checkinLeaderboardCache to evict expired entries and
enforce a bounded maximum capacity before adding a new entry, while preserving
existing cache-hit behavior and response generation.
| {isLoading ? ( | ||
| <UsageLoading /> | ||
| ) : error ? ( | ||
| <UsageError | ||
| message={ | ||
| error instanceof Error | ||
| ? error.message | ||
| : t('Unable to load leaderboard data') | ||
| } | ||
| /> | ||
| ) : isCheckin ? ( | ||
| <CheckinBoard | ||
| entries={checkinQuery.data?.entries ?? []} | ||
| date={checkinQuery.data?.date} | ||
| fromCache={checkinQuery.data?.from_cache} | ||
| /> | ||
| ) : ( | ||
| <UsageBoard | ||
| entries={usageQuery.data?.entries ?? []} | ||
| fromCache={usageQuery.data?.from_cache} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Three-level nested ternary for board rendering.
isLoading ? ... : error ? ... : isCheckin ? ... : ... nests three ternaries, violating the project's ternary-nesting rule and hurting readability.
As per coding guidelines, "禁止两层及以上嵌套三元表达式；复杂逻辑应拆分为小函数" and "Keep new code direct and readable: prefer early returns, clear branches, and well-named local variables over deep nesting or layered control flow."
♻️ Proposed fix
- {isLoading ? (
- <UsageLoading />
- ) : error ? (
- <UsageError
- message={
- error instanceof Error
- ? error.message
- : t('Unable to load leaderboard data')
- }
- />
- ) : isCheckin ? (
- <CheckinBoard
- entries={checkinQuery.data?.entries ?? []}
- date={checkinQuery.data?.date}
- fromCache={checkinQuery.data?.from_cache}
- />
- ) : (
- <UsageBoard
- entries={usageQuery.data?.entries ?? []}
- fromCache={usageQuery.data?.from_cache}
- />
- )}
+ {renderBoard()}function renderBoard() {
if (isLoading) return <UsageLoading />
if (error) return <UsageError message={getErrorMessage(error, t)} />
if (isCheckin) {
return (
<CheckinBoard
entries={checkinQuery.data?.entries ?? []}
date={checkinQuery.data?.date}
fromCache={checkinQuery.data?.from_cache}
/>
)
}
return (
<UsageBoard
entries={usageQuery.data?.entries ?? []}
fromCache={usageQuery.data?.from_cache}
/>
)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/usage/index.tsx` around lines 79 - 100, Replace the nested
ternary in the board-rendering JSX with a clear branch-based helper such as
renderBoard, using early returns for isLoading and error, then separate
isCheckin and UsageBoard branches. Preserve the existing error-message fallback
and all board props while calling the helper from the component render.
Source: Coding guidelines
| ) : error ? ( | ||
| <UsageError | ||
| message={ | ||
| error instanceof Error | ||
| ? error.message | ||
| : t('Unable to load leaderboard data') | ||
| } | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Raw error message shown to the user instead of going through handleServerError.
error.message is rendered directly, bypassing the project's standard server-error handling/localization path.
As per coding guidelines, "服务端错误统一使用 handleServerError；错误提示使用 i18n，统一通过 toast.error 等方式展示。"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/features/usage/index.tsx` around lines 81 - 88, Update the error
message logic in the usage component’s <code>UsageError</code> rendering to
route server errors through the existing <code>handleServerError</code> helper
instead of displaying <code>error.message</code> directly. Preserve the fallback
i18n message for non-<code>Error</code> values and ensure the resulting
user-facing text follows the project’s localized server-error handling path.
Source: Coding guidelines
| "Expiration time": "Date d'expiration", | ||
| "Expiration Time": "Heure d'expiration", | ||
| "Expiration time must be in the future": "La date d'expiration doit être dans le futur", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate locale files =="
fd -a 'fr\.json$' . | sed 's#^\./##'
echo "== keys and translations around French expiry entries =="
python3 - <<'PY'
import json
from pathlib import Path
p = Path('web/src/i18n/locales/fr.json')
data = json.loads(p.read_text())
for k,v in data.get('translation', data).items():
if 'expiration' in k.lower() or 'Expiration' in k:
print(f"{k!r}: {v!r}")
PY
echo "== source usage of exact keys =="
rg -n '"Expiration time"|"Expiration Time"|i18next|t\(' web/src -g '*.{ts,tsx,js,jsx,json}' || true
echo "== all references to expiration translation keys =="
rg -n 'Expiration (time|Time)' web/src i18n . --glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: QuantumNous/new-api
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== French expiry keys and values =="
python3 - <<'PY'
import json
from pathlib import Path
p = Path('web/src/i18n/locales/fr.json')
data = json.loads(p.read_text())
for k, v in (data.get('translation') or data).items():
if 'Expiration' in k:
print(f"{k!r}: {v!r}")
PY
echo "== exact source references around key literals =="
rg -n --word-regexp 'Expiration (time|Time)' web/src -g '*.{ts,tsx,js,jsx}' || true
echo "== all source references to key literals (no word-boundary) =="
rg -n --no-heading 'Expiration time|Expiration Time' web/src -g '*.{ts,tsx,js,jsx}' || true
echo "== locale entry context =="
sed -n '1768,1776p' web/src/i18n/locales/fr.jsonRepository: QuantumNous/new-api
Length of output: 2336
Keep the expiration label translations aligned.
"Expiration time" is used for invitation-code expiration, while "Expiration Time" is used for API and redemption-code expiration labels. Translate these consistently unless the controls are intentionally different; for example, use the same wording such as Heure d'expiration for both labels.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/i18n/locales/fr.json` around lines 1772 - 1774, Align the French
translations for the “Expiration time” and “Expiration Time” keys in the locale
file by using the same wording, such as “Heure d'expiration,” while leaving the
future-expiration message unchanged.
| "Check-in leaderboard for {{date}}": "Bảng xếp hạng đăng nhập cho {{date}}", | ||
| "Check-in reward": "Phần thưởng đăng nhập", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use consistent “điểm danh” terminology for check-in features.
The current translations use “đăng nhập” (login), which changes the meaning of daily check-ins and their rewards/rankings.
web/src/i18n/locales/vi.json#L799-L800: Translate the leaderboard and reward as “điểm danh”.web/src/i18n/locales/vi.json#L4085: Translate daily check-in rewards as “phần thưởng điểm danh hàng ngày”.web/src/i18n/locales/vi.json#L4966-L4967: Translate daily check-in rankings as “xếp hạng điểm danh hàng ngày”.
📍 Affects 1 file
web/src/i18n/locales/vi.json#L799-L800(this comment)web/src/i18n/locales/vi.json#L4085-L4085web/src/i18n/locales/vi.json#L4966-L4967
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/i18n/locales/vi.json` around lines 799 - 800, The Vietnamese
translations in web/src/i18n/locales/vi.json lines 799-800, 4085, and 4966-4967
use “đăng nhập” instead of the intended “điểm danh” terminology; update the
leaderboard and reward entries at lines 799-800, the daily check-in reward at
line 4085 to “phần thưởng điểm danh hàng ngày”, and the daily check-in ranking
entries at lines 4966-4967 to “xếp hạng điểm danh hàng ngày”.
| "Security & Limits": "安全與限制", | ||
| "Security Check": "安全驗證", | ||
| "Security verification": "安全驗證", | ||
| "See who is leading the platform by usage and daily check-in rewards. Administrators are excluded.": "查看誰在用量和每日簽到獎勵上領先平台。管理員不參與排名。", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the leaderboard description translation.
The current text reads as “who leads the platform,” rather than “who leads on the platform.” Consider:
- "See who is leading the platform by usage and daily check-in rewards. Administrators are excluded.": "查看誰在用量和每日簽到獎勵上領先平台。管理員不參與排名。",
+ "See who is leading the platform by usage and daily check-in rewards. Administrators are excluded.": "查看平台上用量和每日簽到獎勵的領先者。管理員不參與排名。",📝 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.
| "See who is leading the platform by usage and daily check-in rewards. Administrators are excluded.": "查看誰在用量和每日簽到獎勵上領先平台。管理員不參與排名。", | |
| "See who is leading the platform by usage and daily check-in rewards. Administrators are excluded.": "查看平台上用量和每日簽到獎勵的領先者。管理員不參與排名。", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/i18n/locales/zh-TW.json` at line 4085, Update the zh-TW translation
for the leaderboard description key “See who is leading the platform by usage
and daily check-in rewards. Administrators are excluded.” so it conveys who
leads on the platform in usage and daily check-in rewards, rather than implying
someone leads the platform itself.
2 participants
Important
📝 变更描述 / Description
新增"用量排行榜"功能，作为与首页/控制台/模型广场同级的顶部菜单项"用量"，需登录查看，可在系统设置中开关。
后端
model/user_leaderboard.go：两步聚合查询（log 库 SUM(quota) → user 库补 username/role），排除管理员（role >= RoleAdminUser），结果上限 100，跨库兼容（SQLite/MySQL/PostgreSQL/ClickHouse）service/user_leaderboard.go：内存缓存 2 分钟 TTL，缓存 key 按 period+limit 共享，IsSelf标记按请求克隆应用（避免缓存污染）controller/user_leaderboard.go：/api/usage/leaderboard（period=today|week|month）与/api/usage/checkin（date）middleware/header_nav.go：新增HeaderNavModuleRequiredAuth，用量模块启用时强制鉴权（忽略 RequireAuth 标志，防止误公开用户数据）前端
web/src/features/usage/：主页面 + Day/Week/Month/当日签到 Tab 切换components/podium.tsx：金银铜领奖台（奥运视觉顺序 2-1-3），OKLCH 色彩 + 渐变发光，复用全局animate-appear工具类的 staggered 入场动画（已尊重prefers-reduced-motion）components/usage-list.tsx：第 4 名起依次排开，当前用户行高亮/usage路由：beforeLoad中做模块门控 + 强制鉴权_sync-report.jsonmissing/extras/untranslated 均为 0性能保障：结果集上限 100、两步聚合避免跨库 JOIN、双层缓存（后端 2min + 前端 React Query staleTime 2min）、纯 CSS 动画零运行时成本、骨架屏加载。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
go build ./controller/ ./service/ ./middleware/ ./router/ ./model/通过；前端bun run typecheck+bun run build通过。📸 运行证明 / Proof of Work
后端 build
前端 typecheck + build
i18n 同步报告（
web/src/i18n/locales/_reports/_sync-report.json）：7 种语言 missingCount/extrasCount/untranslatedCount 均为 0。Summary by CodeRabbit
New Features
Bug Fixes
Documentation