From 7d02bedc5f0a40116a639b084edd5f0a2a0fe4b7 Mon Sep 17 00:00:00 2001 From: 2GT-Rich <107219997+2GT-Rich@users.noreply.github.com> Date: Wed, 2 Sep 2026 08:18:08 -0700 Subject: [PATCH] feat: schedule in the operator's timezone, and make Top Talkers searchable (v0.23.21-beta) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #117. **Scheduled work ran on the container's clock.** cronMatchesNow used Date's own accessors, which read the process timezone — UTC in the shipped image. A backup set for 02:00 therefore fired at 02:00 UTC wherever the operator was: seven hours early on the US west coast, and on the wrong day entirely for anyone far enough east. The settings page said "server time", which was accurate and precisely the problem. Reported by a user managing devices across regions, who described it as a configuration gap; it is a correctness bug. There is now an app_timezone setting, chosen from the zones the runtime knows and offered alongside the browser's own as a one-click default. Schedules are evaluated in it, and the page states the zone and the current time in it, so a wrong setting is obvious immediately rather than at 2am three days later. The matching logic moved to utils/cron.ts with 13 tests, because the bug was invisible where it lived: it only manifests when the process zone and the operator's differ, which never happens in a developer's own shell. The tests pin the cases that matter — the same instant matching in exactly one of three zones, a job on the 2nd UTC still being the 1st in Los Angeles, day-of-week resolved locally, and daylight saving handled by Intl rather than by a fixed offset. Zone handling goes through Intl for that last reason; offset arithmetic would be wrong twice a year. An unknown zone falls back to UTC rather than throwing, so a bad value cannot stop every schedule in the product. **Top Talkers was ten rows with no way to look anything up**, which is unusable on a network of any size — you could see the loudest ten and nothing else. It now filters on name, IP, MAC or vendor, and ranks 10, 25 or 50. Filtering is client-side because the rows are already present; asking the server per keystroke would be slower and no more accurate. Requested in the discussion thread, where the backend already served everything needed and only the UI was missing. Co-Authored-By: Claude Opus 5 --- README.md | 2 +- backend/package.json | 2 +- backend/src/db/migrate.ts | 4 + backend/src/services/PollerService.ts | 50 ++--------- backend/src/utils/__tests__/cron.test.ts | 94 ++++++++++++++++++++ backend/src/utils/cron.ts | 96 +++++++++++++++++++++ frontend/package.json | 2 +- frontend/src/pages/SettingsPage.tsx | 55 +++++++++++- frontend/src/pages/TrafficAnalyticsPage.tsx | 57 ++++++++++-- frontend/src/version.ts | 2 +- 10 files changed, 307 insertions(+), 57 deletions(-) create mode 100644 backend/src/utils/__tests__/cron.test.ts create mode 100644 backend/src/utils/cron.ts diff --git a/README.md b/README.md index 79cdd05..90fc9b6 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@

