Copilot commented on code in PR #537:
URL:
https://github.com/apache/skywalking-booster-ui/pull/537#discussion_r3007374556
##########
src/constants/data.ts:
##########
@@ -16,6 +16,7 @@
*/
export enum TimeType {
+ SENCOND_TIME = "SECOND",
Review Comment:
`TimeType` enum member name `SENCOND_TIME` appears to be a typo ("sencond").
To match existing naming (`MINUTE_TIME`, `HOUR_TIME`, ...), rename this to
`SECOND_TIME` and update all references to avoid baking the misspelling into
the public constants API.
```suggestion
SECOND_TIME = "SECOND",
```
##########
src/hooks/useDuration.ts:
##########
@@ -43,7 +45,7 @@ export function useDuration() {
}
function setDurationRow(data: Duration) {
const appStore = useAppStoreWithOut();
- durationRow = { ...data, coldStage: appStore.coldStageMode };
+ durationRow = { ...data, coldStage: appStore.coldStageMode, step:
TimeType.SENCOND_TIME };
}
Review Comment:
`useDuration` now hard-codes `step` to SECOND in `getDuration()`,
`getDurationTime()`, and even overwrites the caller-provided `data.step` in
`setDurationRow()`. This changes the hook contract (callers pass a step from
`timeFormat()` and tests expect it to be preserved) and can break any flow that
relies on MINUTE/HOUR/DAY behavior. Consider only defaulting to SECOND when the
incoming step is missing/empty, or make the desired step an explicit
parameter/option so existing behavior can remain intact where needed.
##########
src/views/dashboard/related/trace/components/TraceList/SearchBar.vue:
##########
@@ -132,7 +133,7 @@ limitations under the License. -->
const { duration: filtersDuration } = props.data.filters || {};
const duration = ref<DurationTime>(
filtersDuration
- ? { start: filtersDuration.startTime || "", end: filtersDuration.endTime
|| "", step: filtersDuration.step || "" }
+ ? { start: filtersDuration.startTime || "", end: filtersDuration.endTime
|| "", step: TimeType.SENCOND_TIME }
: appStore.durationTime,
Review Comment:
When `filtersDuration` is not provided, this still initializes `duration`
from `appStore.durationTime`, whose `step` comes from the global duration
(typically MINUTE/HOUR/DAY). If the goal is to ensure Trace queries always use
SECOND step, this path will continue sending a non-SECOND `queryDuration.step`.
Consider deriving the default duration via `useDuration().getDurationTime()`
(or otherwise formatting start/end with SECOND) so both branches use a
consistent step.
```suggestion
: { ...appStore.durationTime, step: TimeType.SENCOND_TIME },
```
##########
src/views/dashboard/related/trace/components/TraceList/Filter.vue:
##########
@@ -152,7 +153,7 @@ limitations under the License. -->
}
async function init() {
- duration.value = filters.value.duration || appStore.durationTime;
+ duration.value = filters.value.duration || getDurationTime();
Review Comment:
`init()` assigns `duration.value = filters.value.duration ||
getDurationTime();`. If `filters.value.duration` came from URL/widget state and
lacks `step` (or uses `startTime/endTime` shape), this bypasses the
normalization done above and can reintroduce an empty/non-SECOND step in
`queryDuration`. Consider normalizing `filters.value.duration` here (mapping
`startTime/endTime` to `start/end` and defaulting `step` to SECOND) to keep
behavior consistent.
```suggestion
const baseDuration = getDurationTime();
if (filters.value.duration) {
const rawDuration: any = filters.value.duration as any;
const {
start,
end,
startTime,
endTime,
...rest
} = rawDuration;
const normalizedDuration: Duration = {
// start from defaults (including default step, typically SECOND)
...baseDuration,
// spread any additional/overriding fields from filters
...rest,
// normalize start/end, giving priority to explicit start/end, then
startTime/endTime
start: start ?? startTime ?? baseDuration.start,
end: end ?? endTime ?? baseDuration.end,
};
duration.value = normalizedDuration;
} else {
duration.value = baseDuration;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
