feat(api): enable/disable destinations via Create and Update#895
Conversation
…eate; disabled_at on Update Enables importing destinations from another system (e.g. Svix migrations) with their original timestamps preserved. The Update API also accepts disabled_at (omit/null/timestamp) so callers can enable/disable as part of a normal PATCH without a separate /enable or /disable round-trip. Validation: created_at <= updated_at <= now and created_at <= disabled_at <= now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
alexluong
changed the title
… provided on Create Importers can now send disabled_at alone without also having to send a created_at; the destination's created_at silently mirrors disabled_at in that case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Trust import payloads — accept created_at/updated_at/disabled_at as-is instead of enforcing future and ordering checks. Importers from other systems may carry inconsistent data, and rejecting it would block migrations for no real benefit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Is the expected usage for importing a disabled destination to set |
No, the current logic is if disabled_at exists -> take effect right away. That said, I reckon it's quite trivial to check the value of that timestamp compared to 'now'. In what scenario would someone want to set a future timestamp for disabled_at? |
I don't see the use case, but I'm pointing it out as a potential point of confusion if there's no validation |
|
We can add validation that disabled_at cannot be in the future? |
|
Yeah although that does bring up the question as to why created_at/updated_at could be in the future 🤔 |
The only validation worth keeping — a disabled_at in the future would mean "scheduled to disable later," which is not a feature we support. Other timestamp fields stay unvalidated; importers can carry whatever their source system has. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Aside we should probably limit created_at/updated_at input to API key auth? |
Mirrors the disabled_at future check. None of these timestamps make sense in the future at create time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fair 😅
hmm, let me think, I don't believe we have a lot of logic that's limited to API key auth vs JWT, but I see the point. |
JWT-authenticated tenants can no longer rewrite their own audit timestamps; sending created_at or updated_at without admin auth returns 403. disabled_at stays accessible to both roles, matching the existing /enable and /disable endpoint semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Verify destination is not persisted when 403 is returned - Cover both fields together - Regression guard: JWT can still create destinations without these fields Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ErrorResponse.Parse leaves Code unset when it falls through to the
default branch (e.g. plain errors.New from handlers), which caused
AbortWithValidationError to emit {"status":0,...} alongside an HTTP
422. Set Code at the call site since AbortWithValidationError is
always 422 by definition; Parse stays caller-agnostic.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@alexbouchardd updated, ready for another round of review now! |
2 participants
Summary
This PR bundles two related but distinct scopes.
1. Enable/disable via Create and Update (closes #488, #612)
disabled_atso a destination can be created in a disabled state (e.g. importing a disabled Svix endpoint without a window where traffic could leak to it).disabled_at(omit /null/ timestamp) so callers can enable/disable as part of a normal PATCH. The dedicated/enableand/disableendpoints stay — this is the import-friendly path.destination disabled/destination enabledaudit log alongsidedestination updatedwhen disability state changes.2. Importable timestamps on Create
created_atandupdated_atso importers can preserve the original timestamps from the source system.Test plan
go test ./internal/apirouter/...— all passcreated_at/disabled_at, verify timestamps preserved in response and storage🤖 Generated with Claude Code