forked from molecule-ai/molecule-core
fix(schedules): backfill legacy rows to 'template' + extract import SQL const
Addresses code-review warnings on PR #76: - Migration 022 now backfills pre-existing workspace_schedules rows to source='template' before flipping NOT NULL + DEFAULT 'runtime'. Legacy rows (all seeded via org/import historically) stay refreshable on re-import. Down migration drops the CHECK constraint too. - Extracted the import UPSERT into const orgImportScheduleSQL so the shape test asserts against the const directly instead of file-scraping org.go. Removed the os.ReadFile helper. - scheduleResponse.Source gets json:\",omitempty\" so old clients that predate the migration don't see an empty string they can't explain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
2e9fb51ff9
commit
b15e30ccde
@ -29,6 +29,28 @@ import (
|
||||
// during org import. Prevents overwhelming Docker when creating many containers.
|
||||
const workspaceCreatePacingMs = 50
|
||||
|
||||
// orgImportScheduleSQL is the upsert executed for every schedule during
|
||||
// org/import. Extracted to a const so TestImport_OrgScheduleSQLShape can
|
||||
// assert its shape without regex-scanning org.go (issue #24 follow-up).
|
||||
//
|
||||
// Guarantees, in one statement:
|
||||
// - INSERT new rows with source='template'
|
||||
// - On (workspace_id, name) collision, only refresh template-source rows
|
||||
// (runtime-added schedules are preserved across re-imports)
|
||||
// - No DELETE — removal is out of scope (additive semantics)
|
||||
const orgImportScheduleSQL = `
|
||||
INSERT INTO workspace_schedules (workspace_id, name, cron_expr, timezone, prompt, enabled, next_run_at, source)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7, 'template')
|
||||
ON CONFLICT (workspace_id, name) DO UPDATE
|
||||
SET cron_expr = EXCLUDED.cron_expr,
|
||||
timezone = EXCLUDED.timezone,
|
||||
prompt = EXCLUDED.prompt,
|
||||
enabled = EXCLUDED.enabled,
|
||||
next_run_at = EXCLUDED.next_run_at,
|
||||
updated_at = now()
|
||||
WHERE workspace_schedules.source = 'template'
|
||||
`
|
||||
|
||||
type OrgHandler struct {
|
||||
workspace *WorkspaceHandler
|
||||
broadcaster *events.Broadcaster
|
||||
@ -447,25 +469,8 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, defa
|
||||
enabled = *sched.Enabled
|
||||
}
|
||||
nextRun, _ := scheduler.ComputeNextRun(sched.CronExpr, tz, time.Now())
|
||||
// Idempotent (additive) import. The DB is the source of truth (issue #24).
|
||||
// - INSERT new template-source rows when missing.
|
||||
// - On (workspace_id, name) collision, only refresh rows whose
|
||||
// source='template'. Runtime-added schedules with the same name
|
||||
// are preserved across re-imports.
|
||||
// - Never DELETE — rows present in DB but absent from the template
|
||||
// are left alone (could be runtime-added or removed-from-template).
|
||||
if _, err := db.DB.ExecContext(context.Background(), `
|
||||
INSERT INTO workspace_schedules (workspace_id, name, cron_expr, timezone, prompt, enabled, next_run_at, source)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7, 'template')
|
||||
ON CONFLICT (workspace_id, name) DO UPDATE
|
||||
SET cron_expr = EXCLUDED.cron_expr,
|
||||
timezone = EXCLUDED.timezone,
|
||||
prompt = EXCLUDED.prompt,
|
||||
enabled = EXCLUDED.enabled,
|
||||
next_run_at = EXCLUDED.next_run_at,
|
||||
updated_at = now()
|
||||
WHERE workspace_schedules.source = 'template'
|
||||
`, id, sched.Name, sched.CronExpr, tz, sched.Prompt, enabled, nextRun); err != nil {
|
||||
if _, err := db.DB.ExecContext(context.Background(), orgImportScheduleSQL,
|
||||
id, sched.Name, sched.CronExpr, tz, sched.Prompt, enabled, nextRun); err != nil {
|
||||
log.Printf("Org import: failed to upsert schedule '%s' for %s: %v", sched.Name, ws.Name, err)
|
||||
} else {
|
||||
log.Printf("Org import: schedule '%s' (%s) upserted for %s (source=template)", sched.Name, sched.CronExpr, ws.Name)
|
||||
|
||||
@ -32,7 +32,7 @@ type scheduleResponse struct {
|
||||
RunCount int `json:"run_count"`
|
||||
LastStatus string `json:"last_status"`
|
||||
LastError string `json:"last_error"`
|
||||
Source string `json:"source"` // 'template' (seeded by org/import) | 'runtime' (created via Canvas/API). Issue #24.
|
||||
Source string `json:"source,omitempty"` // 'template' (seeded by org/import) | 'runtime' (created via Canvas/API). Issue #24.
|
||||
CreatedAt time.Time `json:"created_at"`
|
||||
UpdatedAt time.Time `json:"updated_at"`
|
||||
}
|
||||
|
||||
@ -4,7 +4,6 @@ import (
|
||||
"bytes"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"regexp"
|
||||
"strings"
|
||||
"testing"
|
||||
@ -61,7 +60,7 @@ func TestRuntimeSchedule_HasSourceRuntime(t *testing.T) {
|
||||
// regression that would silently break user-created schedules across
|
||||
// re-imports without needing a full provisioner harness.
|
||||
func TestImport_OrgScheduleSQLShape(t *testing.T) {
|
||||
got := orgImportScheduleSQL(t)
|
||||
got := orgImportScheduleSQL
|
||||
|
||||
// Single test covers four CEO requirements at once: additive seed
|
||||
// (template marker), idempotent refresh (ON CONFLICT DO UPDATE),
|
||||
@ -83,27 +82,6 @@ func TestImport_OrgScheduleSQLShape(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// orgImportScheduleSQL pulls the relevant SQL fragment from org.go for shape
|
||||
// assertions. Keeps this test independent of the provisioner.
|
||||
func orgImportScheduleSQL(t *testing.T) string {
|
||||
t.Helper()
|
||||
// Test runs from package dir (platform/internal/handlers) so org.go is local.
|
||||
data, err := os.ReadFile("org.go")
|
||||
if err != nil {
|
||||
t.Fatalf("read org.go: %v", err)
|
||||
}
|
||||
src := string(data)
|
||||
idx := strings.Index(src, "INSERT INTO workspace_schedules")
|
||||
if idx < 0 {
|
||||
t.Fatalf("could not find schedules INSERT in org.go")
|
||||
}
|
||||
end := strings.Index(src[idx:], "`")
|
||||
if end < 0 {
|
||||
t.Fatalf("could not find closing backtick after schedules INSERT")
|
||||
}
|
||||
return src[idx : idx+end]
|
||||
}
|
||||
|
||||
// TestList_IncludesSourceColumn asserts GET /workspaces/:id/schedules
|
||||
// returns the source field so Canvas can render template/runtime badges.
|
||||
func TestList_IncludesSourceColumn(t *testing.T) {
|
||||
|
||||
@ -1,2 +1,3 @@
|
||||
DROP INDEX IF EXISTS idx_schedules_workspace_name;
|
||||
ALTER TABLE workspace_schedules DROP CONSTRAINT IF EXISTS workspace_schedules_source_check;
|
||||
ALTER TABLE workspace_schedules DROP COLUMN IF EXISTS source;
|
||||
|
||||
@ -3,10 +3,33 @@
|
||||
-- API/Canvas ('runtime'). DB is the source of truth; org/import is now
|
||||
-- additive — it only INSERTs missing template rows and only UPDATEs rows
|
||||
-- where source = 'template'. Runtime-added schedules survive re-imports.
|
||||
--
|
||||
-- Legacy-row policy: every row predating this migration is backfilled to
|
||||
-- 'template'. Rationale — before this migration the only way to get a row
|
||||
-- into workspace_schedules at scale was org/import (Canvas UI for schedules
|
||||
-- was minimal); defaulting legacy rows to 'template' preserves the
|
||||
-- idempotent-refresh path on re-import. Users who had runtime-created
|
||||
-- schedules can reclassify them via UPDATE post-deployment.
|
||||
|
||||
ALTER TABLE workspace_schedules
|
||||
ADD COLUMN IF NOT EXISTS source TEXT NOT NULL DEFAULT 'runtime'
|
||||
CHECK (source IN ('template', 'runtime'));
|
||||
ADD COLUMN IF NOT EXISTS source TEXT;
|
||||
|
||||
UPDATE workspace_schedules SET source = 'template' WHERE source IS NULL;
|
||||
|
||||
ALTER TABLE workspace_schedules ALTER COLUMN source SET NOT NULL;
|
||||
ALTER TABLE workspace_schedules ALTER COLUMN source SET DEFAULT 'runtime';
|
||||
|
||||
-- idempotent constraint add (Postgres lacks IF NOT EXISTS on ADD CONSTRAINT pre-15)
|
||||
DO $$
|
||||
BEGIN
|
||||
IF NOT EXISTS (
|
||||
SELECT 1 FROM pg_constraint WHERE conname = 'workspace_schedules_source_check'
|
||||
) THEN
|
||||
ALTER TABLE workspace_schedules
|
||||
ADD CONSTRAINT workspace_schedules_source_check
|
||||
CHECK (source IN ('template', 'runtime'));
|
||||
END IF;
|
||||
END$$;
|
||||
|
||||
COMMENT ON COLUMN workspace_schedules.source IS
|
||||
'template = seeded by org/import (refreshable); runtime = created via Canvas/API (preserved across re-imports)';
|
||||
|
||||
Loading…
Reference in New Issue
Block a user