diff --git a/plans/260509-2317-per-group-warning-settings/phase-01-schema-threshold-resolver.md b/plans/260509-2317-per-group-warning-settings/phase-01-schema-threshold-resolver.md new file mode 100644 index 0000000..a3ebc65 --- /dev/null +++ b/plans/260509-2317-per-group-warning-settings/phase-01-schema-threshold-resolver.md @@ -0,0 +1,80 @@ +--- +phase: 1 +title: Schema + threshold resolver +status: completed +priority: P2 +effort: 30m +dependencies: [] +--- + +# Phase 1: Schema + threshold resolver + +## Overview + +Extend the per-group state with an optional `settings` object and add one helper that resolves the effective warning threshold. Replace direct `config.numDaysWarningNotUpdated` reads at all call sites. + +## Architecture + +**Storage shape (Upstash key `group:{chatId}`)** +```js +{ appleApps: [...], googleApps: [...], settings: { numDaysWarningNotUpdated?: number } } +``` +- `settings` is optional. Missing → behaves identically to today. +- Additive change; no migration needed. + +**Resolver (single source of truth)** +```js +// src/util/group-settings.js +export function resolveDaysWarning(group, config) { + const v = group?.settings?.numDaysWarningNotUpdated; + return Number.isFinite(v) && v > 0 ? v : config.numDaysWarningNotUpdated; +} +``` + +**Repository surface** +Add to `createGroupRepository`: +```js +async function setSetting(groupId, key, value) { + return mutateAndSave(groupId, (g) => { + g.settings ??= {}; + if (value === undefined || value === null) delete g.settings[key]; + else g.settings[key] = value; + return true; + }); +} +``` +- Generic so `/setdayswarning` and any future setter share one path. +- `emptyGroup()` does NOT add `settings: {}` — keep payload minimal; resolver tolerates absence. + +## Related Code Files + +**Create** +- `src/util/group-settings.js` — `resolveDaysWarning(group, config)` + +**Modify** +- `src/repository/group-repository.js` — add `setSetting` to public surface +- `src/scheduler/scheduler.js` — replace `config.numDaysWarningNotUpdated` reads (lines 39, 114) with `resolveDaysWarning(group, config)` +- `src/bot/commands/check-app.js` — replace `config.numDaysWarningNotUpdated` (line 15) with resolver + +## Implementation Steps + +1. Create `src/util/group-settings.js` with `resolveDaysWarning`. +2. In `group-repository.js`, add `setSetting` mutator + export it. +3. In `scheduler.js#checkGroup`, after `await store.group.getGroup(groupId)`, compute `const threshold = resolveDaysWarning(group, config);` — drop the `config.numDaysWarningNotUpdated` ref. +4. In `scheduler.js#buildReport`, accept the resolved threshold (pass it through) and use it in the header string instead of `config.numDaysWarningNotUpdated`. +5. In `check-app.js`, swap line 15 to `const threshold = resolveDaysWarning(group, config);`. +6. `node --check` all four touched files. + +## Success Criteria + +- [ ] Resolver returns env default when `group.settings` absent +- [ ] Resolver returns group override when valid integer > 0 stored +- [ ] Resolver ignores zero / negative / non-finite stored values (falls back) +- [ ] Scheduler + `/checkapp` use resolved value at every call site +- [ ] Daily report header shows the per-group threshold, not the global default + +## Risk Assessment + +- **Header drift:** scheduler builds report header with global default today. If we forget to thread the resolved value into `buildReport`, groups with overrides see a misleading header. Mitigation: pass the resolved threshold as a fn arg, not a closure. +- **Bad stored values:** if a future bug writes a string/null, resolver must not crash. Mitigation: `Number.isFinite && > 0` guard. +- **Schema rot:** `emptyGroup()` not adding `settings` keeps payload minimal but means every reader must tolerate undefined. Mitigation: only the resolver reads it; `setSetting` lazily inits. diff --git a/plans/260509-2317-per-group-warning-settings/phase-02-commands-wiring.md b/plans/260509-2317-per-group-warning-settings/phase-02-commands-wiring.md new file mode 100644 index 0000000..dfd4587 --- /dev/null +++ b/plans/260509-2317-per-group-warning-settings/phase-02-commands-wiring.md @@ -0,0 +1,91 @@ +--- +phase: 2 +title: Commands + wiring +status: completed +priority: P2 +effort: 45m +dependencies: + - 1 +--- + +# Phase 2: Commands + wiring + +## Overview + +Add `/settings` (read) and `/setdayswarning` (write) commands. Wire both into the dispatcher map in `bot.js`. Both gate on `authorizeGroup` like every other command except `/info`. + +## Architecture + +**Command names** +- `/settings` — generic reader, prints all settings + their effective values. Future settings drop in here. +- `/setdayswarning ` — single-purpose setter. Pattern: one setter command per setting key. Easier UX than a generic `/set `. + +**Reset semantics for `/setdayswarning`** +- `` parsed as integer +- `n >= 1 && n <= 3650` → store +- `n == 0` or arg `default` → reset (delete the setting key, fall back to env default) +- Anything else → "Invalid arguments" + +**`/settings` output** +``` +Group Settings +
┌──────────────────────────┬───────┬─────────┐
+│ Setting                  │ Value │ Default │
+├──────────────────────────┼───────┼─────────┤
+│ numDaysWarningNotUpdated │ 45    │ 30      │
+└──────────────────────────┴───────┴─────────┘
+``` +Use existing `buildTable` from `src/util/table.js`. Show env default in a separate column so user understands what `0`/reset means. + +## Related Code Files + +**Create** +- `src/bot/commands/get-settings.js` — `createGetSettingsCommand(config, store)` returns handler +- `src/bot/commands/set-days-warning.js` — `createSetDaysWarningCommand(config, store)` returns handler + +**Modify** +- `src/bot/bot.js` — import + register both: + ```js + settings: createGetSettingsCommand(config, store), + setdayswarning: createSetDaysWarningCommand(config, store), + ``` + +## Implementation Steps + +1. Create `src/bot/commands/get-settings.js`: + - Signature `(msg, sender, args) => …` + - First line: `if (!(await authorizeGroup(msg.chat.id, store, sender))) return;` + - Reject if `args.length !== 0` + - Read group via `store.group.getGroup(msg.chat.id)` + - Build settings rows: known keys list `[['numDaysWarningNotUpdated', g?.settings?.numDaysWarningNotUpdated, config.numDaysWarningNotUpdated]]` + - Render table with `buildTable` + - Send via `sender.sendMessage` + +2. Create `src/bot/commands/set-days-warning.js`: + - Same auth guard + - Reject if `args.length !== 1` + - Parse arg: `Number.parseInt` + - If `args[0] === 'default'` or parsed `=== 0` → call `store.group.setSetting(chatId, 'numDaysWarningNotUpdated', undefined)`, reply "Reset to default (Nd)" + - If parsed valid (`Number.isFinite && >= 1 && <= 3650`) → `setSetting(chatId, 'numDaysWarningNotUpdated', parsed)`, reply "Days-to-warning set to N" + - Otherwise `sender.sendMessage(chatId, 'Invalid arguments')` and return + +3. In `bot.js`: + - Add two imports near the other command imports + - Add two map entries in the `commands` object (preserve alphabetical-ish grouping or stick at end with raw\* commands — match existing convention) + +4. `node --check` on the three modified/new files. + +## Success Criteria + +- [ ] `/settings` from authorized group prints table with current value + env default +- [ ] `/settings` from unauthorized group returns "Group is not allowed to use bot" +- [ ] `/setdayswarning 45` persists to Upstash; subsequent `/settings` shows 45 +- [ ] `/setdayswarning 0` (or `default`) deletes the setting; resolver falls back to env default +- [ ] `/setdayswarning abc` / `/setdayswarning -5` / `/setdayswarning 9999` → "Invalid arguments" +- [ ] After `/setdayswarning 45`, daily cron + `/checkapp` use 45 for that group only + +## Risk Assessment + +- **Command discoverability:** users may not know the command exists. Mitigation: out-of-scope (no help command exists in either Java or JS bot). Consider a future `/help` command. +- **Race on concurrent writes:** `mutateAndSave` is read-modify-write without a transaction. Two simultaneous `/setdayswarning` calls in the same group could clobber each other. Acceptable — low likelihood, low blast radius (last write wins on the same key). +- **No admin gate:** any group member can change the threshold. Matches existing model (any member can `/addapple`). If product wants admin-only later, add `requireAdminUser` check — pattern already exists in `command-utils.js`. diff --git a/plans/260509-2317-per-group-warning-settings/phase-03-smoke-test.md b/plans/260509-2317-per-group-warning-settings/phase-03-smoke-test.md new file mode 100644 index 0000000..927324b --- /dev/null +++ b/plans/260509-2317-per-group-warning-settings/phase-03-smoke-test.md @@ -0,0 +1,51 @@ +--- +phase: 3 +title: Smoke test +status: completed +priority: P3 +effort: 15m +dependencies: + - 2 +--- + +# Phase 3: Smoke test + +## Overview + +No unit-test suite exists. Validate via syntax checks, the existing secret-leak lint, and a manual Telegram run-through against the deployed bot. + +## Implementation Steps + +1. `node --check` on every modified/new file: + - `src/util/group-settings.js` + - `src/repository/group-repository.js` + - `src/scheduler/scheduler.js` + - `src/bot/commands/check-app.js` + - `src/bot/commands/get-settings.js` + - `src/bot/commands/set-days-warning.js` + - `src/bot/bot.js` +2. `npm run lint` (secret-leak scan, must pass). +3. Deploy preview to Vercel (or merge to `main` if direct-deploy is normal). +4. Manual Telegram smoke (in an authorized test group): + - `/settings` → table shows `numDaysWarningNotUpdated = (unset) / default 30` (or whatever env value is) + - `/setdayswarning 5` → "Days-to-warning set to 5" + - `/settings` → shows `5 / 30` + - `/checkapp` → header reads `>5 days`, results reflect 5-day threshold + - `/setdayswarning 0` → "Reset to default (30d)" + - `/settings` → back to unset / 30 + - `/setdayswarning abc` → "Invalid arguments" + - `/setdayswarning 9999` → "Invalid arguments" + - From an unauthorized chat: `/settings` → "Group is not allowed to use bot" +5. Wait for next daily cron OR force-trigger via Vercel cron-now panel; confirm authoritized group with override sees correct threshold in report header. + +## Success Criteria + +- [ ] All `node --check` pass +- [ ] `npm run lint` passes +- [ ] Manual Telegram run-through hits every bullet above +- [ ] Daily cron report header reflects per-group override (or shows env default for groups with no override) + +## Risk Assessment + +- **No automated regression:** future refactors could silently break the resolver. Acceptable for now — repo has no tests at all. Document the resolver invariants in the file's leading comment. +- **Cron timing:** waiting up to 24h for natural cron firing slows verification. Mitigation: hit `/api/cron` directly with `Authorization: Bearer $CRON_SECRET` to force a run. diff --git a/plans/260509-2317-per-group-warning-settings/plan.md b/plans/260509-2317-per-group-warning-settings/plan.md new file mode 100644 index 0000000..259613e --- /dev/null +++ b/plans/260509-2317-per-group-warning-settings/plan.md @@ -0,0 +1,40 @@ +--- +title: per-group days-to-warning setting + settings commands +description: >- + Allow each Telegram group to override numDaysWarningNotUpdated. Add /settings + (read) and /setdayswarning (write). +status: completed +priority: P2 +created: 2026-05-09T00:00:00.000Z +--- + +# per-group days-to-warning setting + settings commands + +## Overview + +Today `numDaysWarningNotUpdated` is global env config (default 30). One threshold for all groups. Goal: let each group set its own threshold, falling back to env default. Also expose generic `/settings` reader so future per-group settings (timezone, silent days, etc.) drop in without new commands. + +## Goals & Non-Goals + +**Goals** +- `group.settings.numDaysWarningNotUpdated` optional override, persisted in Upstash with the rest of group state +- `/settings` — show all settings for the calling group (with effective + default values) +- `/setdayswarning ` — set the threshold (1..3650 integer); `0` or empty resets to default +- Scheduler + `/checkapp` use the per-group resolved threshold + +**Non-Goals** +- Other settings (timezone, silent weekend, language) — schema is extensible but no values added yet +- Admin-only restriction — `authorizeGroup` only (matches sibling commands like `/addapple`) +- Migration of existing groups — schema is additive; missing `settings` reads as `{}` + +## Phases + +| Phase | Name | Status | +|-------|------|--------| +| 1 | [Schema + threshold resolver](./phase-01-schema-threshold-resolver.md) | Completed | +| 2 | [Commands + wiring](./phase-02-commands-wiring.md) | Completed | +| 3 | [Smoke test](./phase-03-smoke-test.md) | Completed | + +## Dependencies + +None. Self-contained change. Depends only on existing Upstash group repository + command pattern. diff --git a/src/bot/bot.js b/src/bot/bot.js index 5edddd3..857840b 100644 --- a/src/bot/bot.js +++ b/src/bot/bot.js @@ -11,6 +11,8 @@ import { createCheckAppCommand } from './commands/check-app.js'; import { createCheckAppScoresCommand } from './commands/check-app-scores.js'; import { createRawAppleAppCommand } from './commands/raw-apple-app.js'; import { createRawGoogleAppCommand } from './commands/raw-google-app.js'; +import { createGetSettingsCommand } from './commands/get-settings.js'; +import { createSetDaysWarningCommand } from './commands/set-days-warning.js'; const PARSE_MODE = 'HTML'; @@ -69,6 +71,8 @@ export function createBot(config, store, appleScraper, googleScraper) { checkappscore: createCheckAppScoresCommand(store, appleScraper, googleScraper), rawappleapp: createRawAppleAppCommand(store, appleScraper), rawgoogleapp: createRawGoogleAppCommand(store, googleScraper), + settings: createGetSettingsCommand(config, store), + setdayswarning: createSetDaysWarningCommand(config, store), }; return { sender, commands, api }; diff --git a/src/bot/commands/check-app.js b/src/bot/commands/check-app.js index 24d20cb..2ebbd90 100644 --- a/src/bot/commands/check-app.js +++ b/src/bot/commands/check-app.js @@ -1,5 +1,6 @@ import { buildTable } from '../../util/table.js'; import { daysBetween, formatDateInTz } from '../../util/time.js'; +import { resolveDaysWarning } from '../../util/group-settings.js'; import { authorizeGroup } from './command-utils.js'; // /checkapp — reports update status per app, per store. @@ -12,7 +13,7 @@ export function createCheckAppCommand(config, store, appleScraper, googleScraper } const group = await store.group.getGroup(msg.chat.id); const nowMs = Date.now(); - const threshold = config.numDaysWarningNotUpdated; + const threshold = resolveDaysWarning(group, config); const headers = ['AppId', 'Updated', 'Days', 'OK']; const appleRows = await appleRowsFor(group.appleApps, appleScraper, nowMs, threshold, config.timezone); diff --git a/src/bot/commands/get-settings.js b/src/bot/commands/get-settings.js new file mode 100644 index 0000000..8c3f9ec --- /dev/null +++ b/src/bot/commands/get-settings.js @@ -0,0 +1,29 @@ +import { buildTable } from '../../util/table.js'; +import { authorizeGroup } from './command-utils.js'; + +// /settings — table of per-group setting overrides + their env defaults. +// Add new rows here when introducing new per-group settings. +export function createGetSettingsCommand(config, store) { + return async (msg, sender, args) => { + if (!(await authorizeGroup(msg.chat.id, store, sender))) return; + if (args.length !== 0) { + await sender.sendMessage(msg.chat.id, 'Invalid arguments'); + return; + } + const group = await store.group.getGroup(msg.chat.id); + const s = group?.settings ?? {}; + const rows = [ + [ + 'numDaysWarningNotUpdated', + formatValue(s.numDaysWarningNotUpdated), + String(config.numDaysWarningNotUpdated), + ], + ]; + const out = 'Group Settings\n' + `
${buildTable(['Setting', 'Value', 'Default'], rows)}
`; + await sender.sendMessage(msg.chat.id, out); + }; +} + +function formatValue(v) { + return v === undefined || v === null ? '(unset)' : String(v); +} diff --git a/src/bot/commands/set-days-warning.js b/src/bot/commands/set-days-warning.js new file mode 100644 index 0000000..65b9bf1 --- /dev/null +++ b/src/bot/commands/set-days-warning.js @@ -0,0 +1,34 @@ +import { authorizeGroup } from './command-utils.js'; + +const MAX_DAYS = 3650; + +// /setdayswarning — sets the per-group warning threshold. +// `0` or `default` resets to the env-config default. +export function createSetDaysWarningCommand(config, store) { + return async (msg, sender, args) => { + if (!(await authorizeGroup(msg.chat.id, store, sender))) return; + if (args.length !== 1) { + await sender.sendMessage(msg.chat.id, 'Invalid arguments'); + return; + } + const arg = args[0]; + const parsed = Number.parseInt(arg, 10); + + if (arg === 'default' || parsed === 0) { + await store.group.setSetting(msg.chat.id, 'numDaysWarningNotUpdated', undefined); + await sender.sendMessage( + msg.chat.id, + `Reset to default (${config.numDaysWarningNotUpdated}d)`, + ); + return; + } + + if (!Number.isFinite(parsed) || String(parsed) !== arg || parsed < 1 || parsed > MAX_DAYS) { + await sender.sendMessage(msg.chat.id, 'Invalid arguments'); + return; + } + + await store.group.setSetting(msg.chat.id, 'numDaysWarningNotUpdated', parsed); + await sender.sendMessage(msg.chat.id, `Days-to-warning set to ${parsed}`); + }; +} diff --git a/src/repository/group-repository.js b/src/repository/group-repository.js index 571dbd7..8880d74 100644 --- a/src/repository/group-repository.js +++ b/src/repository/group-repository.js @@ -34,6 +34,15 @@ export function createGroupRepository(handle) { return true; } + async function setSetting(groupId, key, value) { + return mutateAndSave(groupId, (g) => { + g.settings ??= {}; + if (value === undefined || value === null) delete g.settings[key]; + else g.settings[key] = value; + return true; + }); + } + function addApp(list, appId, country) { if (list.some((a) => a.appId === appId)) return false; list.push({ appId, country }); @@ -51,6 +60,7 @@ export function createGroupRepository(handle) { getGroup, initGroup, deleteGroup, + setSetting, addAppleApp: (groupId, appId, country) => mutateAndSave(groupId, (g) => addApp(g.appleApps, appId, country)), removeAppleApp: (groupId, appId) => diff --git a/src/scheduler/scheduler.js b/src/scheduler/scheduler.js index c66ca67..0227211 100644 --- a/src/scheduler/scheduler.js +++ b/src/scheduler/scheduler.js @@ -1,5 +1,6 @@ import { buildTable, formatNumber, truncateString } from '../util/table.js'; import { daysBetween, formatDateInTz, formatDateTimeInTz, weekdayInTz } from '../util/time.js'; +import { resolveDaysWarning } from '../util/group-settings.js'; // One-shot daily check, invoked from api/cron.js. The cron schedule lives in // vercel.json ("0 0 * * *" UTC = 7am Asia/Ho_Chi_Minh). @@ -36,7 +37,7 @@ async function checkGroup(groupId, silent, now, config, store, sender, appleScra return; } - const threshold = config.numDaysWarningNotUpdated; + const threshold = resolveDaysWarning(group, config); const stale = []; for (const info of group.appleApps) { @@ -91,12 +92,12 @@ async function checkGroup(groupId, silent, now, config, store, sender, appleScra logger.info({ groupId }, 'All apps up-to-date'); return; } - const message = buildReport(groupId, stale, now, config); + const message = buildReport(groupId, stale, now, config, threshold); if (silent) await sender.sendMessageSilent(groupId, message); else await sender.sendMessage(groupId, message); } -function buildReport(groupId, apps, now, config) { +function buildReport(groupId, apps, now, config, threshold) { const headers = ['App', 'Store', 'Days', 'Updated', 'Score', 'Reviews', 'Ratings']; const rows = apps.map((a) => [ truncateString(a.title || '', 30), @@ -111,7 +112,7 @@ function buildReport(groupId, apps, now, config) { `Daily App Check Report\n` + `Date: ${formatDateTimeInTz(now, config.timezone)}\n` + `Group: ${groupId}\n` + - `Apps not updated in >${config.numDaysWarningNotUpdated} days: ${apps.length}\n\n` + + `Apps not updated in >${threshold} days: ${apps.length}\n\n` + `
${buildTable(headers, rows)}
` ); } diff --git a/src/util/group-settings.js b/src/util/group-settings.js new file mode 100644 index 0000000..982bd5c --- /dev/null +++ b/src/util/group-settings.js @@ -0,0 +1,7 @@ +// Resolves per-group setting overrides to effective values, falling back to +// global config defaults when a group has no override (or stored a bad value). + +export function resolveDaysWarning(group, config) { + const v = group?.settings?.numDaysWarningNotUpdated; + return Number.isFinite(v) && v > 0 ? v : config.numDaysWarningNotUpdated; +}