[feat]: add schedule report log persistence - #42
Conversation
5468ccb to
3f555b8
Compare
3f555b8 to
5f376a3
Compare
| sinceTimestamp: number, | ||
| ): Promise<AdminActionLogEntry[]> { | ||
| const days = daysBetween(sinceTimestamp); | ||
| const perDay = await Promise.all( |
There was a problem hiding this comment.
ScheduleRecord.lastReportSentAt is a non-optional number with no default set anywhere, so the first report run after a schedule is created, before markSent has ever run, sees 0. Running the base-branch daysBetween directly
since=0 -> 20665 days (1970-01-01 … 2026-07-30)
since=undefined -> []
since=NaN -> []
That's a Promise.all of ~20k readByAssociation calls from inside a scheduled job. Suggest clamping the window:
const MAX_REPORT_WINDOW_DAYS = 31;
const days = daysBetween(sinceTimestamp).slice(-MAX_REPORT_WINDOW_DAYS);
…and initializing lastReportSentAt: Date.now() on schedule creation. The silent [] on undefined/NaN is the mirror hazard: a malformed timestamp produces an empty report rather than an error
There was a problem hiding this comment.
Added MAX_REPORT_WINDOW_DAYS = 31 and clamped it exactly as suggested: daysBetween(sinceTimestamp).slice(-MAX_REPORT_WINDOW_DAYS). Applied to both AdminActionLogStore.getActionsSince and FlagLogStore.getDailySummariesSince since they share the same pattern.
Also added a guard that throws on a non-finite sinceTimestamp instead of silently returning [] so a malformed timestamp surfaces as an error rather than a quietly-empty report, per your mirror-hazard note.
| MAX_DAILY_ACTIONS, | ||
| } from '../../constants/scheduleLogStore'; | ||
|
|
||
| export class AdminActionLogStore { |
There was a problem hiding this comment.
No retention or cleanup for day-keyed records. On a busy server, this is permanent linear growth in app persistence.
There was a problem hiding this comment.
Added record retention with pruning, and a bounded query window, plus the constants for both.
- Day-keyed records now have retention: each store keeps a small index of which days it has data for and prunes anything past DAY_RECORD_RETENTION_DAYS on write, so they don't grow forever.
- Added a cache so the pruning check only runs once per calendar day instead of on every write.
|
hey @scuciatto ! I've added all the suggestions given please take a look ! |
358e03f
into
RocketChat:feature/automated-schedule-reports
Description
Adds the persistence layer for the scheduled spam-report feature: storing
the active schedule, in-progress setup drafts, and the flag/admin-action
logs the daily report will summarize.
Changes
4 persistence layers were added
AdminActionLogStore
FlagLogStore
ScheduleDraftStorage
ScheduleStore
NOTE -
Known limitation: AdminActionLogStore.log and FlagLogStore.log do read-modify-write against a shared doc, and Apps-Engine's persistence API has no atomic increment/CAS — so concurrent writes to the same user/day can race and clobber each other.
For AdminActionLogStore, exposure is minimal (human-triggered, low frequency). FlagLogStore is the one actually worth thinking about, since the detector can fire multiple flags in a burst — a race there could undercount flagCount/trigger tallies in the daily summary. I considered switching it to append-only (one record per flag, no read), but that shifts cost to read time — the daily summary would need to reduce N raw events into a rollup on every report run instead of reading one doc, and FlagLogStore has meaningfully higher volume than admin actions, so that trade isn't obviously a win. Either way, this only affects report accuracy — flag levels, restriction actions, and enforcement are driven by UserStatusStore, not this log. Leaving as a documented limitation for now.