Skip to content

feat(settings): configurable default check interval (fixes #239) - #249

Merged
mikeknight85 merged 1 commit into
mainfrom
feat/default-check-interval
Oct 11, 2026
Merged

mikeknight85 merged 1 commit into
mainfrom
feat/default-check-interval

Conversation

@mikeknight85

Copy link
Copy Markdown
Owner

Implements #239. Thanks @GeilerGelber — bulk-adding right after setup is exactly the case this was annoying for.

Both levels, which answers @stevene1919's open question on the issue

He asked whether this should be admin-only. It is genuinely both, because each level answers a different need:

  • Instance default — system_settings.default_refresh_interval, under Admin → System → Product Tracking. The only value that applies to an account which has never opened Settings, including every account on a fresh install. This is where someone responsible for how hard the install leans on retailers sets the house cadence.
  • Per-user preference — users.default_refresh_interval, under Settings → Tracking (new section). Literally what the issue asks for; a self-hosted instance with several people tracking different kinds of thing has no business forcing one cadence on everyone.

The change stayed contained because both halves reuse established patterns — the admin field is one more CollapsibleCard in the existing SystemSection form, and the new user section copies RegionalSection exactly, following the save model #237 and #238 established.

Deliberately not built: the instance value is a default, not a floor on user preferences. A floor is a different feature and the issue does not ask for one.

Resolution order

explicit request value  >  users.default_refresh_interval  >  system_settings.default_refresh_interval  >  43200

Ranked in a pure module (utils/system/check-interval.ts) and applied in ProductAddService.addProduct, so the discovery and confirmation paths resolve identically. Every step treats null, '', 'auto', 0, -1 and NaN as "not set" and falls through — so an instance with nothing configured lands on exactly the 12 hours the form previously hardcoded.

Existing products are untouched. Migration 022 contains no UPDATE products, and nothing in the new path reads or writes an existing listing's interval. A default only ever decides the interval of a listing being created.

The form prefills from a server-resolved number (GET /api/settings/product-defaults), so it never knows the ranking and a non-admin can see the effective default without being handed Admin → System.

A minimum interval, on stored defaults only

One hour floor, one week ceiling, on both write paths and again at resolution.

A default is the one interval here that multiplies: set it to five minutes and a bulk-add of thirty listings is thirty retailers' worth of traffic every five minutes, forever. And #226's DOMAIN_GAP_MS = 30_000 serialises per retailer, so an interval shorter than a sweep can complete in is not honoured anyway — the sweep just never catches up. One hour is the shortest the UI has ever offered, so nothing a person could previously pick is now refused.

An explicit per-product interval is honoured verbatim, floor included — clamping it would change the behaviour of an API call that works today. A floor on PUT /products/:id would be a separate policy decision, flagged rather than quietly added.

On the SettingsCache TTL

Sidestepped rather than worked around: this reads system_settings directly rather than through settingsCache. The cache exists so the scraper can consult selector lists on every extraction; this value is read once when the form mounts and once per product added, so the cache saves nothing measurable — while 30 minutes of staleness would mean an admin changes the default, adds a product to check, sees the old number and concludes the feature is broken.

Migration 022, tested against a real Postgres

Not just tsc. A throwaway Homebrew cluster, driven through the project's actual migrate CLI:

  • fresh database up — all 22 applied
  • idempotency against a partly-migrated database — value changed by hand to 21600, the migrations row removed, up re-run: re-applied and the administrator's value left intact
  • down — both halves gone; up again clean

Plus an end-to-end script against that database exercising the real repository SQL, confirming the ranking, the clamp (requested 60 stored as 3600), and that a save from another settings section cannot clear the preference.

The 021 gap is deliberate and harmless — #245 claims that number and is still in review. The runner globs the directory and tracks applied names in a migrations table, and 021 is a seed-only insert, so it can land later in any order.

Verification

tsc clean both sides. 836 backend tests (814 on main, +22) and 39 frontend (33, +6). Playwright 20 passed on private port 4402, bundle ownership proved by grepping the served chunks for Default Check Interval.

The one as any in the diff is in routeTree.gen.ts, which TanStack Router generates for the new /settings/tracking route.

Noted, not fixed

UserProfileService.updateProfile's generic change log prints the requested value, so a clamped save logs -> 60 while 3600 is stored. Pre-existing shared loop over Object.keys(data); restructuring it for one field would be churn beyond this issue.

…239)

The Add Product form opened on 12 hours and both product-add paths fell
back to the same hardcoded 43200, so anybody wanting a different cadence
changed the field on every single product -- real friction when
bulk-adding a list right after setup.

Two places now answer it, because in a self-hosted multi-user instance
two different people have a claim on it:

- system_settings.default_refresh_interval, set under Admin -> System, is
  the instance default and the only one that applies to an account that
  has never opened Settings.
- users.default_refresh_interval, set under Settings -> Tracking, is one
  person's own preference and wins over the instance default for the
  products they add. NULL means follow the instance default, which is
  what every existing account gets.

Resolution is explicit value > user preference > instance default > the
old 12 hours, ranked in utils/system/check-interval.ts and read once per
add by DefaultCheckIntervalService. Nothing reads or writes an existing
product's refresh_interval, and an instance that has configured neither
behaves exactly as it did before.

A stored default is held to a one-hour floor and a one-week ceiling: it
is the one interval here that is applied to every product somebody adds,
and PriceCheckTask's per-retailer 30s spacing paces requests rather than
protecting a retailer from an aggressive setting. An explicit
per-product interval is still honoured verbatim.

The resolved default is read straight from the repositories rather than
through SettingsCache, so a changed setting takes effect on the next
product added instead of after the cache's 30-minute TTL.

Migration 022 adds the nullable column and seeds the instance default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant