Skip to content

Commit

Permalink
SceneTimeRange: Set weekstart when evaluating time range (#1007)
Browse files Browse the repository at this point in the history
  • Loading branch information
torkelo authored Dec 18, 2024
1 parent a5e7915 commit 76d9a3a
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 49 deletions.
4 changes: 3 additions & 1 deletion packages/scenes/src/components/VizPanel/VizPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -265,13 +265,15 @@ export class VizPanel<TOptions = {}, TFieldConfig extends {} = {}> extends Scene
public getTimeRange = (data?: PanelData) => {
const liveNowTimer = sceneGraph.findObject(this, (o) => o instanceof LiveNowTimer);
const sceneTimeRange = sceneGraph.getTimeRange(this);

if (liveNowTimer instanceof LiveNowTimer && liveNowTimer.isEnabled) {
return evaluateTimeRange(
sceneTimeRange.state.from,
sceneTimeRange.state.to,
sceneTimeRange.getTimeZone(),
sceneTimeRange.state.fiscalYearStartMonth,
sceneTimeRange.state.UNSAFE_nowDelay
sceneTimeRange.state.UNSAFE_nowDelay,
sceneTimeRange.state.weekStart
);
}

Expand Down
13 changes: 5 additions & 8 deletions packages/scenes/src/core/SceneTimeRange.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { toUtc, dateMath, setWeekStart } from '@grafana/data';
import { toUtc, dateMath } from '@grafana/data';
import { SceneFlexItem, SceneFlexLayout } from '../components/layout/SceneFlexLayout';
import { PanelBuilders } from './PanelBuilders';
import { SceneTimeRange } from './SceneTimeRange';
Expand All @@ -8,7 +8,6 @@ import { SceneReactObject } from '../components/SceneReactObject';

jest.mock('@grafana/data', () => ({
...jest.requireActual('@grafana/data'),
setWeekStart: jest.fn(),
}));

function simulateDelay(newDateString: string, scene: EmbeddedScene) {
Expand All @@ -24,13 +23,11 @@ describe('SceneTimeRange', () => {
expect(timeRange.state.value.raw.from).toBe('now-1h');
});

it('When weekStart i set should call on activation', () => {
const timeRange = new SceneTimeRange({ from: 'now-1h', to: 'now', weekStart: 'saturday' });
const deactivate = timeRange.activate();
expect(setWeekStart).toHaveBeenCalledWith('saturday');
it('When weekStart use it when evaluting time range', () => {
const timeRange = new SceneTimeRange({ from: 'now/w', to: 'now/w', weekStart: 'saturday' });
const weekDay = timeRange.state.value.from.isoWeekday();

deactivate();
expect(setWeekStart).toHaveBeenCalledWith('monday');
expect(weekDay).toBe(6);
});

it('when time range refreshed should evaluate and update value', async () => {
Expand Down
42 changes: 11 additions & 31 deletions packages/scenes/src/core/SceneTimeRange.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ export class SceneTimeRange extends SceneObjectBase<SceneTimeRangeState> impleme
public constructor(state: Partial<SceneTimeRangeState> = {}) {
const from = state.from && isValid(state.from) ? state.from : 'now-6h';
const to = state.to && isValid(state.to) ? state.to : 'now';

const timeZone = state.timeZone;
const value = evaluateTimeRange(
from,
to,
timeZone || getTimeZone(),
state.fiscalYearStartMonth,
state.UNSAFE_nowDelay
state.UNSAFE_nowDelay,
state.weekStart
);
const refreshOnActivate = state.refreshOnActivate ?? { percent: 10 };
super({ from, to, timeZone, value, refreshOnActivate, ...state });
Expand All @@ -40,25 +40,13 @@ export class SceneTimeRange extends SceneObjectBase<SceneTimeRangeState> impleme
this._subs.add(
timeZoneSource.subscribeToState((n, p) => {
if (n.timeZone !== undefined && n.timeZone !== p.timeZone) {
this.setState({
value: evaluateTimeRange(
this.state.from,
this.state.to,
timeZoneSource.getTimeZone(),
this.state.fiscalYearStartMonth,
this.state.UNSAFE_nowDelay
),
});
this.refreshRange(0);
}
})
);
}
}

if (this.state.weekStart) {
setWeekStart(this.state.weekStart);
}

if (rangeUtil.isRelativeTimeRange(this.state.value.raw)) {
this.refreshIfStale();
}
Expand Down Expand Up @@ -116,14 +104,13 @@ export class SceneTimeRange extends SceneObjectBase<SceneTimeRangeState> impleme
this.state.to,
this.state.timeZone ?? getTimeZone(),
this.state.fiscalYearStartMonth,
this.state.UNSAFE_nowDelay
this.state.UNSAFE_nowDelay,
this.state.weekStart
);

const diff = value.to.diff(this.state.value.to, 'milliseconds');
if (diff >= refreshAfterMs) {
this.setState({
value,
});
this.setState({ value });
}
}

Expand Down Expand Up @@ -168,7 +155,8 @@ export class SceneTimeRange extends SceneObjectBase<SceneTimeRangeState> impleme
update.to,
this.getTimeZone(),
this.state.fiscalYearStartMonth,
this.state.UNSAFE_nowDelay
this.state.UNSAFE_nowDelay,
this.state.weekStart
);

// Only update if time range actually changed
Expand All @@ -186,16 +174,7 @@ export class SceneTimeRange extends SceneObjectBase<SceneTimeRangeState> impleme
};

public onRefresh = () => {
this.setState({
value: evaluateTimeRange(
this.state.from,
this.state.to,
this.getTimeZone(),
this.state.fiscalYearStartMonth,
this.state.UNSAFE_nowDelay
),
});

this.refreshRange(0);
this.publishEvent(new RefreshEvent(), true);
};

Expand Down Expand Up @@ -252,7 +231,8 @@ export class SceneTimeRange extends SceneObjectBase<SceneTimeRangeState> impleme
update.to ?? this.state.to,
update.timeZone ?? this.getTimeZone(),
this.state.fiscalYearStartMonth,
this.state.UNSAFE_nowDelay
this.state.UNSAFE_nowDelay,
this.state.weekStart
);

return this.setState(update);
Expand Down
16 changes: 10 additions & 6 deletions packages/scenes/src/core/SceneTimeZoneOverride.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ export class SceneTimeZoneOverride
timeRange.to,
this.state.timeZone,
timeRange.fiscalYearStartMonth,
timeRange.UNSAFE_nowDelay
timeRange.UNSAFE_nowDelay,
timeRange.weekStart
),
});
}
Expand All @@ -42,14 +43,17 @@ export class SceneTimeZoneOverride
}

