Copilot commented on code in PR #521:
URL:
https://github.com/apache/skywalking-booster-ui/pull/521#discussion_r2780434081
##########
src/store/modules/app.ts:
##########
@@ -124,10 +127,10 @@ export const appStore = defineStore({
},
actions: {
setDuration(data: Duration): void {
- this.durationRow = data;
+ this.durationRow = { ...data, coldStage: this.coldStageMode };
},
updateDurationRow(data: Duration) {
- this.durationRow = data;
+ this.durationRow = { ...data, coldStage: this.coldStageMode };
},
Review Comment:
`setDuration` / `updateDurationRow` always overwrite `data.coldStage` with
`this.coldStageMode`. If callers start passing `coldStage` (now part of the
`Duration` type), their value will be ignored. Consider using `data.coldStage
?? this.coldStageMode` (or removing `coldStage` from the input type) to avoid
surprising behavior.
##########
src/store/modules/app.ts:
##########
@@ -62,7 +63,7 @@ export const appStore = defineStore({
reloadTimer: null,
allMenus: [],
theme: Themes.Dark,
- coldStageMode: false,
+ coldStageMode: InitializationDurationRow.coldStage || false,
Review Comment:
`InitializationDurationRow` is a mutable object (contains `Date`s) and is
assigned directly into Pinia state (`durationRow: InitializationDurationRow`).
This means different store instances (and tests) can end up sharing and
mutating the same `durationRow` reference. Prefer cloning when seeding state
(e.g., create a factory or spread/structuredClone) so each store gets an
independent object.
##########
src/hooks/useDuration.ts:
##########
@@ -28,18 +28,21 @@ export function useDuration() {
start: getLocalTime(appStore.utc, durationRow.start),
end: getLocalTime(appStore.utc, durationRow.end),
step: durationRow.step,
+ coldStage: appStore.coldStageMode,
};
}
function getDurationTime(): DurationTime {
- const { start, step, end } = getDuration();
+ const { start, step, end, coldStage } = getDuration();
return {
start: dateFormatStep(start, step, true),
end: dateFormatStep(end, step, true),
- step: step,
+ step,
+ coldStage,
};
}
function setDurationRow(data: Duration) {
- durationRow = data;
+ const appStore = useAppStoreWithOut();
+ durationRow = { ...data, coldStage: appStore.coldStageMode };
}
Review Comment:
In `useDuration`, `setDurationRow` writes `coldStage` onto the local
`durationRow`, but `getDuration()` ignores `durationRow.coldStage` and always
uses `appStore.coldStageMode`. This makes the stored `coldStage` redundant and
can confuse future callers. Either derive `coldStage` consistently from one
source (store mode vs. durationRow) or keep `durationRow.coldStage` and have
`getDuration()` prefer it when provided.
##########
src/layout/components/NavBar.vue:
##########
@@ -174,7 +174,20 @@ limitations under the License. -->
async function setTTL() {
await getMetricsTTL();
await getRecordsTTL();
- changeDataMode();
+ // Initialize TTL handling without triggering duration update
+ if (coldStage.value) {
+ handleMetricsTTL({
+ minute: appStore.metricsTTL?.coldMinute || NaN,
+ hour: appStore.metricsTTL?.coldHour || NaN,
+ day: appStore.metricsTTL?.coldDay || NaN,
+ });
+ } else {
+ handleMetricsTTL({
+ minute: appStore.metricsTTL?.minute || NaN,
+ hour: appStore.metricsTTL?.hour || NaN,
+ day: appStore.metricsTTL?.day || NaN,
Review Comment:
In `setTTL`, using `|| NaN` will treat valid TTL values of `0` as falsy and
replace them with `NaN`, which changes behavior in `handleMetricsTTL` (it will
clear `maxRange`). Use nullish coalescing (`??`) or an explicit `typeof ===
'number'` check so that `0` is preserved.
```suggestion
minute: appStore.metricsTTL?.coldMinute ?? NaN,
hour: appStore.metricsTTL?.coldHour ?? NaN,
day: appStore.metricsTTL?.coldDay ?? NaN,
});
} else {
handleMetricsTTL({
minute: appStore.metricsTTL?.minute ?? NaN,
hour: appStore.metricsTTL?.hour ?? NaN,
day: appStore.metricsTTL?.day ?? NaN,
```
--
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]
