Files
coder/coderd/database/modelqueries_internal_test.go
Michael Suchacz b957e66ccc feat!: drop the Coder Tasks tables and queries (#29086)
Drops the `tasks`, `task_workspace_apps` and `task_snapshots` tables,
the `tasks_with_status` view and the `task_status` enum, and recreates
`workspaces_expanded` without the `tasks` join and `task_id` column.
Deletes `queries/tasks.sql` and every wrapper, fixture and test that
existed only for those queries (dbauthz, dbmock, dbmetrics, dbfake,
dbgen, querier_test), plus the `task_id` projection in
`GetWorkspaceAgentAndWorkspaceByAuthToken`.

**Breaking change:** migration 000594 deletes all task rows. The down
migration restores the schema (tables, constraints, indexes, views), not
the data.

**Review changes:** the schema-only migration test was removed per
review (`TestMigrate/UpDownUp` still exercises the down path), the test
file no longer reorders existing tests, and the migration moved from
000593 to 000594. The `dump.sql` reorder is pg_dump dependency ordering:
`tasks_with_status` pulled its four workspace tables ahead of it, and
without the view they return to alphabetical positions.

**Stack context:** Coder Tasks removal stack (CODAGT-642): #28743
frontend → #28788 CLI → #28789 API → #28790 provisioner → #29084
notifications → #29085 build reasons → **#29086 tables** → #28750
permissions. The last four PRs replace the single data-drop migration
that used to live in #28750; each carries one concern and its own
migration number.

> Created by Xum (AI agent) on behalf of @ibetitsmike.


## Delivery record

**Decision (2026-09-11 ~14:40Z): merge** through the stack merge
endpoint, squash, pinned to head
`f1d236a0c82f123b8a0d6878799c227ef9bd5776` (merge SHA appended below
once landed).

**Independent substantive reviews on this PR (3):**

| # | Kind | Reviewed commit | Findings and disposition | Source |
|---|---|---|---|---|
| 1 | Codex code review (bot), PR opened | `24535f8410` | No findings,
no threads |
[summary](https://github.com/coder/coder/pull/29086#issuecomment-5583877627)
|
| 2 | Human review, spikecurtis (COMMENTED) | `24535f8410` | 4 findings:
dump.sql reorder question (answered: pg_dump dependency hoisting by
`tasks_with_status`); two moved tests (undone, the test file is no
longer touched); schema-only migration test (deleted). All resolved. |
[review](https://github.com/coder/coder/pull/29086#pullrequestreview-5142045164)
|
| 3 | Human review, spikecurtis (APPROVED) | `f1d236a0c8` (this head) |
Approval on the exact final head |
[review](https://github.com/coder/coder/pull/29086#pullrequestreview-5179353695)
|

The four COMMENTED reviews by @ibetitsmike on `f35688f40b` are
thread-reply wrappers and are not counted.

**Pre-split history relevant to this diff:** this layer's content lived
in the monolithic #28750 until 2026-09-08, which received 10 completed
Codex code reviews (2026-08-28 to 2026-09-01) and spikecurtis's
CHANGES_REQUESTED review of 2026-09-04 that asked for the split.
Counting those, the completed-review total relevant to this diff exceeds
the six-review budget, so no further discretionary review or fresh final
advisory was commissioned. The usable final recommendation for the final
diff is review 3 above: the reviewer who authored every finding on this
layer approved the exact head.

**Exact-head checks:** `required` SUCCESS in [run
34580949353](https://github.com/coder/coder/actions/runs/34580949353/job/103205933212);
rollup 15 success, 8 skipped, 1 failure. The failure is the non-required
`Analyze PR for Documentation Updates Needed` job
([103204101137](https://github.com/coder/coder/actions/runs/34580949071/job/103204101137)),
caused by the dormant `doc-check-pr-29086` review workspace
(CODAGT-1025), not a code finding; GitHub flags only `required` as
required for this PR. Merge-ref freshness: `refs/pull/29086/merge` has
the exact head as second parent and `25af21b5d5d` (the #29085 squash) as
first parent; main has since gained #29231, #28639 and #29218 (12 files,
none overlapping this PR). The composed tree (`git merge-tree` of
current main and this head) builds (`go build ./...`) and vets (`go vet`
on `coderd`, `coderd/database/...`, `coderd/wsrelated/...`) cleanly.
Layer diff parity with the previously pushed layer verified by numstat
after both GitHub restacks. `gh stack push` did not run the local
pre-push suite; CI is the validation evidence.

**Deferred:** the dormant doc-check workspace (CODAGT-1025) is outside
this PR. Next in the stack, #28750, stays blocked: CHANGES_REQUESTED,
its approval and CI predate the restack, and its review budget is
exhausted.
2026-09-11 14:24:32 +00:00

181 lines
6.8 KiB
Go

package database
import (
"reflect"
"regexp"
"slices"
"strings"
"testing"
"time"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/testutil"
)
func TestIsAuthorizedQuery(t *testing.T) {
t.Parallel()
query := `SELECT true;`
_, err := insertAuthorizedFilter(query, "")
require.ErrorContains(t, err, "does not contain authorized replace string", "ensure replace string")
}
// TestWorkspaceTableConvert verifies all workspace fields are converted
// when reducing a `Workspace` to a `WorkspaceTable`.
// This test is a guard rail to prevent developer oversight mistakes.
func TestWorkspaceTableConvert(t *testing.T) {
t.Parallel()
staticRandoms := &testutil.Random{
String: func() string { return "foo" },
Bool: func() bool { return true },
Int: func() int64 { return 500 },
Uint: func() uint64 { return 126 },
Float: func() float64 { return 3.14 },
Complex: func() complex128 { return 6.24 },
Time: func() time.Time {
return time.Date(2020, 5, 2, 5, 19, 21, 30, time.UTC)
},
}
// This feels a bit janky, but it works.
// If you use 'PopulateStruct' to create 2 workspaces, using the same
// "random" values for each type. Then they should be identical.
//
// So if 'workspace.WorkspaceTable()' was missing any fields in its
// conversion, the comparison would fail.
var workspace Workspace
err := testutil.PopulateStruct(&workspace, staticRandoms)
require.NoError(t, err)
var subset WorkspaceTable
err = testutil.PopulateStruct(&subset, staticRandoms)
require.NoError(t, err)
require.Equal(t, workspace.WorkspaceTable(), subset,
"'workspace.WorkspaceTable()' is not missing at least 1 field when converting to 'WorkspaceTable'. "+
"To resolve this, go to the 'func (w Workspace) WorkspaceTable()' and ensure all fields are converted.")
}
// TestAuditLogsQueryConsistency ensures that GetAuditLogsOffset and CountAuditLogs
// have identical WHERE clauses to prevent filtering inconsistencies.
// This test is a guard rail to prevent developer oversight mistakes.
func TestAuditLogsQueryConsistency(t *testing.T) {
t.Parallel()
getWhereClause := extractWhereClause(getAuditLogsOffset)
require.NotEmpty(t, getWhereClause, "failed to extract WHERE clause from GetAuditLogsOffset")
countWhereClause := extractWhereClause(countAuditLogs)
require.NotEmpty(t, countWhereClause, "failed to extract WHERE clause from CountAuditLogs")
// Compare the WHERE clauses
if diff := cmp.Diff(getWhereClause, countWhereClause); diff != "" {
t.Errorf("GetAuditLogsOffset and CountAuditLogs WHERE clauses must be identical to ensure consistent filtering.\nDiff:\n%s", diff)
}
}
// Same as TestAuditLogsQueryConsistency, but for connection logs.
func TestConnectionLogsQueryConsistency(t *testing.T) {
t.Parallel()
getWhereClause := extractWhereClause(getConnectionLogsOffset)
require.NotEmpty(t, getWhereClause, "getConnectionLogsOffset query should have a WHERE clause")
countWhereClause := extractWhereClause(countConnectionLogs)
require.NotEmpty(t, countWhereClause, "countConnectionLogs query should have a WHERE clause")
require.Equal(t, getWhereClause, countWhereClause, "getConnectionLogsOffset and countConnectionLogs queries should have the same WHERE clause")
}
// TestFinalizeStaleChatDebugRows_TerminalStatusAlignment asserts that the
// NOT IN ('completed', 'error', 'interrupted') literals in the
// FinalizeStaleChatDebugRows SQL query match the terminal statuses
// defined by ChatDebugTerminalStatuses in codersdk. If a new terminal
// status is added to Go but not to the SQL, this test fails.
func TestFinalizeStaleChatDebugRows_TerminalStatusAlignment(t *testing.T) {
t.Parallel()
// Extract all NOT IN (...) lists from the SQL constant.
re := regexp.MustCompile(`NOT IN\s*\(([^)]+)\)`)
matches := re.FindAllStringSubmatch(finalizeStaleChatDebugRows, -1)
require.NotEmpty(t, matches, "expected at least one NOT IN clause in finalizeStaleChatDebugRows")
// Parse the quoted status literals from each NOT IN clause.
literalRe := regexp.MustCompile(`'([^']+)'`)
goTerminal := codersdk.ChatDebugTerminalStatuses()
for _, match := range matches {
literals := literalRe.FindAllStringSubmatch(match[1], -1)
var sqlStatuses []string
for _, lit := range literals {
sqlStatuses = append(sqlStatuses, lit[1])
}
slices.Sort(sqlStatuses)
var goStatuses []string
for _, s := range goTerminal {
goStatuses = append(goStatuses, string(s))
}
slices.Sort(goStatuses)
require.Equal(t, goStatuses, sqlStatuses,
"terminal statuses in FinalizeStaleChatDebugRows SQL must match "+
"codersdk.ChatDebugTerminalStatuses(); update both when adding "+
"a new terminal status")
}
}
// TestInsertChatMessagesOrderContract guards the input-order guarantee that
// callers rely on when indexing the returned slice. A behavior test cannot:
// Postgres evaluates the id default in row order anyway, so a batch still looks
// ordered once the guarantee is removed.
func TestInsertChatMessagesOrderContract(t *testing.T) {
t.Parallel()
require.Contains(t, insertChatMessages, "nextval('chat_messages_id_seq')",
"ids must be allocated explicitly so they can be correlated to input array position")
require.Contains(t, insertChatMessages, "ROW_NUMBER() OVER (ORDER BY id)",
"the k-th smallest allocated id must be assigned to input index k")
require.Regexp(t, `(?s)ORDER BY id\s*\z`, strings.TrimSpace(insertChatMessages),
"returned rows must be explicitly ordered by id rather than relying on RETURNING order")
// Every parallel input array must be read at the allocated ordinal. A column
// left on UNNEST would be positioned by the executor instead.
subscripted := regexp.MustCompile(`\)\[allocated\.ord\]`).FindAllString(insertChatMessages, -1)
require.Len(t, subscripted, reflect.TypeOf(InsertChatMessagesParams{}).NumField()-1,
"each InsertChatMessagesParams array field, all but ChatID, must be subscripted by allocated.ord")
}
// extractWhereClause extracts the WHERE clause from a SQL query string
func extractWhereClause(query string) string {
// Find WHERE and get everything after it
wherePattern := regexp.MustCompile(`(?is)WHERE\s+(.*)`)
whereMatches := wherePattern.FindStringSubmatch(query)
if len(whereMatches) < 2 {
return ""
}
whereClause := whereMatches[1]
// Remove ORDER BY, LIMIT, OFFSET clauses from the end
whereClause = regexp.MustCompile(`(?is)\s+(ORDER BY|LIMIT|OFFSET).*$`).ReplaceAllString(whereClause, "")
// Remove SQL comments
whereClause = regexp.MustCompile(`(?m)--.*$`).ReplaceAllString(whereClause, "")
// Normalize indentation so subquery wrapping doesn't cause
// mismatches.
lines := strings.Split(whereClause, "\n")
for i, line := range lines {
lines[i] = strings.TrimLeft(line, " \t")
}
whereClause = strings.Join(lines, "\n")
return strings.TrimSpace(whereClause)
}