Conversation
- Added a new function to determine the redirect target after login based on user role and permissions. - Updated the `validateKey` function to allow read-only access for users without Web UI login permissions. - Modified the dashboard layout to redirect users without admin access to their usage page. - Introduced new methods to retrieve total usage and distinct models for a given API key. This update improves user experience by ensuring appropriate access control and navigation based on user roles.
- Introduced READ_ONLY_PATH_PATTERNS to allow specific paths to bypass the canLoginWebUi check. - Updated the proxyHandler to check for read-only access, modifying the validateKey function call accordingly. This change improves access control for users without Web UI login permissions, ensuring they can still access necessary information.
- Removed redundant nesting in myUsage JSON files for cleaner structure. - Updated filter handling in the usage logs section to use a default value of "__all__" for model and status filters, enhancing user experience. - Ensured consistent terminology across multiple language files for better localization support. This refactor improves the maintainability of the code and enhances the user interface for filtering usage logs.
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant new feature: a 'My Usage' section for users. This section provides a clear overview of API key quotas and usage logs, enhancing user transparency and control over their consumption. The changes span across new UI components, server-side data fetching logic, and crucial updates to the authentication system to correctly route users based on their key's web UI access permissions. It also adjusts the default behavior for new API keys regarding web interface login capabilities. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
ding113
added
size/L
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'My Usage' page, which is a great feature for end-users to monitor their quota and usage logs. The implementation is well-structured, with clear separation of concerns between server actions, components, and internationalization files. The access control logic, which redirects users based on their permissions (canLoginWebUi), is handled robustly in the middleware, API routes, and page layouts.
My review includes a few suggestions for improvement:
- A high-severity note on a change to a default value in the database schema that could have a significant impact on new key creation.
- A couple of medium-severity suggestions to improve performance and maintainability in the data-fetching logic.
Overall, this is a solid contribution. Addressing these points will make the implementation even more robust and scalable.
I am having trouble creating individual review comments. Click here to see my feedback.
src/drizzle/schema.ts (59)
The default value for canLoginWebUi has been changed from true to false. This is a significant behavioral change that will affect all newly created API keys, as they will no longer have access to the main dashboard by default and will be redirected to the "My Usage" page instead. While this seems intentional for the new feature, it's a breaking change for administrators who expect the previous default behavior. Please ensure this change is clearly communicated and that existing key creation workflows are updated to explicitly set canLoginWebUi: true if full dashboard access is desired for new keys.
src/actions/my-usage.ts (372-385)
The getUserConcurrentSessions function fetches all keys for a user and then iterates through them, calling SessionTracker.getKeySessionCount for each one. This results in N+1 database/Redis queries (1 to get keys, N to get session counts), which can be inefficient if a user has a large number of keys.
Consider optimizing this by creating a batch-capable function in SessionTracker, similar to getProviderSessionCountBatch, that can fetch session counts for multiple keys in a more efficient way (e.g., using a single Redis pipeline).
src/repository/usage-logs.ts (80-92)
The getDistinctModelsForKey function uses raw SQL with db.execute. While this works, it bypasses Drizzle's type safety and query building capabilities. For better maintainability and consistency with the rest of the repository's data access patterns, consider refactoring this to use the Drizzle query builder.
export async function getDistinctModelsForKey(keyString: string): Promise<string[]> {
const modelColumn = sql<string>`coalesce(${messageRequest.originalModel}, ${messageRequest.model})`.as("model");
const rows = await db
.selectDistinct({ model: modelColumn })
.from(messageRequest)
.where(and(eq(messageRequest.key, keyString), isNull(messageRequest.deletedAt)))
.orderBy(modelColumn);
return rows
.map((row) => row.model)
.filter((model): model is string => !!model && model.trim().length > 0);
}
| filters: MyUsageLogsFilters = {} | ||
| ): Promise<ActionResult<MyUsageLogsResult>> { | ||
| try { | ||
| const session = await getSession({ allowReadOnlyAccess: true }); |
There was a problem hiding this comment.
High Type mismatch in usage log filters
Why this is a problem: The UsageLogFilters interface expects startTime and endTime as numbers (milliseconds), but the code is passing startDateLocal and endDateLocal as strings. This will cause the time filters to be silently ignored, and the query will return all logs instead of the filtered time range.
Suggested fix:
const usageFilters: UsageLogFilters = {
keyId: session.key.id,
startTime: filters.startDate ? new Date(`${filters.startDate}T00:00`).getTime() : undefined,
endTime: filters.endDate ? new Date(`${filters.endDate}T23:59:59.999`).getTime() : undefined,
model: filters.model,
statusCode: filters.statusCode,
page,
pageSize,
};
| return Number(row?.total ?? 0); | ||
| } | ||
|
|
||
| export async function getDistinctModelsForKey(keyString: string): Promise<string[]> { |
There was a problem hiding this comment.
Medium Potential SQL injection in raw SQL query
Why this is a problem: The getDistinctModelsForKey function uses raw SQL with string interpolation that could be vulnerable to SQL injection if the keyString parameter is not properly sanitized. While Drizzle's sql tagged template should handle parameterization, mixing it with direct variable references is error-prone.
Suggested fix:
export async function getDistinctModelsForKey(keyString: string): Promise<string[]> {
const result = await db
.selectDistinct({
model: sql<string>`coalesce(${messageRequest.originalModel}, ${messageRequest.model})`,
})
.from(messageRequest)
.where(
and(
eq(messageRequest.key, keyString),
isNull(messageRequest.deletedAt),
sql`coalesce(${messageRequest.originalModel}, ${messageRequest.model}) is not null`
)
)
.orderBy(sql`coalesce(${messageRequest.originalModel}, ${messageRequest.model}) asc`);
return result
.map((row) => row.model)
.filter((model): model is string => !!model && model.trim().length > 0);
}
| } | ||
|
|
||
| async function getUserConcurrentSessions(userId: number): Promise<number> { | ||
| try { |
There was a problem hiding this comment.
Low Error handling silently returns 0 on failure
Why this is a problem: The getUserConcurrentSessions function catches all errors and returns 0, which could mask database connection issues or other failures. Callers won't know if the returned 0 means "no sessions" or "failed to check".
Suggested fix:
async function getUserConcurrentSessions(userId: number): Promise<number> {
const keys = await db
.select({ id: keysTable.id })
.from(keysTable)
.where(and(eq(keysTable.userId, userId), isNull(keysTable.deletedAt)));
const counts = await Promise.all(keys.map((k) => SessionTracker.getKeySessionCount(k.id)));
return counts.reduce((sum, value) => sum + value, 0);
}Remove the try-catch and let errors propagate to the caller's error handler.
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
Code Review Summary
This PR adds a "My Usage" feature for non-admin users to view their quota and usage logs. The implementation is mostly solid but contains one critical type mismatch bug that will break time filtering functionality.
PR Size: L
- Lines changed: 1529
- Files changed: 25
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Code Quality | 0 | 1 | 0 | 1 |
| Security | 0 | 0 | 1 | 0 |
| Dependencies | 0 | 0 | 0 | 0 |
| Documentation | 0 | 0 | 0 | 0 |
Priority Actions
-
Fix the type mismatch in getMyUsageLogs (High) - The time filter parameters use wrong field names (startDateLocal/endDateLocal instead of startTime/endTime), causing filters to be silently ignored. This is a functional bug.
-
Refactor raw SQL query (Medium) - The getDistinctModelsForKey function uses db.execute with raw SQL which is error-prone. Should use Drizzle's query builder for consistency and safety.
-
Remove unnecessary error swallowing (Low) - The getUserConcurrentSessions function catches all errors and returns 0, masking potential database issues.
Review Coverage
- Code quality and correctness
- Security (OWASP Top 10)
- PR size assessment - Large
- Dependency changes - No dependency changes
- Documentation changes - Multiple i18n translation files added
Automated review by Claude AI
Fix TypeScript error where startDateLocal and endDateLocal properties don't exist in UsageLogFilters interface. The interface expects startTime and endTime as millisecond timestamps instead. - Changed from startDateLocal/endDateLocal to startTime/endTime - Convert date strings to millisecond timestamps using Date.getTime() - Use proper time boundaries: 00:00:00 for start, 23:59:59.999 for end
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
Summary
This PR introduces a self-service "My Usage" portal that allows non-admin users to view their own quotas, usage statistics, and request logs without accessing the admin dashboard. The feature enhances user autonomy and reduces admin workload by providing transparent access to personal usage data.
Problem
Previously, users had no way to:
This lack of visibility forced users to contact administrators for basic usage information, creating unnecessary overhead and reducing transparency.
Solution
Implemented a dedicated
/my-usagepage with three main sections:1. Quota Display
2. Today's Statistics (Auto-refresh)
3. Usage Logs
Changes
New Files
src/actions/my-usage.ts- Server actions for fetching quota, stats, and logssrc/app/[locale]/my-usage/page.tsx- Main usage page componentsrc/app/[locale]/my-usage/_components/- UI components (header, quota cards, today stats, logs table)messages/*/myUsage.json- i18n translations for en/zh-CN/zh-TW/ja/ruModified Files
src/app/[locale]/dashboard/layout.tsx- Redirect non-admin users withoutcanLoginWebUito/my-usagesrc/app/[locale]/login/page.tsx- Support redirectTo from login responsemessages/*/index.ts- Register new myUsage namespace for all localesKey Technical Details
getSession({ allowReadOnlyAccess: true })for read-only access pathsgetEnvConfig().TZfor "today" statsbillingModelSourcesystem setting for displaying billing modelsTesting
Screenshots
UI screenshots showing quota cards, today's statistics, and usage logs table
Related Changes
This PR builds on recent auth infrastructure improvements:
Migration Notes
No database migrations required. Uses existing tables (
message_request,keys,users).