feat(suggestions): supported/unsupported ack statuses + gated requester notice - #226
Open
BSalaeddin wants to merge 1 commit into
Conversation
This was referenced Sep 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #225 (
feat/site-suggestions-capture-requester) — review that one first; this PR's base is that branch, notdev.#225 made a "please support this store" request answerable: it recorded WHO asked and gave the coupons pipeline a read/ack door. This PR is the answer itself, and the notice that closes the loop with the person who asked.
The product rule
Nothing emails a user automatically until the owner decides. The ack endpoint is driven by an automated pipeline, so it only ever MARKS a row. Turning a mark into mail is an explicit act.
1. Statuses on the ack
POST /api/ingest/site-suggestions/ackbody is now{ ids, status? },statusdefaulting toimported.importedrejectedunsupportedsupportedA closed row (
rejected/unsupported/supported) can never be dragged back toimported. Without that rule, a re-drain of stale ids would re-open every store we had already decided about, and would sendsupportedrows round the loop to re-notify their requesters. Terminal-to-terminal IS allowed and is a real operator path (unsupportedlater becomingsupported).The guard lives in the UPDATE's own
WHEREclause (oneupdateManyAndReturn), so it is enforced by Postgres rather than by the read that preceded it; a row someone else moved in between is reportedraced, never counted as changed. The pre-read exists only to tell the refusal reasons apart — a plain count could not say whether an id was unknown, already there, or being dragged backwards.status_changed_atis stamped on every transition.imported_atstays the legacy stamp for the FIRST hand-over only, so a later answer cannot rewrite the day the pipeline took the row.2. The requester notice, gated
SITE_SUGGESTIONS_AUTO_NOTIFY— defaultfalse, a strict'true' | 'false'enum (a typo fails boot with a named variable rather than being guessed at). With it off, asupportedack marks the rows, leavesnotified_atNULL, and reports the pending count.POST /api/ingest/site-suggestions/notifywith{ ids }is the owner's send. It works regardless of the switch — the switch gates the ACK, never the owner.Idempotency is a claim, not a check:
notified_atis stamped BEFORE the mail goes out under anotifiedAt: nullguard, so two callers racing the same row cannot both send. A send that FAILS releases its claim and is reportedfailed, so the notice stays visibly pending and the same command retries it — a row saying the person was told when nobody told them would bury the notice forever, because nothing re-reads a stamped row.One mail per requester email per domain, case-folded, across calls. Duplicate suggestions are stamped with the instant the person was ACTUALLY told, not with now.
3. Ops notice on
supportedOne email to the same inbox as the suggestion mail (now ONE shared constant, so the two cannot drift apart on a deploy-env edit), listing the domains, requesters with an email, how many were auto-notified, how many await a decision, and the exact command with the pending ids already filled in.
Contract for caramel-coupons
The shipped
{ids}-only caller (coupons PR #126) is untouched: defaultimported, onlynewrows move,acknowledgedstill the count that really flipped. New response keys are additive.Verification
tsc --noEmitvitest run)prisma migrate resetfrom emptyRed-proofed in four arms, each reverting one half of the change:
The in-memory table under the unit suite is an announced fake that really evaluates the
where/selectthe lib sends and throws on an operator it does not implement, so a wrong WHERE clause cannot pass. Schema facts (the columns, the guard as a real database predicate, the claim serialising a send) are pinned on real Postgres.Deliberately not here
notify_decisioncolumn: asupportedrow with an email and a NULLnotified_atIS the pending decision.domainindex serves the "already told?" lookup, and adding one would have meant editing a migration already applied on developer machines.TODO(#225): the danger-zone "Delete my data" route still does not scrubrequester_emailfrom these rows — carried over, not addressed here.