feat(alerts): accept notify_on on the REST alert endpoints - #6379
Draft
claude[bot] wants to merge 3 commits into
Draft
claude[bot] wants to merge 3 commits into
claude[bot] wants to merge 3 commits into
Conversation
## Context A threshold needs a way to say which transitions should notify, so the dashboard can offer the choice before the customer-facing switch exists. The dashboard schema is reachable only with a dashboard session, and no screen reads this field yet, so it arrives inert and lets the front end build against it instead of waiting. ## Description Thresholds gain the field on both the input and the read type, with an enum of the transitions a threshold may notify on. The rules are enforced where create, update and batch all pass, so none of them can drift from the others. Notifying on a trigger is always required, since a resolution only makes sense as the closing half of a pair. Asking for resolution requires a code, because that is how a resolution names which line it refers to, and the code must be unique within the alert or the reference would be ambiguous. Recurring thresholds cannot ask for it at all, because one code there names an unbounded ladder of lines and there is no single line to resolve. Uniqueness is checked only when something asks for resolution. Two thresholds may share a code today, so checking it unconditionally would reject alerts that are currently valid. A missing field takes the column default. An explicit empty list is treated as a caller clearing it, and is refused for the same reason as above.
## Context The comment above the duplicate code check restated what the method name and its body already say, so it added a second place to keep in sync without adding anything a reader could not see. ## Description Removes it.
## Context A threshold decides which transitions notify, but the field was reachable only from the dashboard schema. An alert created through the public API therefore had no way to ask for the resolution notice, and no way to see what it had been given, so an integration could only ever get the default. ## Description Both alert endpoints, for a subscription and for a wallet, now accept the field on their thresholds for create, update and batch alike, and the serializer returns it so a caller can read back what was stored. Values are left to the shared create-or-update rules the dashboard already goes through, so the two surfaces cannot drift on what a threshold may ask for. A caller that says nothing still takes the column default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLys3y8nVyx1rgUqqXDFP2
aquinofb
force-pushed
the
ing-545-graphql-notify-on
branch
from
September 14, 2026 15:32
1536a8d to
1ffb24a
Compare
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
2 participants
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.
Requested by Felipe Rodrigues · Slack thread
Before: A threshold carries
notify_on, which decides whether crossing back down sends thealert.resolvednotice, and #6133 lets the dashboard set it. The public REST API never saw the field: the alert endpoints permitted onlycode,valueandrecurringon a threshold, and the serializer returned only those three. An alert created or updated through the API was therefore stuck on the column default forever, with no way to ask for the resolution notice and no way to read back what it had been given. The only route to it was the dashboard.After: Both alert endpoints — the subscription one and the wallet one — accept
notify_onon their thresholds, for create, update and batch alike, and the serializer returns it. An integration can now opt a threshold in toalert.resolvedat the moment it creates the alert, and read back what was stored.How: the field is added to the permitted threshold keys in each of the three param methods on both controllers, and to the threshold hash the serializer emits. Nothing else moves. The values themselves are left to
validate_notify_on!inUsageMonitoring::Concerns::CreateOrUpdateConcern, whichCreateAlertServiceandUpdateAlertServicealready run — the same path the dashboard goes through — so the two surfaces cannot drift on what a threshold may ask for, and no second validation exists to keep in step. A caller that says nothing still takes the column default.This stacks on #6133 and must merge after it: the validation it relies on, and the
prepare_thresholdshandling that lets an explicit null fall back to the default, both arrive there. The base of this PR ising-545-graphql-notify-onrather thanmainfor that reason; it should be retargeted once #6133 lands. It closes the last gap in the chain — the webhook itself lands in #6373, and until this merges an API-created threshold could never opt in to receiving it.A ticket id still needs assigning. No Linear ticket covers this work, so the branch and title carry none; please add one before merge.
Note on validation: the test suite was not run. This environment has Ruby 3.3.6 against a Gemfile that wants 4.0.6, no bundle installed, and no Postgres or docker-compose, so
rspecandrubocop/standardrbcould not be executed. What was checked instead:ruby -con every changed file, an adversarial re-read of the diff, and a comparison of the new expectations against the existing patterns in the same spec files. The specs added below need a real run before this leaves draft.Specs added: create and update on both endpoints, each asserting the field round-trips through the response and is persisted on the row; one create case asserting an unknown value is refused with the service's
thresholds:notify_on/value_is_invaliderror and no alert created. The alert serializer spec's threshold expectation is an exact match, so it was updated for the new key.🤖 Generated with Claude Code
https://claude.ai/code/session_01RLys3y8nVyx1rgUqqXDFP2
Generated by Claude Code