Add dashboard time range switcher and custom picker#3383
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds interactive dashboard chart time-range controls (quick presets + custom date-time), threads range state/handlers through Dashboard and useDashboardData, persists/validates chart-range, updates quota loading to accept overrides, and adds i18n keys for range labels. Changes
Sequence DiagramsequenceDiagram
participant User as "User"
participant ChartsPanel as "ChartsPanel"
participant Dashboard as "Dashboard"
participant Hook as "useDashboardData"
participant API as "Quota API"
User->>ChartsPanel: select preset / open custom picker / confirm
ChartsPanel->>Dashboard: handleRangePresetChange(preset) / handleCustomRangeConfirm()
Dashboard->>Hook: applyChartRangePreset(preset) / applyCustomRange()
Hook->>Hook: update inputs, dataExportDefaultTime, activeRangePreset, customRangeDraft
Hook->>API: loadQuotaData(overrideInputs, overrideDefaultTime)
API-->>Hook: return data
Hook-->>Dashboard: updated chart data
Dashboard-->>ChartsPanel: pass activeRangePreset, customRangeDraft, chart data
ChartsPanel->>User: render updated range UI and charts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/src/components/dashboard/index.jsx`:
- Around line 105-135: The preset and custom-range handlers currently call
dashboardData.refresh() which triggers a full dashboard reload (including
uptime) and races unrelated requests; change both handleRangePresetChange and
handleCustomRangeConfirm to avoid dashboardData.refresh(): after getting
rangeState from dashboardData.applyChartRangePreset(...) or
dashboardData.applyCustomRange(), call the chart-only fetch method (e.g.,
dashboardData.fetchChartData or dashboardData.getChartData) with
rangeState.nextInputs and rangeState.nextDefaultTime, then pass the returned
data to dashboardCharts.updateChartData(...); keep dashboardData.refresh()
reserved only for the explicit dashboard-wide refresh button.
In `@web/src/hooks/dashboard/useDashboardData.js`:
- Around line 325-332: The synthetic empty-state point is being stamped with
created_at: now.getTime() / 1000 which places the fake point at the current time
and stretches the x-axis; change that to a timestamp inside the requested range
(use the existing range bounds used for the query—e.g., rangeStart/rangeEnd or
from/to variables in this hook—such as the midpoint ((start + end)/2) or the
range start) when constructing the fallback object before calling setQuotaData;
also remove the const now = new Date() declaration above if it becomes unused
after this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d626e84-1052-4b1a-b8a2-836c66d0db62
⛔ Files ignored due to path filters (1)
docs/pr-assets/dashboard-time-range-90d.pngis excluded by!**/*.png
📒 Files selected for processing (10)
web/src/components/dashboard/ChartsPanel.jsxweb/src/components/dashboard/index.jsxweb/src/hooks/dashboard/useDashboardData.jsweb/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-CN.jsonweb/src/i18n/locales/zh-TW.json
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/src/hooks/dashboard/useDashboardData.js (1)
35-38: Document the 1-hour buffer in the end timestamp calculation.The
+ 3600adds 1 hour to the current time, which I assume is intentional buffering to ensure recent data isn't clipped. Consider adding a brief comment explaining this design choice, or extracting it as a named constant.📝 Suggested improvement
+ const END_TIME_BUFFER_SECONDS = 3600; // Buffer to include recently arrived data + const getCurrentEndTimestamp = useCallback( - () => timestamp2string(new Date().getTime() / 1000 + 3600), + () => timestamp2string(new Date().getTime() / 1000 + END_TIME_BUFFER_SECONDS), [], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/dashboard/useDashboardData.js` around lines 35 - 38, The getCurrentEndTimestamp useCallback currently adds a magic +3600 seconds to Now (timestamp2string(new Date().getTime() / 1000 + 3600)); document this 1-hour buffer or extract it as a named constant (e.g., ONE_HOUR_BUFFER_SECONDS = 3600) and replace the literal, and add a short comment above getCurrentEndTimestamp explaining that the extra hour is an intentional buffer to avoid clipping recent data; reference the getCurrentEndTimestamp function and timestamp2string to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web/src/hooks/dashboard/useDashboardData.js`:
- Around line 35-38: The getCurrentEndTimestamp useCallback currently adds a
magic +3600 seconds to Now (timestamp2string(new Date().getTime() / 1000 +
3600)); document this 1-hour buffer or extract it as a named constant (e.g.,
ONE_HOUR_BUFFER_SECONDS = 3600) and replace the literal, and add a short comment
above getCurrentEndTimestamp explaining that the extra hour is an intentional
buffer to avoid clipping recent data; reference the getCurrentEndTimestamp
function and timestamp2string to locate the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 06faef02-1d24-41f0-a308-ae7255b40b57
📒 Files selected for processing (2)
web/src/components/dashboard/index.jsxweb/src/hooks/dashboard/useDashboardData.js
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/src/hooks/dashboard/useDashboardData.js (1)
330-339: Normalizedatabefore storing it in state.The empty-state
push()andsort()happen aftersetQuotaData(data), so the array already stored in React state is mutated outside the setter. BuildnextDatafirst, then set and return that value once.♻️ Proposed fix
if (success) { - setQuotaData(data); - if (data.length === 0) { - data.push({ - count: 0, - model_name: '无数据', - quota: 0, - created_at: emptyStateTimestamp, - }); - } - data.sort((a, b) => a.created_at - b.created_at); - return data; + const nextData = + data.length === 0 + ? [ + { + count: 0, + model_name: '无数据', + quota: 0, + created_at: emptyStateTimestamp, + }, + ] + : [...data]; + nextData.sort((a, b) => a.created_at - b.created_at); + setQuotaData(nextData); + return nextData; } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/dashboard/useDashboardData.js` around lines 330 - 339, The current code calls setQuotaData(data) before mutating the same array (pushing empty-state item and sorting), which mutates React state after setting it; instead, create a new array (e.g., nextData) from data, perform the empty-state push and sort on nextData, then call setQuotaData(nextData) and return/use nextData; reference the setQuotaData call and the local data variable in your fix and ensure you never mutate the passed-in data array in-place.
🤖 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/src/hooks/dashboard/useDashboardData.js`:
- Around line 277-295: The applyCustomRange function currently only checks for
presence of start_timestamp and end_timestamp but still commits reversed or
zero-length ranges; update applyCustomRange to validate that start_timestamp and
end_timestamp are numeric and that start_timestamp < end_timestamp (and
optionally that end - start > 0) before calling setInputs,
setDataExportDefaultTime and setActiveRangePreset('custom'); if the validation
fails, do not mutate state (return null) and optionally surface an
error/validation flag to the UI. Keep checks tied to customRangeDraft and update
the dependency array accordingly.
- Around line 55-65: The initial range uses a raw now while presets use a
buffered end timestamp, causing a ~3600s mismatch; update the code so
getInitialTimestamp() uses the same buffered end-time that
applyChartRangePreset() uses (either by changing getInitialTimestamp to accept
an endTimestamp parameter or by computing the initial start_timestamp using the
buffered end timestamp before calling getInitialTimestamp), and ensure
detectQuickRangePreset()/getQuickRangeConfig() comparisons still operate on that
aligned start/end pair so the preset detection and initial load produce
identical ranges.
---
Nitpick comments:
In `@web/src/hooks/dashboard/useDashboardData.js`:
- Around line 330-339: The current code calls setQuotaData(data) before mutating
the same array (pushing empty-state item and sorting), which mutates React state
after setting it; instead, create a new array (e.g., nextData) from data,
perform the empty-state push and sort on nextData, then call
setQuotaData(nextData) and return/use nextData; reference the setQuotaData call
and the local data variable in your fix and ensure you never mutate the
passed-in data array in-place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8f06c65-608b-44a6-b0dc-3666e5b61164
📒 Files selected for processing (1)
web/src/hooks/dashboard/useDashboardData.js
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/src/hooks/dashboard/useDashboardData.js`:
- Around line 79-88: The current logic only persists granularity
(data_export_default_time) causing ambiguity between presets (e.g., 7d vs 30d
both map to "day"); update persistence so the active preset or explicit window
is stored and restored: when applyChartRangePreset() runs, save a clear
identifier for the preset (e.g., preset key like "7d"/"30d"/"90d") or the
concrete start_timestamp and end_timestamp in addition to granularity; then
update the restore path used by initialChartRange (and the similar restore code
at the other locations mentioned) to prefer restoring the saved preset
identifier (or exact start/end) before falling back to
getDefaultTime/getInitialTimestamp, ensuring the dashboard reconstructs the
exact user-selected window rather than inferring from granularity alone.
- Around line 323-367: The loadQuotaData hook currently assumes API.get(url)
always resolves; wrap the await API.get(url) call in a try/catch so
network/non-2xx rejections are handled inside loadQuotaData: on catch call
showError with the caught error (or its message), ensure you return [] and still
call setLoading(false) (the finally can remain) and avoid letting the rejection
bubble to refresh/click handlers; keep updating setQuotaData only on success and
reference loadQuotaData, API.get, showError, setQuotaData and setLoading when
making the change.
- Around line 349-353: The empty-state row is hardcoding the Chinese label '无数据'
inside the nextData.push in useDashboardData.js; import and call the i18n hook
(useTranslation) in this hook/component and replace the literal with t('无数据') so
the label is localized (ensure useTranslation is referenced at top and t is used
where nextData is built, e.g., inside the code path that pushes the empty row).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 71b30141-557c-42f2-9988-e4c1ef17bf85
📒 Files selected for processing (3)
web/src/components/dashboard/ChartsPanel.jsxweb/src/helpers/dashboard.jsxweb/src/hooks/dashboard/useDashboardData.js
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/src/helpers/dashboard.jsx (1)
84-86: Validate (or clear) on write, not only on read.
setStoredChartRangecurrently persists any payload, including invalid/null shapes, and relies on later reads to clean it up. Prefer enforcing the invariant at write-time.Proposed refactor
export const setStoredChartRange = (range) => { - localStorage.setItem(STORAGE_KEYS.DASHBOARD_CHART_RANGE, JSON.stringify(range)); + if (!isValidStoredChartRange(range)) { + localStorage.removeItem(STORAGE_KEYS.DASHBOARD_CHART_RANGE); + return; + } + localStorage.setItem( + STORAGE_KEYS.DASHBOARD_CHART_RANGE, + JSON.stringify(range), + ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/helpers/dashboard.jsx` around lines 84 - 86, setStoredChartRange currently writes any value (including null/invalid shapes) to STORAGE_KEYS.DASHBOARD_CHART_RANGE; change it to validate the incoming range before persisting and clear the stored key when invalid. In the setStoredChartRange function, check that the provided range matches the expected shape (e.g. required properties and types) and only call localStorage.setItem when valid; if range is null/invalid, call localStorage.removeItem(STORAGE_KEYS.DASHBOARD_CHART_RANGE) (or set a safe default) so subsequent reads don’t need to sanitize bad data.
🤖 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/src/helpers/dashboard.jsx`:
- Around line 117-122: In getInitialChartRange, don't assign raw
getDefaultTime() to default_time; instead pass the result through the same
normalization/validation used elsewhere (e.g., call the existing
normalizeDefaultTime/validateDefaultTime helper) before returning. Update the
return object so default_time is set to the normalized value
(normalizeDefaultTime(getDefaultTime())), ensuring getInitialChartRange,
getDefaultTime, getInitialTimestamp and the returned default_time use the same
validation path to avoid emitting invalid defaults from stale/corrupt storage.
- Around line 96-99: The code uses Date.parse on endTimestamp (used in
parsedEndTimestamp/baseTimestamp) which can silently fail for the "YYYY-MM-DD
HH:mm:ss" produced by timestamp2string(); normalize or explicitly parse that
format before calling Date.parse. Update the logic around
parsedEndTimestamp/baseTimestamp (and the provider
getCurrentEndTimestamp()/timestamp2string()) to either convert the "YYYY-MM-DD
HH:mm:ss" to strict ISO (replace the space with 'T' and append timezone like 'Z'
or local offset) or implement an explicit parser that extracts
year/month/day/hour/minute/second and builds a UTC timestamp (e.g., via
Date.UTC) so Date.parse won't rely on browser-dependent behavior and won't
silently fall back to new Date().
---
Nitpick comments:
In `@web/src/helpers/dashboard.jsx`:
- Around line 84-86: setStoredChartRange currently writes any value (including
null/invalid shapes) to STORAGE_KEYS.DASHBOARD_CHART_RANGE; change it to
validate the incoming range before persisting and clear the stored key when
invalid. In the setStoredChartRange function, check that the provided range
matches the expected shape (e.g. required properties and types) and only call
localStorage.setItem when valid; if range is null/invalid, call
localStorage.removeItem(STORAGE_KEYS.DASHBOARD_CHART_RANGE) (or set a safe
default) so subsequent reads don’t need to sanitize bad data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6b16f07a-2957-4237-8528-22f750b9713e
📒 Files selected for processing (3)
web/src/constants/dashboard.constants.jsweb/src/helpers/dashboard.jsxweb/src/hooks/dashboard/useDashboardData.js
✅ Files skipped from review due to trivial changes (1)
- web/src/constants/dashboard.constants.js
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/hooks/dashboard/useDashboardData.js
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/src/helpers/dashboard.jsx (1)
54-59: Derive valid presets from the config map.
VALID_RANGE_PRESETSnow repeats the same preset names already declared inDASHBOARD_QUICK_RANGE_CONFIGS, and the preset loop inweb/src/hooks/dashboard/useDashboardData.jscarries a third copy. That drift will be easy to miss the next time a preset is added.♻️ Proposed refactor
-const VALID_RANGE_PRESETS = new Set(['24h', '7d', '30d', '90d', 'custom']); +const VALID_RANGE_PRESETS = new Set([ + ...Object.keys(DASHBOARD_QUICK_RANGE_CONFIGS), + 'custom', +]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/helpers/dashboard.jsx` around lines 54 - 59, VALID_RANGE_PRESETS is hardcoded and duplicates presets defined in DASHBOARD_QUICK_RANGE_CONFIGS (and a third copy exists in useDashboardData.js); replace the hardcoded Set with one derived from the config map so presets stay in sync: change VALID_RANGE_PRESETS to new Set(Object.keys(DASHBOARD_QUICK_RANGE_CONFIGS)) in web/src/helpers/dashboard.jsx, keep getDashboardQuickRangeConfig as-is, and update any code (e.g., the preset loop in useDashboardData.js) to rely on this derived VALID_RANGE_PRESETS instead of duplicating preset names.
🤖 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/src/helpers/dashboard.jsx`:
- Around line 107-112: The predicate currently uses "!preset" which falsely
rejects only null/undefined; update it to only allow an absent preset or a known
preset string by replacing the preset check with a null/undefined check and a
string-type/whitelist check: use "preset == null || (typeof preset === 'string'
&& VALID_RANGE_PRESETS.has(preset))" in the same return expression that
references startTime, endTime, default_time, normalizeDefaultTime and
VALID_RANGE_PRESETS so corrupted values like '', 0, or false won’t be treated as
missing.
---
Nitpick comments:
In `@web/src/helpers/dashboard.jsx`:
- Around line 54-59: VALID_RANGE_PRESETS is hardcoded and duplicates presets
defined in DASHBOARD_QUICK_RANGE_CONFIGS (and a third copy exists in
useDashboardData.js); replace the hardcoded Set with one derived from the config
map so presets stay in sync: change VALID_RANGE_PRESETS to new
Set(Object.keys(DASHBOARD_QUICK_RANGE_CONFIGS)) in
web/src/helpers/dashboard.jsx, keep getDashboardQuickRangeConfig as-is, and
update any code (e.g., the preset loop in useDashboardData.js) to rely on this
derived VALID_RANGE_PRESETS instead of duplicating preset names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 58f310f8-66f6-47e8-acc8-f2c0e4f4cb54
📒 Files selected for processing (10)
web/src/components/dashboard/ChartsPanel.jsxweb/src/components/dashboard/index.jsxweb/src/constants/dashboard.constants.jsweb/src/helpers/dashboard.jsxweb/src/helpers/render.jsxweb/src/hooks/dashboard/useDashboardData.jsweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.json
✅ Files skipped from review due to trivial changes (1)
- web/src/i18n/locales/vi.json
🚧 Files skipped from review as they are similar to previous changes (5)
- web/src/components/dashboard/index.jsx
- web/src/i18n/locales/fr.json
- web/src/i18n/locales/ja.json
- web/src/components/dashboard/ChartsPanel.jsx
- web/src/hooks/dashboard/useDashboardData.js
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/src/hooks/dashboard/useDashboardData.js (1)
372-383: Consider validating timestamps before building the API URL.If
parseDashboardTimestampreturnsNaNfor malformed inputs, the API URL will includestart_timestamp=NaN&end_timestamp=NaN. WhileemptyStateTimestamphas a fallback, the actual API request proceeds with invalid parameters.Since callers like
handleSearchConfirmandapplyCustomRangevalidate ranges before callingrefresh, this is likely a minor edge case, but adding a guard here would makeloadQuotaDatamore defensive.🛡️ Optional defensive check
let localStartTimestamp = parseDashboardTimestamp(start_timestamp) / 1000; let localEndTimestamp = parseDashboardTimestamp(end_timestamp) / 1000; + if (!Number.isFinite(localStartTimestamp) || !Number.isFinite(localEndTimestamp)) { + showError(t('请求参数无效')); + setLoading(false); + return []; + } const emptyStateTimestamp = Number.isFinite(localStartTimestamp) ? localStartTimestamp : 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/dashboard/useDashboardData.js` around lines 372 - 383, Validate that parseDashboardTimestamp yielded finite numbers before interpolating into the API URL: check Number.isFinite(localStartTimestamp) and Number.isFinite(localEndTimestamp) (computed from parseDashboardTimestamp) and if either is invalid, replace them with emptyStateTimestamp or omit the start/end params when building url in the loadQuotaData/refresh flow; update the conditional that sets url (the branches using isAdminUser) to use these guarded values so the constructed URL never contains "NaN" for start_timestamp or end_timestamp.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web/src/hooks/dashboard/useDashboardData.js`:
- Around line 372-383: Validate that parseDashboardTimestamp yielded finite
numbers before interpolating into the API URL: check
Number.isFinite(localStartTimestamp) and Number.isFinite(localEndTimestamp)
(computed from parseDashboardTimestamp) and if either is invalid, replace them
with emptyStateTimestamp or omit the start/end params when building url in the
loadQuotaData/refresh flow; update the conditional that sets url (the branches
using isAdminUser) to use these guarded values so the constructed URL never
contains "NaN" for start_timestamp or end_timestamp.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b11830ea-4ce2-4a6f-9145-7ae857026ce5
📒 Files selected for processing (2)
web/src/helpers/dashboard.jsxweb/src/hooks/dashboard/useDashboardData.js
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/helpers/dashboard.jsx
|
Latest branch head already includes the invalid timestamp guard in loadQuotaData. @coderabbitai review |
|
✅ Actions performedReview triggered.
|
CodeRabbit follow-up fixes已调整 dashboard 时间范围切换的数据刷新流:
已验证:
commit: |
e9db5e8 to
cbdcb3f
Compare
This comment was marked as spam.
This comment was marked as spam.
f0a3de9 to
015d66a
Compare
015d66a to
1c32491
Compare
|
同步当前整理结果:
|
|
这条 PR 先不再按当前形态继续推进。 原因:上游 dashboard 时间范围相关已有持续演进,但没有采用这里的具体实现。当前 PR 体量偏大,不适合继续挂着。 后续处理:如果继续推进,会改成更小的 dashboard feature 补丁再提。当前这条 PR 先关闭归档。 |
|
关闭当前大 PR,后续按更小补丁重提。 |

📝 变更描述 / Description
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
📸 运行证明 / Proof of Work
cd web && bunx eslint src/components/dashboard/ChartsPanel.jsx src/components/dashboard/index.jsx src/hooks/dashboard/useDashboardData.jscd web && bun run build24h / 7d / 30d / 90d / custom交互与 Apply 行为