Teammate/finalize dashboard and api url - #4574
Closed
jkjk02 wants to merge 23 commits into
Closed
Conversation
# Conflicts: # web/default/src/components/layout/types.ts # web/default/src/features/wallet/components/affiliate-rewards-card.tsx # web/default/src/features/wallet/index.tsx # web/default/src/hooks/use-sidebar-data.ts
add api request url card to keys page
add uptime card to dashboard overview
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughCentralizes root-level permission checks via new helpers, applies them across backend controllers and middleware, broadens frontend admin thresholds, adds model-square and status-monitor routes/pages, refactors wallet/recharge and API-keys UI, injects customizable nav links, updates i18n, and adds deployment SOP and .gitignore entries. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
web/default/src/features/wallet/components/recharge-form-card.tsx (1)
25-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCurrency display is now hardcoded to CNY.
Switching these payment amounts to
formatCnyAmountforces¥formatting and can show incorrect currency in non-CNY setups. This is a checkout-facing correctness issue.Suggested patch
import { - formatCnyAmount, + formatCurrency, getPaymentIcon, getMinTopupAmount, calculatePresetPricing, } from '../lib' @@ - {t('Discount')} {formatCnyAmount(savedAmount)} + {t('Discount')} {formatCurrency(savedAmount)} @@ - {formatCnyAmount(actualPrice)} + {formatCurrency(actualPrice)} @@ - {formatCnyAmount(actualPrice + savedAmount)} + {formatCurrency(actualPrice + savedAmount)} @@ - {formatCnyAmount(paymentAmount)} + {formatCurrency(paymentAmount)}Also applies to: 251-252, 258-259, 265-266, 302-303
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/features/wallet/components/recharge-form-card.tsx` around lines 25 - 29, The UI is forcing CNY by using formatCnyAmount; replace all uses of formatCnyAmount in recharge-form-card.tsx with the project’s currency-aware formatter (e.g., formatAmount or formatCurrency) so amounts respect the current user/checkout currency, and adjust the import list accordingly (remove formatCnyAmount, import the generic formatter). Ensure the formatter is called with the active currency/locale (from whatever currentCurrency or wallet settings are used in this component) for every spot where formatCnyAmount was used (including the other occurrences noted) so calculatePresetPricing/getMinTopupAmount outputs are displayed with the correct currency.web/default/src/features/keys/components/api-keys-mutate-drawer.tsx (2)
156-168:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly apply create defaults when the drawer opens.
defaultUseAutoGroupcomes fromuseStatus(), so it can change after the sheet is already open. In create mode this effect re-runs on that async update and callsform.reset(...), which will wipe any fields the user already started filling out.Suggested guard
-import { useEffect, useState, type ReactNode } from 'react' +import { useEffect, useRef, useState, type ReactNode } from 'react' ... + const wasOpenRef = useRef(false) ... useEffect(() => { + const justOpened = open && !wasOpenRef.current + wasOpenRef.current = open + if (open && isUpdate && currentRow) { // For update, fetch fresh data getApiKey(currentRow.id).then((result) => { if (result.success && result.data) { form.reset(transformApiKeyToFormDefaults(result.data)) } }) - } else if (open && !isUpdate) { + } else if (justOpened && !isUpdate) { // For create, reset to defaults form.reset(getApiKeyFormDefaultValues(defaultUseAutoGroup)) } }, [open, isUpdate, currentRow, form, defaultUseAutoGroup])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/features/keys/components/api-keys-mutate-drawer.tsx` around lines 156 - 168, The effect is resetting create-mode form defaults whenever defaultUseAutoGroup changes while the drawer is open, wiping user input; modify the useEffect so create defaults only apply when the drawer actually opens (i.e., on the transition open: false -> true). Implement a prevOpen ref (e.g., const prevOpen = useRef(false)) and in the effect only call form.reset(getApiKeyFormDefaultValues(defaultUseAutoGroup)) when open && !isUpdate && !prevOpen.current, and then set prevOpen.current = open at the end of the effect; keep the existing update branch (getApiKey(...) -> form.reset(transformApiKeyToFormDefaults(...))) unchanged.
128-148:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the current
autogroup visible while editing.If an existing key loads with
group === 'auto',transformApiKeyToFormDefaults()will repopulate that value, but this filter removes the matching option wheneverdefault_use_auto_groupis false. The combobox then renders as if no group is selected, even though the form still has a valid value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/features/keys/components/api-keys-mutate-drawer.tsx` around lines 128 - 148, The filter currently drops the 'auto' option when defaultUseAutoGroup is false, which hides the selected 'auto' value loaded by transformApiKeyToFormDefaults(); update the filter to keep 'auto' if the form's current group value (from transformApiKeyToFormDefaults() or the form state/initial values) equals 'auto'. Concretely, in the map/filter logic around groupsRaw and defaultUseAutoGroup, change the key === 'auto' branch to return defaultUseAutoGroup || currentFormGroup === 'auto' (where currentFormGroup is obtained from transformApiKeyToFormDefaults() output or the form's values) so the combobox shows the 'auto' option while editing.web/default/src/hooks/use-top-nav-links.ts (1)
72-80:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep a public pricing destination for unauthenticated users.
/model-squareis mounted under/_authenticated, but this branch still allowspricing.requireAuth === false. Guests will now hit an auth gate even when the backend config says pricing should be public.One straightforward fix
const pricing = modules?.pricing if (pricing && typeof pricing === 'object' && pricing.enabled) { const disabled = pricing.requireAuth && !isAuthed links.push({ title: t('Pricing'), - href: '/model-square?view=table', + href: + pricing.requireAuth || isAuthed + ? '/model-square?view=table' + : '/pricing', disabled, }) }Based on learnings: Use
beforeLoadhook for authentication checks and redirects to avoid unnecessary requests; use layout routes and prefixes like_authenticatedfor nested structure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/hooks/use-top-nav-links.ts` around lines 72 - 80, The Pricing link currently always points to the authenticated-mounted route which will trigger the auth gate for guests; update the link creation in use-top-nav-links.ts (the pricing branch handling the modules?.pricing object) to compute href based on pricing.requireAuth: if pricing.requireAuth === false use the public path (no "/_authenticated" prefix) and if true prefix the route with "/_authenticated" (e.g. "/_authenticated/model-square?view=table"), while keeping the disabled flag calculation (disabled = pricing.requireAuth && !isAuthed) unchanged so guests see a public destination when backend config says pricing is public.
🧹 Nitpick comments (8)
docs/private-deploy-sop.md (1)
14-18: ⚡ Quick winClarify owner-only exception for direct push to
private/custom-uiLine 14-18 says collaborators should not push directly to
private/custom-ui, but Line 343 showsgit push origin private/custom-ui. This can be misread in a “协作者版” SOP. Add an explicit “仅仓库负责人执行” label above this command (or move it to a separate owner-only section).Also applies to: 343-344
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/private-deploy-sop.md` around lines 14 - 18, Clarify that the direct push is owner-only by adding an explicit "仅仓库负责人执行" label or moving the command into an owner-only section for the deployment step that runs git push origin private/custom-ui (and any similar commands referencing private/custom-ui or Micah-Zheng/new-api:private/custom-ui); update the SOP text near the existing guidance that collaborators must create PRs to private/custom-ui so it's clear collaborators must not run those push/deploy commands, and ensure the owner-only label appears immediately above the git push command to avoid confusion.middleware/auth.go (1)
182-185: ⚡ Quick win
RootAuthnow has admin semantics—please make that explicit in naming or docs.Line 184 makes
RootAuthequivalent toAdminAuth. Clarifying intent (rename or explicit comment) will prevent accidental misuse on truly root-only endpoints.💡 Minimal clarification option
func RootAuth() func(c *gin.Context) { return func(c *gin.Context) { + // Intentional: root-protected routes now allow admin-or-higher roles. authHelper(c, common.RoleAdminUser) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@middleware/auth.go` around lines 182 - 185, The function RootAuth currently enforces admin-level permissions (calls authHelper with common.RoleAdminUser); rename RootAuth to AdminAuth (and update all callers) or add an explicit doc comment above RootAuth stating it intentionally maps to admin semantics and is NOT for super-root-only endpoints — locate the function RootAuth and its use of authHelper and common.RoleAdminUser to make the change so callers and docs remain consistent.web/default/src/routes/_authenticated/status-monitor.tsx (1)
16-20: ⚡ Quick winHarden the embedded external iframe.
For a third-party page embed, add
sandboxand a stricterreferrerPolicyto reduce unnecessary trust in the framed content.Suggested patch
<iframe src={STATUS_MONITOR_URL} title='Status Monitor' className='h-full w-full border-0' + sandbox='allow-scripts allow-same-origin' + referrerPolicy='no-referrer' + loading='lazy' />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/routes/_authenticated/status-monitor.tsx` around lines 16 - 20, The iframe embedding STATUS_MONITOR_URL should be hardened: update the JSX for the <iframe> (the element using STATUS_MONITOR_URL and title 'Status Monitor') to include a restrictive sandbox attribute (e.g., sandbox="allow-scripts" only if scripts are required, otherwise sandbox with no allowances) and set a stricter referrerPolicy such as referrerPolicy="no-referrer" (or "strict-origin-when-cross-origin" if you need some referrer). Ensure you do not include allow-same-origin unless absolutely necessary and avoid broad allowances like allow-popups unless required.web/default/src/features/pricing/components/model-details.tsx (1)
503-509: 💤 Low valueMinor inconsistency:
wrapContentnot memoized here but is inindex.tsx.Unlike the
Pricingcomponent which wrapswrapContentinuseCallback, this implementation is a plain function. SincewrapContentis only called inline during render and not passed as a prop, this doesn't affect behavior—but aligning the pattern would improve consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/features/pricing/components/model-details.tsx` around lines 503 - 509, The wrapContent helper in model-details.tsx is a plain function; make it consistent with Pricing by memoizing it with React.useCallback: replace the plain wrapContent declaration with a useCallback that depends on props.embedded (and any other referenced props), keeping the same return logic that returns children or <PublicLayout>{children}</PublicLayout>; ensure the identifier wrapContent remains the same so callers (inline in render) continue to work.web/default/src/features/pricing/index.tsx (1)
159-178: ⚡ Quick winNested ternary is 2 levels deep—consider extracting to improve readability.
The condition
filteredModels.length > 0 ? (isMobile || viewMode === VIEW_MODES.LIST ? ... : ...) : ...is a 2-level nested ternary. Per coding guidelines, this should be refactored for clarity.♻️ Proposed refactor using early return or extracted logic
+ const renderModelView = () => { + if (filteredModels.length === 0) { + return ( + <EmptyState + searchQuery={searchInput} + hasActiveFilters={hasActiveFilters} + onClearFilters={handleClearAll} + /> + ) + } + + if (isMobile || viewMode === VIEW_MODES.LIST) { + return ( + <VirtualModelList + models={filteredModels} + onModelClick={handleModelClick} + priceRate={priceRate} + usdExchangeRate={usdExchangeRate} + tokenUnit={tokenUnit} + showRechargePrice={showRechargePrice} + /> + ) + } + + return ( + <PricingTable + models={filteredModels} + priceRate={priceRate} + usdExchangeRate={usdExchangeRate} + tokenUnit={tokenUnit} + showRechargePrice={showRechargePrice} + onModelClick={handleModelClick} + /> + ) + }Then in the JSX:
- {filteredModels.length > 0 ? ( - isMobile || viewMode === VIEW_MODES.LIST ? ( - <VirtualModelList ... /> - ) : ( - <PricingTable ... /> - ) - ) : ( - <EmptyState ... /> - )} + {renderModelView()}As per coding guidelines: "Prohibit nested ternary expressions 2 levels or deeper."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/features/pricing/index.tsx` around lines 159 - 178, The JSX contains a nested ternary using filteredModels.length, isMobile and viewMode (VIEW_MODES.LIST) to choose between VirtualModelList and PricingTable; extract that logic into a small helper or precomputed variable (e.g., renderModels or getModelsComponent) that returns the appropriate component given filteredModels, isMobile, viewMode and props like handleModelClick, priceRate, usdExchangeRate, tokenUnit and showRechargePrice, then use that single variable in the JSX return to remove the 2-level nested ternary and improve readability.web/default/src/custom/site.ts (1)
17-28: 💤 Low valueTranslation keys use flat naming rather than hierarchical structure.
The
titleKeyvalues"Model Square"and"Status Monitor"use flat English keys. As per coding guidelines, prefer hierarchical keys likenavigation.sidebar.modelSquarefor better organization and namespace clarity.Since these keys already exist in the locale files and work correctly, this is a low-priority refinement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/custom/site.ts` around lines 17 - 28, Replace the flat English titleKey values in the customSidebarLinks array with hierarchical locale keys: update the titleKey for the object with url '/model-square?view=table' (in customSidebarLinks) to something like "navigation.sidebar.modelSquare" and update the titleKey for the object with url '/status-monitor' to "navigation.sidebar.statusMonitor"; ensure these new keys match the existing entries in the locale files so translations continue to resolve correctly.web/default/src/features/wallet/components/affiliate-rewards-card.tsx (2)
66-66: 💤 Low valueConsider memoizing
discountTiersto avoid recalculation on every render.The
getDiscountTierscall runs on every render. While the computation is lightweight, memoizing it aligns with the codebase pattern and prevents unnecessary work when unrelated state changes.♻️ Proposed memoization
- const discountTiers = getDiscountTiers(props.topupInfo, priceRatio) + const discountTiers = useMemo( + () => getDiscountTiers(props.topupInfo, priceRatio), + [props.topupInfo, priceRatio] + )Add
useMemoto imports:-import { BadgePercent, ArrowRightLeft } from 'lucide-react' +import { useMemo } from 'react' +import { BadgePercent, ArrowRightLeft } from 'lucide-react'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/features/wallet/components/affiliate-rewards-card.tsx` at line 66, Memoize the discount tiers by wrapping the getDiscountTiers call in a useMemo so it only recalculates when its inputs change: replace const discountTiers = getDiscountTiers(props.topupInfo, priceRatio) with a useMemo that depends on props.topupInfo and priceRatio, and add useMemo to the component imports; reference getDiscountTiers and the discountTiers constant so the memo uses those inputs.
20-45: Discount calculation is correct; consider memoizingdiscountTiersfor performance.The
savedAmountcalculation is correct. Throughout the codebase (includinggetDiscountLabelinlib/format.tsandcalculatePresetPricing), discount values represent a "payment ratio" where 1.0 = no discount and values < 1.0 represent the portion paid. The formulasavedAmount = originalPrice * (1 - discount)is consistent with this convention.For minor performance optimization, wrap the
getDiscountTiers()call at line 66 withuseMemo()to avoid recalculating tiers on every render:const discountTiers = useMemo( () => getDiscountTiers(props.topupInfo, priceRatio), [props.topupInfo, priceRatio] )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/features/wallet/components/affiliate-rewards-card.tsx` around lines 20 - 45, The getDiscountTiers function is fine but its result should be memoized in the component to avoid recalculating on every render: replace the direct call that assigns discountTiers with a useMemo wrapper that calls getDiscountTiers(props.topupInfo, priceRatio) and list props.topupInfo and priceRatio as dependencies; ensure React's useMemo is imported and the memoized variable keeps the name discountTiers so existing references remain valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/constants.go`:
- Around line 175-177: The HasRootPermission function currently uses a numeric
threshold (role >= RoleAdminUser) which can accidentally grant new future roles
root access; update HasRootPermission to check explicitly against an allow-list
of privileged role constants (for example compare role == RoleAdminUser or role
== RoleRoot or use a switch/map of allowed roles) so only explicitly named
constants are treated as root-capable; modify the function body to perform
explicit equality checks (or a lookup in a set) against those role identifiers
instead of a >= comparison.
In `@controller/user.go`:
- Around line 547-552: The guards currently allow anyone with
common.HasRootPermission (which may include admins) to assign or delete
root-level accounts; tighten them to root-only by replacing checks like "if
myRole <= originUser.Role && !common.HasRootPermission(myRole)" and the similar
updatedUser check with a strict root-only check (e.g., "if myRole <=
originUser.Role && myRole != common.RoleRoot" or use a dedicated helper like
common.IsRoot(myRole) that returns true only for the root role), and apply the
same change at the other occurrences around the file (the blocks you noted at
lines ~752-753 and ~803-805) so only the root account can be
created/updated/hard-deleted.
In `@web/default/src/components/layout/components/nav-group.tsx`:
- Around line 207-225: The collapsed submenu (SidebarMenuCollapsedDropdown)
still passes sub.url directly into the Link, so replicate the splitUrl
normalization used above: inside SidebarMenuCollapsedDropdown (where sub.url is
used) call splitUrl(String(sub.url)) to get { pathname, search }, pass pathname
to Link's to prop and, if search is non-empty, pass Object.fromEntries(new
URLSearchParams(search)) to the Link's search prop, and preserve behavior like
closing mobile via setOpenMobile(false) and rendering sub.icon, sub.title and
NavBadge as before.
In `@web/default/src/components/layout/components/top-nav.tsx`:
- Around line 18-25: splitHref currently uses href.split('?') which splits on
every '?' and can truncate query values that contain '?' (e.g.,
next=/path?tab=1); update splitHref to only split at the first '?' by finding
the first index (e.g., href.indexOf('?')) and then slice pathname and search
accordingly (when index === -1 return pathname=href, search=''), ensuring the
function name splitHref is modified to use this single-split logic.
In `@web/default/src/features/keys/components/api-request-url-card.tsx`:
- Around line 20-24: The component currently falls back to
window.location.origin for the API URL; instead, import and use the project's
unified api client to derive the configured base URL (e.g., read
api.defaults.baseURL or api.getUri()/config on your api instance) and only fall
back to window.location.origin if that configured base URL is missing; update
the component in api-request-url-card.tsx to reference the api instance (import
the existing shared api) and return its base URL as the primary source for the
displayed/copyable API URL.
In `@web/default/src/features/pricing/components/pricing-table.tsx`:
- Around line 69-73: handleRowClick currently calls
onModelClick(model.model_name || '') which can pass an empty string; update
handleRowClick to guard for a missing model.model_name and return early (do not
call onModelClick) when model.model_name is falsy, ensuring you only invoke
onModelClick with a valid model id (use the model.model_name symbol and the
onModelClick callback to locate the code).
In `@web/default/src/hooks/use-sidebar-data.ts`:
- Around line 89-94: useSidebarData is calling t(link.titleKey) with dynamic
keys (via customSidebarLinks) which can evade scanner-based i18n extraction;
update the i18n registry by adding each dynamic title key to
web/default/src/i18n/static-keys.ts (or convert the dynamic keys to literal
t('...') usages where possible) so the scanner picks them up—locate the
customSidebarLinks usage in use-sidebar-data.ts and ensure every link.titleKey
value is present in static-keys.ts (or replaced with explicit t('literal.key')).
In `@web/default/src/lib/constants.ts`:
- Line 7: The footer component still uses a hardcoded fallback '/logo.png' which
is inconsistent with the new DEFAULT_LOGO constant; update
web/default/src/components/layout/components/footer.tsx to import DEFAULT_LOGO
from lib/constants and use that constant (or replace the hardcoded '/logo.png'
with DEFAULT_LOGO) wherever the fallback logo path is used (e.g., the image src
fallback at the fallback logic around line 62) so the footer and DEFAULT_LOGO
remain synchronized.
In `@web/default/src/lib/roles.ts`:
- Around line 22-24: The role-to-label mapping currently treats any role >=
ROLE.ADMIN as SUPER_ADMIN; change the conditional to check for ROLE.SUPER_ADMIN
instead and return the SUPER_ADMIN label key only for that threshold. In the
block using (role ?? DEFAULT_ROLE) and ROLE_LABEL_KEYS, replace the comparison
>= ROLE.ADMIN with >= ROLE.SUPER_ADMIN and ensure you return
ROLE_LABEL_KEYS[ROLE.SUPER_ADMIN]; keep the final return using
ROLE_LABEL_KEYS[role as RoleValue] ?? ROLE_LABEL_KEYS[DEFAULT_ROLE] unchanged.
In `@web/default/src/routes/_authenticated/status-monitor.tsx`:
- Line 18: Replace the hard-coded iframe title string with a localized value:
import and call useTranslation() in the component that renders the iframe in
status-monitor.tsx, replace title='Status Monitor' with
title={t('statusMonitor.title', 'Status Monitor')}, and add the corresponding
translation key ("statusMonitor.title") to your i18n resource files; ensure
useTranslation() is invoked (e.g., const { t } = useTranslation()) before using
t().
---
Outside diff comments:
In `@web/default/src/features/keys/components/api-keys-mutate-drawer.tsx`:
- Around line 156-168: The effect is resetting create-mode form defaults
whenever defaultUseAutoGroup changes while the drawer is open, wiping user
input; modify the useEffect so create defaults only apply when the drawer
actually opens (i.e., on the transition open: false -> true). Implement a
prevOpen ref (e.g., const prevOpen = useRef(false)) and in the effect only call
form.reset(getApiKeyFormDefaultValues(defaultUseAutoGroup)) when open &&
!isUpdate && !prevOpen.current, and then set prevOpen.current = open at the end
of the effect; keep the existing update branch (getApiKey(...) ->
form.reset(transformApiKeyToFormDefaults(...))) unchanged.
- Around line 128-148: The filter currently drops the 'auto' option when
defaultUseAutoGroup is false, which hides the selected 'auto' value loaded by
transformApiKeyToFormDefaults(); update the filter to keep 'auto' if the form's
current group value (from transformApiKeyToFormDefaults() or the form
state/initial values) equals 'auto'. Concretely, in the map/filter logic around
groupsRaw and defaultUseAutoGroup, change the key === 'auto' branch to return
defaultUseAutoGroup || currentFormGroup === 'auto' (where currentFormGroup is
obtained from transformApiKeyToFormDefaults() output or the form's values) so
the combobox shows the 'auto' option while editing.
In `@web/default/src/features/wallet/components/recharge-form-card.tsx`:
- Around line 25-29: The UI is forcing CNY by using formatCnyAmount; replace all
uses of formatCnyAmount in recharge-form-card.tsx with the project’s
currency-aware formatter (e.g., formatAmount or formatCurrency) so amounts
respect the current user/checkout currency, and adjust the import list
accordingly (remove formatCnyAmount, import the generic formatter). Ensure the
formatter is called with the active currency/locale (from whatever
currentCurrency or wallet settings are used in this component) for every spot
where formatCnyAmount was used (including the other occurrences noted) so
calculatePresetPricing/getMinTopupAmount outputs are displayed with the correct
currency.
In `@web/default/src/hooks/use-top-nav-links.ts`:
- Around line 72-80: The Pricing link currently always points to the
authenticated-mounted route which will trigger the auth gate for guests; update
the link creation in use-top-nav-links.ts (the pricing branch handling the
modules?.pricing object) to compute href based on pricing.requireAuth: if
pricing.requireAuth === false use the public path (no "/_authenticated" prefix)
and if true prefix the route with "/_authenticated" (e.g.
"/_authenticated/model-square?view=table"), while keeping the disabled flag
calculation (disabled = pricing.requireAuth && !isAuthed) unchanged so guests
see a public destination when backend config says pricing is public.
---
Nitpick comments:
In `@docs/private-deploy-sop.md`:
- Around line 14-18: Clarify that the direct push is owner-only by adding an
explicit "仅仓库负责人执行" label or moving the command into an owner-only section for
the deployment step that runs git push origin private/custom-ui (and any similar
commands referencing private/custom-ui or
Micah-Zheng/new-api:private/custom-ui); update the SOP text near the existing
guidance that collaborators must create PRs to private/custom-ui so it's clear
collaborators must not run those push/deploy commands, and ensure the owner-only
label appears immediately above the git push command to avoid confusion.
In `@middleware/auth.go`:
- Around line 182-185: The function RootAuth currently enforces admin-level
permissions (calls authHelper with common.RoleAdminUser); rename RootAuth to
AdminAuth (and update all callers) or add an explicit doc comment above RootAuth
stating it intentionally maps to admin semantics and is NOT for super-root-only
endpoints — locate the function RootAuth and its use of authHelper and
common.RoleAdminUser to make the change so callers and docs remain consistent.
In `@web/default/src/custom/site.ts`:
- Around line 17-28: Replace the flat English titleKey values in the
customSidebarLinks array with hierarchical locale keys: update the titleKey for
the object with url '/model-square?view=table' (in customSidebarLinks) to
something like "navigation.sidebar.modelSquare" and update the titleKey for the
object with url '/status-monitor' to "navigation.sidebar.statusMonitor"; ensure
these new keys match the existing entries in the locale files so translations
continue to resolve correctly.
In `@web/default/src/features/pricing/components/model-details.tsx`:
- Around line 503-509: The wrapContent helper in model-details.tsx is a plain
function; make it consistent with Pricing by memoizing it with
React.useCallback: replace the plain wrapContent declaration with a useCallback
that depends on props.embedded (and any other referenced props), keeping the
same return logic that returns children or
<PublicLayout>{children}</PublicLayout>; ensure the identifier wrapContent
remains the same so callers (inline in render) continue to work.
In `@web/default/src/features/pricing/index.tsx`:
- Around line 159-178: The JSX contains a nested ternary using
filteredModels.length, isMobile and viewMode (VIEW_MODES.LIST) to choose between
VirtualModelList and PricingTable; extract that logic into a small helper or
precomputed variable (e.g., renderModels or getModelsComponent) that returns the
appropriate component given filteredModels, isMobile, viewMode and props like
handleModelClick, priceRate, usdExchangeRate, tokenUnit and showRechargePrice,
then use that single variable in the JSX return to remove the 2-level nested
ternary and improve readability.
In `@web/default/src/features/wallet/components/affiliate-rewards-card.tsx`:
- Line 66: Memoize the discount tiers by wrapping the getDiscountTiers call in a
useMemo so it only recalculates when its inputs change: replace const
discountTiers = getDiscountTiers(props.topupInfo, priceRatio) with a useMemo
that depends on props.topupInfo and priceRatio, and add useMemo to the component
imports; reference getDiscountTiers and the discountTiers constant so the memo
uses those inputs.
- Around line 20-45: The getDiscountTiers function is fine but its result should
be memoized in the component to avoid recalculating on every render: replace the
direct call that assigns discountTiers with a useMemo wrapper that calls
getDiscountTiers(props.topupInfo, priceRatio) and list props.topupInfo and
priceRatio as dependencies; ensure React's useMemo is imported and the memoized
variable keeps the name discountTiers so existing references remain valid.
In `@web/default/src/routes/_authenticated/status-monitor.tsx`:
- Around line 16-20: The iframe embedding STATUS_MONITOR_URL should be hardened:
update the JSX for the <iframe> (the element using STATUS_MONITOR_URL and title
'Status Monitor') to include a restrictive sandbox attribute (e.g.,
sandbox="allow-scripts" only if scripts are required, otherwise sandbox with no
allowances) and set a stricter referrerPolicy such as
referrerPolicy="no-referrer" (or "strict-origin-when-cross-origin" if you need
some referrer). Ensure you do not include allow-same-origin unless absolutely
necessary and avoid broad allowances like allow-popups unless required.
🪄 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
Run ID: dc78ea73-8649-40dc-a84f-b5894e9e6db1
⛔ Files ignored due to path filters (4)
web/default/public/favicon-custom.icois excluded by!**/*.icoweb/default/public/favicon.icois excluded by!**/*.icoweb/default/public/logo-custom.pngis excluded by!**/*.pngweb/default/public/logo.pngis excluded by!**/*.png
📒 Files selected for processing (48)
.gitignorecommon/constants.gocontroller/custom_oauth.gocontroller/twofa.gocontroller/user.godocs/private-deploy-sop.mdmiddleware/auth.gomodel/user.goweb/default/index.htmlweb/default/src/components/layout/components/nav-group.tsxweb/default/src/components/layout/components/top-nav.tsxweb/default/src/components/layout/components/workspace-switcher.tsxweb/default/src/components/layout/types.tsweb/default/src/components/profile-dropdown.tsxweb/default/src/custom/site.tsweb/default/src/features/dashboard/components/overview/summary-cards.tsxweb/default/src/features/dashboard/hooks/use-dashboard-config.tsxweb/default/src/features/keys/components/api-key-group-combobox.tsxweb/default/src/features/keys/components/api-keys-mutate-drawer.tsxweb/default/src/features/keys/components/api-request-url-card.tsxweb/default/src/features/keys/constants.tsweb/default/src/features/keys/index.tsxweb/default/src/features/keys/lib/api-key-form.tsweb/default/src/features/keys/lib/index.tsweb/default/src/features/pricing/components/model-details.tsxweb/default/src/features/pricing/components/pricing-table.tsxweb/default/src/features/pricing/hooks/use-filters.tsweb/default/src/features/pricing/index.tsxweb/default/src/features/wallet/components/affiliate-rewards-card.tsxweb/default/src/features/wallet/components/dialogs/payment-confirm-dialog.tsxweb/default/src/features/wallet/components/recharge-form-card.tsxweb/default/src/features/wallet/index.tsxweb/default/src/features/wallet/lib/format.tsweb/default/src/hooks/use-sidebar-data.tsweb/default/src/hooks/use-top-nav-links.tsweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.jsonweb/default/src/lib/constants.tsweb/default/src/lib/roles.tsweb/default/src/routeTree.gen.tsweb/default/src/routes/_authenticated/model-square/$modelId/index.tsxweb/default/src/routes/_authenticated/model-square/index.tsxweb/default/src/routes/_authenticated/status-monitor.tsxweb/default/src/routes/_authenticated/system-settings/route.tsx
Comment on lines
+175
to
+177
| func HasRootPermission(role int) bool { | ||
| return role >= RoleAdminUser | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Prefer explicit privileged-role checks over numeric threshold comparison.
Line 176 grants root-capable access to any future role value above admin. An explicit role allow-list avoids accidental privilege expansion.
💡 Suggested fix
func HasRootPermission(role int) bool {
- return role >= RoleAdminUser
+ return role == RoleAdminUser || role == RoleRootUser
}📝 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.
Suggested change
| func HasRootPermission(role int) bool { | |
| return role >= RoleAdminUser | |
| } | |
| func HasRootPermission(role int) bool { | |
| return role == RoleAdminUser || role == RoleRootUser | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@common/constants.go` around lines 175 - 177, The HasRootPermission function
currently uses a numeric threshold (role >= RoleAdminUser) which can
accidentally grant new future roles root access; update HasRootPermission to
check explicitly against an allow-list of privileged role constants (for example
compare role == RoleAdminUser or role == RoleRoot or use a switch/map of allowed
roles) so only explicitly named constants are treated as root-capable; modify
the function body to perform explicit equality checks (or a lookup in a set)
against those role identifiers instead of a >= comparison.
Comment on lines
+547
to
552
| if myRole <= originUser.Role && !common.HasRootPermission(myRole) { | ||
| common.ApiErrorI18n(c, i18n.MsgUserNoPermissionHigherLevel) | ||
| return | ||
| } | ||
| if myRole <= updatedUser.Role && myRole != common.RoleRootUser { | ||
| if myRole <= updatedUser.Role && !common.HasRootPermission(myRole) { | ||
| common.ApiErrorI18n(c, i18n.MsgUserCannotCreateHigherLevel) |
Contributor
There was a problem hiding this comment.
Keep root-account mutations root-only.
These guards now reuse HasRootPermission, but these endpoints do more than grant private/admin access: they can assign user.Role directly and hard-delete privileged accounts. If HasRootPermission includes admins, an admin can create/update a root user here and delete the current root account, bypassing the stricter promote flow that still tops out at RoleAdminUser.
Suggested tightening
- if myRole <= originUser.Role && !common.HasRootPermission(myRole) {
+ if myRole <= originUser.Role && myRole != common.RoleRootUser {
common.ApiErrorI18n(c, i18n.MsgUserNoPermissionHigherLevel)
return
}
- if myRole <= updatedUser.Role && !common.HasRootPermission(myRole) {
+ if myRole <= updatedUser.Role && myRole != common.RoleRootUser {
common.ApiErrorI18n(c, i18n.MsgUserCannotCreateHigherLevel)
return
}
- if user.Role >= myRole && !common.HasRootPermission(myRole) {
+ if user.Role >= myRole && myRole != common.RoleRootUser {
common.ApiErrorI18n(c, i18n.MsgUserCannotCreateHigherLevel)
return
}Also applies to: 752-753, 803-805
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/user.go` around lines 547 - 552, The guards currently allow anyone
with common.HasRootPermission (which may include admins) to assign or delete
root-level accounts; tighten them to root-only by replacing checks like "if
myRole <= originUser.Role && !common.HasRootPermission(myRole)" and the similar
updatedUser check with a strict root-only check (e.g., "if myRole <=
originUser.Role && myRole != common.RoleRoot" or use a dedicated helper like
common.IsRoot(myRole) that returns true only for the root role), and apply the
same change at the other occurrences around the file (the blocks you noted at
lines ~752-753 and ~803-805) so only the root account can be
created/updated/hard-deleted.
Comment on lines
+207
to
+225
| {(() => { | ||
| const { pathname, search } = splitUrl(String(subItem.url)) | ||
|
|
||
| return ( | ||
| <Link | ||
| to={pathname} | ||
| search={ | ||
| search | ||
| ? Object.fromEntries(new URLSearchParams(search)) | ||
| : undefined | ||
| } | ||
| onClick={() => setOpenMobile(false)} | ||
| > | ||
| {subItem.icon && <subItem.icon />} | ||
| <span>{subItem.title}</span> | ||
| {subItem.badge && <NavBadge>{subItem.badge}</NavBadge>} | ||
| </Link> | ||
| ) | ||
| })()} |
Contributor
There was a problem hiding this comment.
Apply the same URL-splitting fix to collapsed submenus.
This branch now normalizes subItem.url, but SidebarMenuCollapsedDropdown below still passes sub.url straight into <Link>. Any submenu item with search params, like ?view=table, will still behave differently when the sidebar is collapsed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/components/layout/components/nav-group.tsx` around lines 207
- 225, The collapsed submenu (SidebarMenuCollapsedDropdown) still passes sub.url
directly into the Link, so replicate the splitUrl normalization used above:
inside SidebarMenuCollapsedDropdown (where sub.url is used) call
splitUrl(String(sub.url)) to get { pathname, search }, pass pathname to Link's
to prop and, if search is non-empty, pass Object.fromEntries(new
URLSearchParams(search)) to the Link's search prop, and preserve behavior like
closing mobile via setOpenMobile(false) and rendering sub.icon, sub.title and
NavBadge as before.
Comment on lines
+18
to
+25
| function splitHref(href: string) { | ||
| const [pathname, search = ''] = href.split('?') | ||
|
|
||
| return { | ||
| pathname, | ||
| search, | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
splitHref truncates valid query strings containing additional ?.
Using href.split('?') splits all occurrences, so values like next=/path?tab=1 are cut off. Split only at the first ?.
🔧 Proposed fix
function splitHref(href: string) {
- const [pathname, search = ''] = href.split('?')
+ const queryIndex = href.indexOf('?')
+ if (queryIndex === -1) {
+ return {
+ pathname: href,
+ search: '',
+ }
+ }
+
+ const pathname = href.slice(0, queryIndex)
+ const search = href.slice(queryIndex + 1)
return {
pathname,
search,
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/components/layout/components/top-nav.tsx` around lines 18 -
25, splitHref currently uses href.split('?') which splits on every '?' and can
truncate query values that contain '?' (e.g., next=/path?tab=1); update
splitHref to only split at the first '?' by finding the first index (e.g.,
href.indexOf('?')) and then slice pathname and search accordingly (when index
=== -1 return pathname=href, search=''), ensuring the function name splitHref is
modified to use this single-split logic.
Comment on lines
+20
to
+24
| if (typeof window !== 'undefined') { | ||
| return window.location.origin | ||
| } | ||
|
|
||
| return '' |
Contributor
There was a problem hiding this comment.
Don't use the console origin as the API URL fallback.
On private or split deployments, window.location.origin can point at the frontend host while requests actually go to a different configured backend. That makes the copied URL wrong in exactly the setups this card is supposed to clarify. Prefer the shared API client's configured base URL before falling back to the page origin.
As per coding guidelines: Use the project's unified api instance (containing baseURL, headers, withCredentials: true); GET requests use default deduplication; special requests can disable via configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/features/keys/components/api-request-url-card.tsx` around
lines 20 - 24, The component currently falls back to window.location.origin for
the API URL; instead, import and use the project's unified api client to derive
the configured base URL (e.g., read api.defaults.baseURL or api.getUri()/config
on your api instance) and only fall back to window.location.origin if that
configured base URL is missing; update the component in api-request-url-card.tsx
to reference the api instance (import the existing shared api) and return its
base URL as the primary source for the displayed/copyable API URL.
Comment on lines
69
to
+73
| const handleRowClick = useCallback( | ||
| (model: PricingModel) => { | ||
| navigate({ | ||
| to: '/pricing/$modelId', | ||
| params: { modelId: model.model_name }, | ||
| search: (prev) => prev, | ||
| }) | ||
| onModelClick(model.model_name || '') | ||
| }, | ||
| [navigate] | ||
| [onModelClick] |
Contributor
There was a problem hiding this comment.
Avoid invoking click handler with an empty model id.
Fallbacking to '' can trigger invalid navigation targets; guard and return early when model_name is missing.
Suggested patch
const handleRowClick = useCallback(
(model: PricingModel) => {
- onModelClick(model.model_name || '')
+ if (!model.model_name) return
+ onModelClick(model.model_name)
},
[onModelClick]
)📝 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.
Suggested change
| const handleRowClick = useCallback( | |
| (model: PricingModel) => { | |
| navigate({ | |
| to: '/pricing/$modelId', | |
| params: { modelId: model.model_name }, | |
| search: (prev) => prev, | |
| }) | |
| onModelClick(model.model_name || '') | |
| }, | |
| [navigate] | |
| [onModelClick] | |
| const handleRowClick = useCallback( | |
| (model: PricingModel) => { | |
| if (!model.model_name) return | |
| onModelClick(model.model_name) | |
| }, | |
| [onModelClick] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/features/pricing/components/pricing-table.tsx` around lines
69 - 73, handleRowClick currently calls onModelClick(model.model_name || '')
which can pass an empty string; update handleRowClick to guard for a missing
model.model_name and return early (do not call onModelClick) when
model.model_name is falsy, ensuring you only invoke onModelClick with a valid
model id (use the model.model_name symbol and the onModelClick callback to
locate the code).
Comment on lines
+89
to
+94
| ...customSidebarLinks.map((link) => ({ | ||
| title: t(link.titleKey), | ||
| url: link.url, | ||
| icon: link.icon, | ||
| newTab: link.newTab, | ||
| })), |
Contributor
There was a problem hiding this comment.
Dynamic t(link.titleKey) can bypass i18n key extraction.
Because keys are runtime values, scanner-based extraction may miss these titles (e.g., newly added sidebar labels). Please register these keys in web/default/src/i18n/static-keys.ts (or otherwise ensure literal extraction coverage).
As per coding guidelines web/default/src/i18n/**: “Register new i18n keys in src/i18n/static-keys.ts or ensure text appears as t('...') string literals for extraction scanning to avoid missed translations”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/hooks/use-sidebar-data.ts` around lines 89 - 94,
useSidebarData is calling t(link.titleKey) with dynamic keys (via
customSidebarLinks) which can evade scanner-based i18n extraction; update the
i18n registry by adding each dynamic title key to
web/default/src/i18n/static-keys.ts (or convert the dynamic keys to literal
t('...') usages where possible) so the scanner picks them up—locate the
customSidebarLinks usage in use-sidebar-data.ts and ensure every link.titleKey
value is present in static-keys.ts (or replaced with explicit t('literal.key')).
| // System Configuration Defaults | ||
| export const DEFAULT_SYSTEM_NAME = 'New API' | ||
| export const DEFAULT_LOGO = '/logo.png' | ||
| export const DEFAULT_LOGO = '/logo-custom.png' |
Contributor
There was a problem hiding this comment.
Align remaining hardcoded fallback logo path with the new default.
DEFAULT_LOGO now points to /logo-custom.png, but web/default/src/components/layout/components/footer.tsx (Line 62) still falls back to '/logo.png'. This can produce inconsistent branding or a broken fallback if the old asset is removed.
Suggested follow-up patch (outside this file)
-const displayLogo = systemLogo || props.logo || '/logo.png'
+const displayLogo = systemLogo || props.logo || DEFAULT_LOGO+import { DEFAULT_LOGO } from '@/lib/constants'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/lib/constants.ts` at line 7, The footer component still uses
a hardcoded fallback '/logo.png' which is inconsistent with the new DEFAULT_LOGO
constant; update web/default/src/components/layout/components/footer.tsx to
import DEFAULT_LOGO from lib/constants and use that constant (or replace the
hardcoded '/logo.png' with DEFAULT_LOGO) wherever the fallback logo path is used
(e.g., the image src fallback at the fallback logic around line 62) so the
footer and DEFAULT_LOGO remain synchronized.
Comment on lines
+22
to
24
| if ((role ?? DEFAULT_ROLE) >= ROLE.ADMIN) return ROLE_LABEL_KEYS[ROLE.SUPER_ADMIN] | ||
|
|
||
| return ROLE_LABEL_KEYS[role as RoleValue] ?? ROLE_LABEL_KEYS[DEFAULT_ROLE] |
Contributor
There was a problem hiding this comment.
Admin role is mislabeled as “Super Admin”.
Line 22 currently maps all roles >= ROLE.ADMIN to the super-admin label key, so normal admins display the wrong role label.
💡 Suggested fix
export function getRoleLabelKey(role?: number): string {
- if ((role ?? DEFAULT_ROLE) >= ROLE.ADMIN) return ROLE_LABEL_KEYS[ROLE.SUPER_ADMIN]
-
- return ROLE_LABEL_KEYS[role as RoleValue] ?? ROLE_LABEL_KEYS[DEFAULT_ROLE]
+ const resolvedRole = (role ?? DEFAULT_ROLE) as RoleValue
+ return ROLE_LABEL_KEYS[resolvedRole] ?? ROLE_LABEL_KEYS[DEFAULT_ROLE]
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/lib/roles.ts` around lines 22 - 24, The role-to-label mapping
currently treats any role >= ROLE.ADMIN as SUPER_ADMIN; change the conditional
to check for ROLE.SUPER_ADMIN instead and return the SUPER_ADMIN label key only
for that threshold. In the block using (role ?? DEFAULT_ROLE) and
ROLE_LABEL_KEYS, replace the comparison >= ROLE.ADMIN with >= ROLE.SUPER_ADMIN
and ensure you return ROLE_LABEL_KEYS[ROLE.SUPER_ADMIN]; keep the final return
using ROLE_LABEL_KEYS[role as RoleValue] ?? ROLE_LABEL_KEYS[DEFAULT_ROLE]
unchanged.
| <div className='bg-background min-h-0 flex-1 overflow-hidden rounded-xl border'> | ||
| <iframe | ||
| src={STATUS_MONITOR_URL} | ||
| title='Status Monitor' |
Contributor
There was a problem hiding this comment.
Localize the iframe title string.
title='Status Monitor' is user-facing text and should go through translations to stay consistent across locales.
Suggested patch
+import { useTranslation } from 'react-i18next'
import { createFileRoute } from '@tanstack/react-router'
import { AppHeader, Main } from '@/components/layout'
@@
function StatusMonitor() {
+ const { t } = useTranslation()
return (
@@
<iframe
src={STATUS_MONITOR_URL}
- title='Status Monitor'
+ title={t('Status Monitor')}
className='h-full w-full border-0'
/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/routes/_authenticated/status-monitor.tsx` at line 18, Replace
the hard-coded iframe title string with a localized value: import and call
useTranslation() in the component that renders the iframe in status-monitor.tsx,
replace title='Status Monitor' with title={t('statusMonitor.title', 'Status
Monitor')}, and add the corresponding translation key ("statusMonitor.title") to
your i18n resource files; ensure useTranslation() is invoked (e.g., const { t }
= useTranslation()) before using t().
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/default/src/features/dashboard/components/overview/summary-cards.tsx (1)
30-38: ⚡ Quick winInconsistent translation key casing for pluralization.
The singular keys use capitalized names (
'Day','Hour','Minute') while plural use lowercase ('days','hours','minutes'). This inconsistency may lead to missing translations and violates the guideline for consistent naming.Consider using i18next's built-in pluralization support:
♻️ Suggested refactor using i18next pluralization
- if (days > 0) { - parts.push(`${days} ${t(days === 1 ? 'Day' : 'days')}`) - } - if (hours > 0) { - parts.push(`${hours} ${t(hours === 1 ? 'Hour' : 'hours')}`) - } - if (minutes > 0 || parts.length === 0) { - parts.push(`${minutes} ${t(minutes === 1 ? 'Minute' : 'minutes')}`) - } + if (days > 0) { + parts.push(`${days} ${t('day', { count: days })}`) + } + if (hours > 0) { + parts.push(`${hours} ${t('hour', { count: hours })}`) + } + if (minutes > 0 || parts.length === 0) { + parts.push(`${minutes} ${t('minute', { count: minutes })}`) + }Then define keys in locale JSON:
{ "day": "day", "day_other": "days", "hour": "hour", "hour_other": "hours", "minute": "minute", "minute_other": "minutes" }As per coding guidelines: "Use hierarchical, semantically clear translation keys with consistent naming."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/default/src/features/dashboard/components/overview/summary-cards.tsx` around lines 30 - 38, The translation keys used in the duration formatter are inconsistently cased and may miss pluralization rules; update the calls to t(...) in summary-cards.tsx (the three uses inside the conditional blocks that push into parts) to use consistent, lowercase keys and i18next pluralization (e.g., t('day', { count: days }), t('hour', { count: hours }), t('minute', { count: minutes })) and add the corresponding plural keys to locale JSON (e.g., "day" and "day_other", etc.) so singular/plural are handled consistently and reliably.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/default/src/features/dashboard/components/overview/summary-cards.tsx`:
- Around line 124-126: Replace the hardcoded "Live" string with an i18n key
using the t() function: import and call useTranslation() in the component (e.g.,
const { t } = useTranslation()) and update the div that currently renders "Live"
to render t('dashboard.live') (or another agreed key) so the label is
translated; also ensure the translation key is added to the appropriate locale
JSONs.
---
Nitpick comments:
In `@web/default/src/features/dashboard/components/overview/summary-cards.tsx`:
- Around line 30-38: The translation keys used in the duration formatter are
inconsistently cased and may miss pluralization rules; update the calls to
t(...) in summary-cards.tsx (the three uses inside the conditional blocks that
push into parts) to use consistent, lowercase keys and i18next pluralization
(e.g., t('day', { count: days }), t('hour', { count: hours }), t('minute', {
count: minutes })) and add the corresponding plural keys to locale JSON (e.g.,
"day" and "day_other", etc.) so singular/plural are handled consistently and
reliably.
🪄 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
Run ID: b3f3e4d3-635c-43e3-b0b9-72aed040e111
📒 Files selected for processing (1)
web/default/src/features/dashboard/components/overview/summary-cards.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/default/src/features/dashboard/components/overview/uptime-panel.tsx`:
- Around line 117-125: The code uses unchecked assertions for status?.start_time
and status?.data?.start_time which can produce NaN/invalid dates; update the
startTime computation (the variable named startTime used by runtimeCard and
produced from useStatus()) to validate the value is a finite number (e.g.,
typeof === 'number' && Number.isFinite(value')) before using it, otherwise fall
back to undefined so formatUptimeDuration and formatTimestampToDate receive only
valid numbers or trigger the existing 'Unknown' fallback; adjust the runtimeCard
creation to rely on this validated startTime.
- Around line 45-55: Replace the English-only plural logic and the hardcoded
"Live" by using i18next count-based translations: instead of conditional strings
like `${days} ${t(days === 1 ? 'Day' : 'days')}`, call t with a count (e.g.,
t('uptime.units.day', { count: days })) and same for hours/minutes (use
days/hours/minutes variables as the count); build the human duration from those
translated pieces and pass it into a single translated sentence using
interpolation (e.g., t('uptime.since', { duration: parts.join(' ') })); also
replace the hardcoded "Live" text with t('uptime.live') and apply the same
changes for the other occurrence referenced (lines ~169-179) so all user-facing
text uses t() and i18next pluralization/interpolation.
- Around line 231-234: PanelWrapper is being passed empty and emptyMessage which
causes it to short-circuit and hide the runtime card; remove the empty and
emptyMessage props from the PanelWrapper usage in uptime-panel.tsx so the inline
conditional that checks groups.length === 0 controls the empty-state rendering
and the runtime card (the JSX block rendering RuntimeCard / "No uptime
monitoring configured" message) remains visible; update the PanelWrapper
invocation to only pass the remaining props (e.g., title, subtitle, actions) and
verify RuntimeCard and the inline conditional render as intended.
🪄 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
Run ID: e0bfdb09-fbe3-4f98-9a10-525edd1015a1
📒 Files selected for processing (1)
web/default/src/features/dashboard/components/overview/uptime-panel.tsx
Comment on lines
+45
to
+55
| if (days > 0) { | ||
| parts.push(`${days} ${t(days === 1 ? 'Day' : 'days')}`) | ||
| } | ||
| if (hours > 0) { | ||
| parts.push(`${hours} ${t(hours === 1 ? 'Hour' : 'hours')}`) | ||
| } | ||
| if (minutes > 0 || parts.length === 0) { | ||
| parts.push(`${minutes} ${t(minutes === 1 ? 'Minute' : 'minutes')}`) | ||
| } | ||
|
|
||
| return parts.join(' ') |
Contributor
There was a problem hiding this comment.
Finish internationalizing the new runtime labels.
Live is still hardcoded, and the duration text is pluralized with English-specific conditionals. That leaves untranslated UI and incorrect grammar in locales with non-binary plural rules. Use i18next count-based keys/interpolation for the duration units and the “uptime since” sentence.
Suggested direction
- parts.push(`${days} ${t(days === 1 ? 'Day' : 'days')}`)
+ parts.push(t('dashboard.overview.uptime.day', { count: days }))
- parts.push(`${hours} ${t(hours === 1 ? 'Hour' : 'hours')}`)
+ parts.push(t('dashboard.overview.uptime.hour', { count: hours }))
- parts.push(`${minutes} ${t(minutes === 1 ? 'Minute' : 'minutes')}`)
+ parts.push(t('dashboard.overview.uptime.minute', { count: minutes }))
- Live
+ {t('dashboard.overview.uptime.live')}
- {t('Uptime since')} {runtimeCard.since}
+ {t('dashboard.overview.uptime.since', { since: runtimeCard.since })}As per coding guidelines "All user-facing text must support i18n using useTranslation() and the t() function in React components" and "Frontend i18n: Use i18next + react-i18next + i18next-browser-languagedetector."
Also applies to: 169-179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/features/dashboard/components/overview/uptime-panel.tsx`
around lines 45 - 55, Replace the English-only plural logic and the hardcoded
"Live" by using i18next count-based translations: instead of conditional strings
like `${days} ${t(days === 1 ? 'Day' : 'days')}`, call t with a count (e.g.,
t('uptime.units.day', { count: days })) and same for hours/minutes (use
days/hours/minutes variables as the count); build the human duration from those
translated pieces and pass it into a single translated sentence using
interpolation (e.g., t('uptime.since', { duration: parts.join(' ') })); also
replace the hardcoded "Live" text with t('uptime.live') and apply the same
changes for the other occurrence referenced (lines ~169-179) so all user-facing
text uses t() and i18next pluralization/interpolation.
Comment on lines
+117
to
+125
| const startTime = | ||
| (status?.start_time as number | undefined) ?? | ||
| (status?.data?.start_time as number | undefined) | ||
|
|
||
| const runtimeCard = useMemo( | ||
| () => ({ | ||
| value: formatUptimeDuration(startTime, nowMs, t), | ||
| since: startTime ? formatTimestampToDate(startTime) : t('Unknown'), | ||
| }), |
Contributor
There was a problem hiding this comment.
Guard start_time before formatting it.
These as number assertions hide the fact that useStatus() is fed by a loosely typed payload. If start_time ever arrives as anything unexpected, this path renders NaN duration text and an invalid date instead of falling back cleanly.
Suggested fix
- const startTime =
- (status?.start_time as number | undefined) ??
- (status?.data?.start_time as number | undefined)
+ const rawStartTime = status?.start_time ?? status?.data?.start_time
+ const startTime =
+ typeof rawStartTime === 'number' && Number.isFinite(rawStartTime)
+ ? rawStartTime
+ : undefined📝 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.
Suggested change
| const startTime = | |
| (status?.start_time as number | undefined) ?? | |
| (status?.data?.start_time as number | undefined) | |
| const runtimeCard = useMemo( | |
| () => ({ | |
| value: formatUptimeDuration(startTime, nowMs, t), | |
| since: startTime ? formatTimestampToDate(startTime) : t('Unknown'), | |
| }), | |
| const rawStartTime = status?.start_time ?? status?.data?.start_time | |
| const startTime = | |
| typeof rawStartTime === 'number' && Number.isFinite(rawStartTime) | |
| ? rawStartTime | |
| : undefined | |
| const runtimeCard = useMemo( | |
| () => ({ | |
| value: formatUptimeDuration(startTime, nowMs, t), | |
| since: startTime ? formatTimestampToDate(startTime) : t('Unknown'), | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/default/src/features/dashboard/components/overview/uptime-panel.tsx`
around lines 117 - 125, The code uses unchecked assertions for
status?.start_time and status?.data?.start_time which can produce NaN/invalid
dates; update the startTime computation (the variable named startTime used by
runtimeCard and produced from useStatus()) to validate the value is a finite
number (e.g., typeof === 'number' && Number.isFinite(value')) before using it,
otherwise fall back to undefined so formatUptimeDuration and
formatTimestampToDate receive only valid numbers or trigger the existing
'Unknown' fallback; adjust the runtimeCard creation to rely on this validated
startTime.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
New Features
Refactor
Chores