- Version + Version License Docker TypeScript diff --git a/backend/package.json b/backend/package.json index b2b6100..35b9983 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-backend", - "version": "0.23.20-beta", + "version": "0.23.21-beta", "description": "Mikrotik Network Management Platform - Backend API", "main": "dist/index.js", "scripts": { diff --git a/backend/src/db/migrate.ts b/backend/src/db/migrate.ts index b3244a1..40a0409 100644 --- a/backend/src/db/migrate.ts +++ b/backend/src/db/migrate.ts @@ -821,6 +821,10 @@ const DEFAULT_SETTINGS = [ { key: 'retention_events_days', value: 30 }, { key: 'backup_schedule_enabled', value: false }, { key: 'backup_schedule_cron', value: '0 2 * * *' }, + // Scheduled work was evaluated against the container clock, which is UTC in + // the shipped image. A "02:00" backup therefore fired at 02:00 UTC regardless + // of where the operator was — seven hours out on the US west coast (#117). + { key: 'app_timezone', value: 'UTC' }, { key: 'mac_scan_enabled', value: true }, { key: 'mac_scan_interval', value: 300 }, { key: 'reverse_dns_enabled', value: false }, diff --git a/backend/src/services/PollerService.ts b/backend/src/services/PollerService.ts index d01d3e8..c63fe7f 100644 --- a/backend/src/services/PollerService.ts +++ b/backend/src/services/PollerService.ts @@ -8,6 +8,7 @@ import { Point } from '@influxdata/influxdb-client'; import { alertService } from './AlertService'; import { runConfigHealth } from './changeGuard/configHealth'; import type { GuardDevice } from './changeGuard/ChangeGuard'; +import { cronMatches } from '../utils/cron'; // ─── Tuning ─────────────────────────────────────────────────────────────────── @@ -240,6 +241,8 @@ export class PollerService { const apScanIntervalHours = (appSettings['ap_scan_interval_hours'] as number) || 24; const backupScheduleEnabled = appSettings['backup_schedule_enabled'] === true; const backupScheduleCron = (appSettings['backup_schedule_cron'] as string) || '0 2 * * *'; + // Schedules are the operator's local time, not the container's (#117). + const appTimezone = (appSettings['app_timezone'] as string) || 'UTC'; const configSnapEnabled = appSettings['config_snapshot_enabled'] !== false; const configSnapIntervalMin = (appSettings['config_snapshot_interval_min'] as number) || 60; const configHealthEnabled = appSettings['config_health_enabled'] !== false; @@ -395,7 +398,7 @@ export class PollerService { // Scheduled backups — fire when the cron expression matches the current minute/hour. // Redis key with 1-hour TTL prevents double-firing within the same cron window. - if (backupScheduleEnabled && this.cronMatchesNow(backupScheduleCron)) { + if (backupScheduleEnabled && cronMatches(backupScheduleCron, new Date(), appTimezone)) { const backupKey = 'task:scheduled_backup'; const lastBackup = await this.getTimestamp(backupKey); if (now - lastBackup > 3_600_000) { @@ -419,7 +422,7 @@ export class PollerService { 'spectral_scan_interval_hours', 'ap_scan_enabled', 'ap_scan_interval_hours', 'backup_schedule_enabled', 'backup_schedule_cron', 'config_snapshot_enabled', - 'config_snapshot_interval_min')` + 'config_snapshot_interval_min', 'app_timezone')` ); const map: Record = {}; for (const row of rows) map[row.key] = row.value; @@ -429,49 +432,6 @@ export class PollerService { } } - // Returns true if the 5-part cron expression matches the current time. - // Evaluates all five fields — minute, hour, day-of-month, month, day-of-week — - // so weekly (e.g. `0 3 * * 0`) and monthly (`0 3 1 * *`) schedules fire only on - // the right day, not every day. Supports: *, exact numbers, comma lists, - // ranges (a-b), and step values (*/n). Day-of-month and day-of-week follow the - // standard cron rule: when both are restricted the job runs if either matches; - // when only one is restricted, only that one must match. - private cronMatchesNow(cron: string): boolean { - const parts = cron.trim().split(/\s+/); - if (parts.length < 5) return false; - const [minuteField, hourField, domField, monthField, dowField] = parts; - const now = new Date(); - const matchField = (field: string, val: number): boolean => { - if (field === '*') return true; - return field.split(',').some((f) => { - if (f.includes('/')) { - const [base, step] = f.split('/'); - const start = base === '*' ? 0 : Number(base); - return val >= start && (val - start) % Number(step) === 0; - } - if (f.includes('-')) { - const [lo, hi] = f.split('-').map(Number); - return val >= lo && val <= hi; - } - return Number(f) === val; - }); - }; - - if (!matchField(minuteField, now.getMinutes())) return false; - if (!matchField(hourField, now.getHours())) return false; - if (!matchField(monthField, now.getMonth() + 1)) return false; // cron months are 1-12 - - // Day-of-month (1-31) and day-of-week (0-6, Sun=0). getDay() returns 0 for Sunday. - const domRestricted = domField !== '*'; - const dowRestricted = dowField !== '*'; - const domMatch = matchField(domField, now.getDate()); - const dowMatch = matchField(dowField, now.getDay()); - let dayMatch: boolean; - if (!domRestricted && !dowRestricted) dayMatch = true; - else if (domRestricted && dowRestricted) dayMatch = domMatch || dowMatch; - else dayMatch = domRestricted ? domMatch : dowMatch; - return dayMatch; - } private async runScheduledBackups(): Promise { const { BackupService } = await import('./BackupService'); diff --git a/backend/src/utils/__tests__/cron.test.ts b/backend/src/utils/__tests__/cron.test.ts new file mode 100644 index 0000000..eb1fad5 --- /dev/null +++ b/backend/src/utils/__tests__/cron.test.ts @@ -0,0 +1,94 @@ +import { cronMatches, zonedParts, matchField } from '../cron'; + +// 2026-09-02T09:30:00Z — a Wednesday. +const AT = new Date('2026-09-02T09:30:00Z'); + +describe('zonedParts', () => { + it('reads wall-clock fields in the requested zone', () => { + expect(zonedParts(AT, 'UTC')).toEqual({ minute: 30, hour: 9, dom: 2, month: 9, dow: 3 }); + // UTC-7 in September. + expect(zonedParts(AT, 'America/Los_Angeles')).toEqual({ minute: 30, hour: 2, dom: 2, month: 9, dow: 3 }); + // UTC+3 in September. + expect(zonedParts(AT, 'Europe/Sofia')).toEqual({ minute: 30, hour: 12, dom: 2, month: 9, dow: 3 }); + }); + + it('rolls the date backwards across the dateline', () => { + // 01:00 UTC on the 2nd is still the 1st in Los Angeles. + const p = zonedParts(new Date('2026-09-02T01:00:00Z'), 'America/Los_Angeles'); + expect(p.dom).toBe(1); + expect(p.hour).toBe(18); + expect(p.dow).toBe(2); // Tuesday, not Wednesday + }); + + it('handles midnight as hour 0, never 24', () => { + expect(zonedParts(new Date('2026-09-02T00:00:00Z'), 'UTC').hour).toBe(0); + }); + + it('respects daylight saving rather than a fixed offset', () => { + const summer = zonedParts(new Date('2026-07-01T12:00:00Z'), 'America/New_York').hour; // EDT, -4 + const winter = zonedParts(new Date('2026-01-01T12:00:00Z'), 'America/New_York').hour; // EST, -5 + expect(summer).toBe(8); + expect(winter).toBe(7); + }); + + it('falls back to UTC for an unknown zone instead of throwing', () => { + expect(zonedParts(AT, 'Mars/Olympus_Mons')).toEqual(zonedParts(AT, 'UTC')); + }); +}); + +describe('matchField', () => { + it('handles wildcards, lists, ranges and steps', () => { + expect(matchField('*', 5)).toBe(true); + expect(matchField('5', 5)).toBe(true); + expect(matchField('1,3,5', 3)).toBe(true); + expect(matchField('1,3,5', 4)).toBe(false); + expect(matchField('1-5', 4)).toBe(true); + expect(matchField('1-5', 6)).toBe(false); + expect(matchField('*/15', 30)).toBe(true); + expect(matchField('*/15', 31)).toBe(false); + }); + + it('rejects a zero or nonsense step rather than dividing by it', () => { + expect(matchField('*/0', 5)).toBe(false); + expect(matchField('*/x', 5)).toBe(false); + }); +}); + +describe('cronMatches — the #117 bug', () => { + it('fires a 02:00 job at 02:00 local, not 02:00 UTC', () => { + // The exact failure: 09:30 UTC is 02:30 in Los Angeles. + const at0230 = new Date('2026-09-02T09:30:00Z'); + expect(cronMatches('30 2 * * *', at0230, 'America/Los_Angeles')).toBe(true); + expect(cronMatches('30 2 * * *', at0230, 'UTC')).toBe(false); + }); + + it('does not fire the same job twice in different zones', () => { + const at = new Date('2026-09-02T09:30:00Z'); + const zones = ['UTC', 'America/Los_Angeles', 'Europe/Sofia']; + const fired = zones.filter((z) => cronMatches('30 2 * * *', at, z)); + expect(fired).toEqual(['America/Los_Angeles']); + }); + + it('matches day-of-week in the local zone', () => { + // 01:00 UTC Wednesday is still Tuesday evening in Los Angeles. + const at = new Date('2026-09-02T01:00:00Z'); + expect(cronMatches('0 18 * * 2', at, 'America/Los_Angeles')).toBe(true); // Tue + expect(cronMatches('0 18 * * 3', at, 'America/Los_Angeles')).toBe(false); // Wed + }); + + it('runs when either dom or dow matches, per the standard rule', () => { + const at = new Date('2026-09-02T09:30:00Z'); // 2nd, a Wednesday, 09:30 UTC + expect(cronMatches('30 9 2 * 0', at, 'UTC')).toBe(true); // dom matches, dow does not + expect(cronMatches('30 9 15 * 3', at, 'UTC')).toBe(true); // dow matches, dom does not + expect(cronMatches('30 9 15 * 0', at, 'UTC')).toBe(false); // neither + }); + + it('requires all five fields', () => { + expect(cronMatches('30 9 * *', AT, 'UTC')).toBe(false); + expect(cronMatches('', AT, 'UTC')).toBe(false); + }); + + it('defaults to UTC when no zone is given', () => { + expect(cronMatches('30 9 * * *', AT)).toBe(true); + }); +}); diff --git a/backend/src/utils/cron.ts b/backend/src/utils/cron.ts new file mode 100644 index 0000000..296e044 --- /dev/null +++ b/backend/src/utils/cron.ts @@ -0,0 +1,96 @@ +/** + * Cron matching in a named timezone. + * + * Split out of PollerService because the bug it fixes was invisible in place: + * scheduling used `Date`'s own accessors, which read the *process* timezone. + * The shipped container runs UTC, so a "02:00" backup fired at 02:00 UTC no + * matter where the operator was — seven hours early on the US west coast, and + * on the wrong day for anyone far enough east (#117). + * + * Zone handling goes through Intl rather than offset arithmetic, which gets + * daylight saving right without a date library or a table of rules. + */ + +export interface ZonedParts { + minute: number; + hour: number; + /** Day of month, 1-31. */ + dom: number; + /** Month, 1-12 — cron's numbering, not JavaScript's. */ + month: number; + /** Day of week, 0-6 with Sunday as 0. */ + dow: number; +} + +const DOW: Record = { Sun: 0, Mon: 1, Tue: 2, Wed: 3, Thu: 4, Fri: 5, Sat: 6 }; + +/** Wall-clock fields of an instant in a given zone. Falls back to UTC if unknown. */ +export function zonedParts(date: Date, timeZone: string): ZonedParts { + try { + const parts = Object.fromEntries( + new Intl.DateTimeFormat('en-US', { + timeZone, hour12: false, + minute: '2-digit', hour: '2-digit', day: '2-digit', month: '2-digit', weekday: 'short', + }).formatToParts(date).map((p) => [p.type, p.value]) + ); + return { + minute: Number(parts.minute), + // Some environments render midnight as hour "24". + hour: Number(parts.hour) % 24, + dom: Number(parts.day), + month: Number(parts.month), + dow: DOW[parts.weekday] ?? 0, + }; + } catch { + return { + minute: date.getUTCMinutes(), hour: date.getUTCHours(), dom: date.getUTCDate(), + month: date.getUTCMonth() + 1, dow: date.getUTCDay(), + }; + } +} + +/** Does one cron field match a value? Supports `*`, lists, ranges and steps. */ +export function matchField(field: string, val: number): boolean { + if (field === '*') return true; + return field.split(',').some((f) => { + if (f.includes('/')) { + const [base, step] = f.split('/'); + const start = base === '*' ? 0 : Number(base); + const by = Number(step); + if (!Number.isFinite(by) || by <= 0) return false; + return val >= start && (val - start) % by === 0; + } + if (f.includes('-')) { + const [lo, hi] = f.split('-').map(Number); + return val >= lo && val <= hi; + } + return Number(f) === val; + }); +} + +/** + * Does a 5-field cron expression match this instant, read in `timeZone`? + * + * Day-of-month and day-of-week follow the standard rule: when both are + * restricted the job runs if *either* matches, not both. + */ +export function cronMatches(cron: string, at: Date, timeZone = 'UTC'): boolean { + const parts = cron.trim().split(/\s+/); + if (parts.length < 5) return false; + const [minuteField, hourField, domField, monthField, dowField] = parts; + const now = zonedParts(at, timeZone); + + if (!matchField(minuteField, now.minute)) return false; + if (!matchField(hourField, now.hour)) return false; + if (!matchField(monthField, now.month)) return false; + + const domRestricted = domField !== '*'; + const dowRestricted = dowField !== '*'; + const domMatch = matchField(domField, now.dom); + const dowMatch = matchField(dowField, now.dow); + + if (domRestricted && dowRestricted) return domMatch || dowMatch; + if (domRestricted) return domMatch; + if (dowRestricted) return dowMatch; + return true; +} diff --git a/frontend/package.json b/frontend/package.json index 8cb0c1f..b680d54 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-frontend", - "version": "0.23.20-beta", + "version": "0.23.21-beta", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/pages/SettingsPage.tsx b/frontend/src/pages/SettingsPage.tsx index 0deb84e..c9a9221 100644 --- a/frontend/src/pages/SettingsPage.tsx +++ b/frontend/src/pages/SettingsPage.tsx @@ -1,4 +1,4 @@ -import { useState, useRef } from 'react'; +import { useState, useRef, useMemo } from 'react'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import { Settings, Users, Key, Plus, Trash2, CheckCircle, AlertCircle, Pencil, X, @@ -51,6 +51,20 @@ export default function SettingsPage() { const { theme, setTheme } = useThemeStore(); const isAdmin = user?.role === 'admin'; const canWrite = user?.role !== 'viewer'; + // Timezone for scheduled work. The browser's zone is offered as a shortcut + // because it is almost always what the operator means. + const timezoneOptions = useMemo(() => { + const supported = (Intl as unknown as { supportedValuesOf?: (k: string) => string[] }).supportedValuesOf; + try { + return supported ? supported('timeZone') : ['UTC']; + } catch { + return ['UTC']; + } + }, []); + const browserTimezone = useMemo(() => { + try { return Intl.DateTimeFormat().resolvedOptions().timeZone; } catch { return ''; } + }, []); + const [activeTab, setActiveTab] = useState<'general' | 'users' | 'sso' | 'credentials' | 'security' | 'certificate' | 'alerting' | 'audit' | 'tags' | 'maintenance' | 'templates' | 'automation' | 'poller'>('general'); const [auditSearch, setAuditSearch] = useState(''); const [auditPage, setAuditPage] = useState(1); @@ -823,6 +837,15 @@ export default function SettingsPage() { const enabled = !!settings['backup_schedule_enabled']; const sched = parseBackupCron(settings['backup_schedule_cron'] as string); const controlsDisabled = !isAdmin || !enabled; + const appTimezone = (settings['app_timezone'] as string) || 'UTC'; + // Showing the clock in the chosen zone makes a wrong setting + // obvious immediately, rather than at 2am three days later. + let currentTimeInAppTz = ''; + try { + currentTimeInAppTz = new Intl.DateTimeFormat(undefined, { + timeZone: appTimezone, hour: '2-digit', minute: '2-digit', hour12: false, + }).format(new Date()); + } catch { currentTimeInAppTz = 'unknown zone'; } const apply = (next: typeof sched) => updateSettingsMutation.mutate({ backup_schedule_cron: scheduleToCron(next) }); return ( @@ -910,9 +933,37 @@ export default function SettingsPage() { + {/* Timezone belongs beside the schedule, because it is the + schedule's unit. Scheduled work used to be evaluated against + the container clock — UTC in the shipped image — so "02:00" + meant 02:00 UTC wherever you were (#117). */} +

+ + + {browserTimezone && browserTimezone !== appTimezone && ( + + )} +
+

{enabled - ? <>Runs {describeBackupSchedule(sched).toLowerCase()}, server time. + ? <>Runs {describeBackupSchedule(sched).toLowerCase()}{' '} + in {appTimezone} + {' '}(currently {currentTimeInAppTz}). : 'Enable to choose a backup schedule.'}

{sched.frequency === 'monthly' && ( diff --git a/frontend/src/pages/TrafficAnalyticsPage.tsx b/frontend/src/pages/TrafficAnalyticsPage.tsx index 26a4402..fd1be1d 100644 --- a/frontend/src/pages/TrafficAnalyticsPage.tsx +++ b/frontend/src/pages/TrafficAnalyticsPage.tsx @@ -54,6 +54,13 @@ function clientLabel(c: { mac: string; custom_name: string | null; hostname: str export default function TrafficAnalyticsPage() { const [range, setRange] = useState('24h'); + // Top Talkers was a fixed list of ten with no way to look anything up, which + // is unusable on a network of any size — you could see the loudest ten and + // nothing else. Filtering happens client-side because the rows are already + // here; asking the server again per keystroke would be slower and no more + // accurate. + const [clientQuery, setClientQuery] = useState(''); + const [topLimit, setTopLimit] = useState(10); const navigate = useNavigate(); const { data: series = [], isLoading: seriesLoading } = useQuery({ @@ -63,8 +70,8 @@ export default function TrafficAnalyticsPage() { }); const { data: topClients = [], isLoading: clientsLoading } = useQuery({ - queryKey: ['traffic-top-clients', range], - queryFn: () => trafficApi.topClients(range, 10).then(r => r.data), + queryKey: ['traffic-top-clients', range, topLimit], + queryFn: () => trafficApi.topClients(range, topLimit).then(r => r.data), refetchInterval: 60_000, }); @@ -88,6 +95,17 @@ export default function TrafficAnalyticsPage() { const totalAppBytes = apps.reduce((s, a) => s + a.bytes, 0); const maxClientBytes = topClients[0]?.total_bytes || 1; + // Match on anything the row actually shows — name, IP, MAC or vendor — so a + // search for "192.168.1." or "apple" both behave the way people expect. + const visibleClients = (() => { + const q = clientQuery.trim().toLowerCase(); + if (!q) return topClients; + return topClients.filter((c) => + [clientLabel(c), c.ip_address, c.mac, c.vendor] + .some((v) => v && String(v).toLowerCase().includes(q)) + ); + })(); + // Collector data-health: how much of the received traffic we could attribute const exporters = [...(status?.exporters || [])].sort((a, b) => b.flows - a.flows); const pktReceived = status?.packetsReceived || 0; @@ -223,15 +241,42 @@ export default function TrafficAnalyticsPage() {
{/* Top talkers */}
-
- -

Top Talkers

+
+
+ +

Top Talkers

+ + {clientQuery.trim() + ? `${visibleClients.length} of ${topClients.length}` + : `top ${topClients.length}`} + +
+
+ setClientQuery(e.target.value)} + placeholder="Filter by IP, name, MAC or vendor…" + className="input text-xs py-1 flex-1 min-w-0" + /> + +
{topClients.length === 0 ?
No client traffic recorded.
+ : visibleClients.length === 0 + ?
+ No client in the top {topClients.length} matches “{clientQuery.trim()}”. +
: (
- {topClients.map(c => { + {visibleClients.map(c => { const isReal = c.mac !== 'unknown' && c.mac !== 'other'; return (