public onTimeZoneChange(timeZone: string): void {
const parentTimeRange = this.getAncestorTimeRange();

this.setState({
timeZone,
value: evaluateTimeRange(
this.state.from,
this.state.to,
this.state.timeZone,
this.getAncestorTimeRange().state.fiscalYearStartMonth,
this.state.UNSAFE_nowDelay
parentTimeRange.state.from,
parentTimeRange.state.to,
timeZone,
parentTimeRange.state.fiscalYearStartMonth,
parentTimeRange.state.UNSAFE_nowDelay,
parentTimeRange.state.weekStart
),
});
}
Expand Down
20 changes: 17 additions & 3 deletions packages/scenes/src/utils/evaluateTimeRange.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
import { dateMath, DateTime, DateTimeInput, TimeRange } from '@grafana/data';
import { dateMath, DateTime, DateTimeInput, setWeekStart, TimeRange } from '@grafana/data';
import { TimeZone } from '@grafana/schema';

export function evaluateTimeRange(
from: string | DateTime,
to: string | DateTime,
timeZone: TimeZone,
fiscalYearStartMonth?: number,
delay?: string
fiscalYearStartMonth: number | undefined,
delay: string | undefined,
weekStart: string | undefined
): TimeRange {
const hasDelay = delay && to === 'now';
const now = Date.now();

if (weekStart) {
setWeekStartIfDifferent(weekStart);
}

/** This tries to use dateMath.toDateTime if available, otherwise falls back to dateMath.parse.
* Using dateMath.parse can potentially result in to and from being calculated using two different timestamps.
* If two different timestamps are used, the time range "now-24h to now" will potentially be 24h +- number of milliseconds it takes between calculations.
Expand Down Expand Up @@ -51,3 +56,12 @@ export function evaluateTimeRange(
},
};
}

let prevWeekStart: string | undefined;

function setWeekStartIfDifferent(weekStart: string) {
if (weekStart !== prevWeekStart) {
prevWeekStart = weekStart;
setWeekStart(weekStart);
}
}

0 comments on commit 76d9a3a

Please sign in to comment.