From 606a5fabc2e753f17fae69eded2982723ddb7b55 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 11 Feb 2025 16:17:25 +0000 Subject: [PATCH 01/32] feat: orgs soft delete --- .../database/migrations/000291_organization_soft_delete.down.sql | 1 + .../database/migrations/000291_organization_soft_delete.up.sql | 1 + 2 files changed, 2 insertions(+) create mode 100644 coderd/database/migrations/000291_organization_soft_delete.down.sql create mode 100644 coderd/database/migrations/000291_organization_soft_delete.up.sql diff --git a/coderd/database/migrations/000291_organization_soft_delete.down.sql b/coderd/database/migrations/000291_organization_soft_delete.down.sql new file mode 100644 index 0000000000000..dfdf8d4ea5bcc --- /dev/null +++ b/coderd/database/migrations/000291_organization_soft_delete.down.sql @@ -0,0 +1 @@ +ALTER TABLE organizations DROP COLUMN deleted; diff --git a/coderd/database/migrations/000291_organization_soft_delete.up.sql b/coderd/database/migrations/000291_organization_soft_delete.up.sql new file mode 100644 index 0000000000000..f3eeb189caf6b --- /dev/null +++ b/coderd/database/migrations/000291_organization_soft_delete.up.sql @@ -0,0 +1 @@ +ALTER TABLE organizations ADD COLUMN deleted boolean DEFAULT FALSE NOT NULL; From 609890f62d04d99513c8ef48d35ff961b1efe3d8 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Sat, 15 Feb 2025 16:38:30 +0000 Subject: [PATCH 02/32] chore: rename migration --- ...t_delete.down.sql => 000292_organization_soft_delete.down.sql} | 0 ..._soft_delete.up.sql => 000292_organization_soft_delete.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000291_organization_soft_delete.down.sql => 000292_organization_soft_delete.down.sql} (100%) rename coderd/database/migrations/{000291_organization_soft_delete.up.sql => 000292_organization_soft_delete.up.sql} (100%) diff --git a/coderd/database/migrations/000291_organization_soft_delete.down.sql b/coderd/database/migrations/000292_organization_soft_delete.down.sql similarity index 100% rename from coderd/database/migrations/000291_organization_soft_delete.down.sql rename to coderd/database/migrations/000292_organization_soft_delete.down.sql diff --git a/coderd/database/migrations/000291_organization_soft_delete.up.sql b/coderd/database/migrations/000292_organization_soft_delete.up.sql similarity index 100% rename from coderd/database/migrations/000291_organization_soft_delete.up.sql rename to coderd/database/migrations/000292_organization_soft_delete.up.sql From 31e5eb2ff9f68f217cd3f6bd5efb8147cb4f4e1a Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Sat, 15 Feb 2025 16:40:16 +0000 Subject: [PATCH 03/32] chore: add deleted to audit table --- enterprise/audit/table.go | 1 + 1 file changed, 1 insertion(+) diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index b9367a6038e85..9c4504ffd854f 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -275,6 +275,7 @@ var auditableResourcesTypes = map[any]map[string]Action{ "id": ActionIgnore, "name": ActionTrack, "description": ActionTrack, + "deleted": ActionIgnore, "created_at": ActionIgnore, "updated_at": ActionTrack, "is_default": ActionTrack, From 608661b41b7c029388081a91e269f65052968eee Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Sat, 15 Feb 2025 16:41:01 +0000 Subject: [PATCH 04/32] chore: update audit table --- docs/admin/security/audit-logs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/admin/security/audit-logs.md b/docs/admin/security/audit-logs.md index 5c6a6e6a802a1..4817ea03f4bc5 100644 --- a/docs/admin/security/audit-logs.md +++ b/docs/admin/security/audit-logs.md @@ -23,7 +23,7 @@ We track the following resources: | NotificationsSettings
| |
FieldTracked
idfalse
notifier_pausedtrue
| | OAuth2ProviderApp
| |
FieldTracked
callback_urltrue
created_atfalse
icontrue
idfalse
nametrue
updated_atfalse
| | OAuth2ProviderAppSecret
| |
FieldTracked
app_idfalse
created_atfalse
display_secretfalse
hashed_secretfalse
idfalse
last_used_atfalse
secret_prefixfalse
| -| Organization
| |
FieldTracked
created_atfalse
descriptiontrue
display_nametrue
icontrue
idfalse
is_defaulttrue
nametrue
updated_attrue
| +| Organization
| |
FieldTracked
created_atfalse
deletedtrue
descriptiontrue
display_nametrue
icontrue
idfalse
is_defaulttrue
nametrue
updated_attrue
| | OrganizationSyncSettings
| |
FieldTracked
assign_defaulttrue
fieldtrue
mappingtrue
| | RoleSyncSettings
| |
FieldTracked
fieldtrue
mappingtrue
| | Template
write, delete | |
FieldTracked
active_version_idtrue
activity_bumptrue
allow_user_autostarttrue
allow_user_autostoptrue
allow_user_cancel_workspace_jobstrue
autostart_block_days_of_weektrue
autostop_requirement_days_of_weektrue
autostop_requirement_weekstrue
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_usernamefalse
default_ttltrue
deletedfalse
deprecatedtrue
descriptiontrue
display_nametrue
failure_ttltrue
group_acltrue
icontrue
idtrue
max_port_sharing_leveltrue
nametrue
organization_display_namefalse
organization_iconfalse
organization_idfalse
organization_namefalse
provisionertrue
require_active_versiontrue
time_til_dormanttrue
time_til_dormant_autodeletetrue
updated_atfalse
user_acltrue
| From 54039cf649cb1f41266d729bd773725e44e9041b Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Sat, 15 Feb 2025 16:41:36 +0000 Subject: [PATCH 05/32] chore: code generated from db migration --- coderd/database/dump.sql | 3 ++- coderd/database/models.go | 1 + coderd/database/queries.sql.go | 21 ++++++++++++++------- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index e699b34bd5433..0224f292b1688 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -967,7 +967,8 @@ CREATE TABLE organizations ( updated_at timestamp with time zone NOT NULL, is_default boolean DEFAULT false NOT NULL, display_name text NOT NULL, - icon text DEFAULT ''::text NOT NULL + icon text DEFAULT ''::text NOT NULL, + deleted boolean DEFAULT false NOT NULL ); CREATE TABLE parameter_schemas ( diff --git a/coderd/database/models.go b/coderd/database/models.go index 5411591eed51c..4e3353f844a02 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -2675,6 +2675,7 @@ type Organization struct { IsDefault bool `db:"is_default" json:"is_default"` DisplayName string `db:"display_name" json:"display_name"` Icon string `db:"icon" json:"icon"` + Deleted bool `db:"deleted" json:"deleted"` } type OrganizationMember struct { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 58722dc152005..a7e3df428c9a6 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5081,7 +5081,7 @@ func (q *sqlQuerier) DeleteOrganization(ctx context.Context, id uuid.UUID) error const getDefaultOrganization = `-- name: GetDefaultOrganization :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM organizations WHERE @@ -5102,13 +5102,14 @@ func (q *sqlQuerier) GetDefaultOrganization(ctx context.Context) (Organization, &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ) return i, err } const getOrganizationByID = `-- name: GetOrganizationByID :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM organizations WHERE @@ -5127,13 +5128,14 @@ func (q *sqlQuerier) GetOrganizationByID(ctx context.Context, id uuid.UUID) (Org &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ) return i, err } const getOrganizationByName = `-- name: GetOrganizationByName :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM organizations WHERE @@ -5154,13 +5156,14 @@ func (q *sqlQuerier) GetOrganizationByName(ctx context.Context, name string) (Or &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ) return i, err } const getOrganizations = `-- name: GetOrganizations :many SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM organizations WHERE @@ -5201,6 +5204,7 @@ func (q *sqlQuerier) GetOrganizations(ctx context.Context, arg GetOrganizationsP &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ); err != nil { return nil, err } @@ -5217,7 +5221,7 @@ func (q *sqlQuerier) GetOrganizations(ctx context.Context, arg GetOrganizationsP const getOrganizationsByUserID = `-- name: GetOrganizationsByUserID :many SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM organizations WHERE @@ -5249,6 +5253,7 @@ func (q *sqlQuerier) GetOrganizationsByUserID(ctx context.Context, userID uuid.U &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ); err != nil { return nil, err } @@ -5268,7 +5273,7 @@ INSERT INTO organizations (id, "name", display_name, description, icon, created_at, updated_at, is_default) VALUES -- If no organizations exist, and this is the first, make it the default. - ($1, $2, $3, $4, $5, $6, $7, (SELECT TRUE FROM organizations LIMIT 1) IS NULL) RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon + ($1, $2, $3, $4, $5, $6, $7, (SELECT TRUE FROM organizations LIMIT 1) IS NULL) RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted ` type InsertOrganizationParams struct { @@ -5301,6 +5306,7 @@ func (q *sqlQuerier) InsertOrganization(ctx context.Context, arg InsertOrganizat &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ) return i, err } @@ -5316,7 +5322,7 @@ SET icon = $5 WHERE id = $6 -RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon +RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted ` type UpdateOrganizationParams struct { @@ -5347,6 +5353,7 @@ func (q *sqlQuerier) UpdateOrganization(ctx context.Context, arg UpdateOrganizat &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ) return i, err } From 951669573592d7c06435a6c332e0a7f4305f2154 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Sat, 15 Feb 2025 18:12:58 +0000 Subject: [PATCH 06/32] feat: updates for soft delete --- coderd/database/db.go | 5 +- coderd/database/dbauthz/dbauthz.go | 19 ++-- coderd/database/dbmem/dbmem.go | 41 ++++---- coderd/database/dbmetrics/querymetrics.go | 29 ++++-- coderd/database/dbmock/dbmock.go | 44 ++++----- coderd/database/querier.go | 6 +- coderd/database/queries/organizations.sql | 108 ++++++++++++---------- coderd/httpmw/organizationparam.go | 5 +- coderd/idpsync/organization.go | 5 +- coderd/searchquery/search.go | 4 +- coderd/users.go | 10 +- docs/CONTRIBUTING.md | 6 +- docs/admin/security/audit-logs.md | 2 +- enterprise/coderd/groups.go | 5 +- enterprise/coderd/organizations.go | 17 +++- 15 files changed, 184 insertions(+), 122 deletions(-) diff --git a/coderd/database/db.go b/coderd/database/db.go index 0f923a861efb4..23ee5028e3a12 100644 --- a/coderd/database/db.go +++ b/coderd/database/db.go @@ -3,9 +3,8 @@ // Query functions are generated using sqlc. // // To modify the database schema: -// 1. Add a new migration using "create_migration.sh" in database/migrations/ -// 2. Run "make coderd/database/generate" in the root to generate models. -// 3. Add/Edit queries in "query.sql" and run "make coderd/database/generate" to create Go code. +// 1. Add a new migration using "create_migration.sh" in database/migrations/ and run "make gen" to generate models. +// 2. Add/Edit queries in "query.sql" and run "make gen" to create Go code. package database import ( diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 9e616dd79dcbc..9685ff2c82936 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1302,10 +1302,6 @@ func (q *querier) DeleteOldWorkspaceAgentStats(ctx context.Context) error { return q.db.DeleteOldWorkspaceAgentStats(ctx) } -func (q *querier) DeleteOrganization(ctx context.Context, id uuid.UUID) error { - return deleteQ(q.log, q.auth, q.db.GetOrganizationByID, q.db.DeleteOrganization)(ctx, id) -} - func (q *querier) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error { return deleteQ[database.OrganizationMember](q.log, q.auth, func(ctx context.Context, arg database.DeleteOrganizationMemberParams) (database.OrganizationMember, error) { member, err := database.ExpectOne(q.OrganizationMembers(ctx, database.OrganizationMembersParams(arg))) @@ -1926,7 +1922,7 @@ func (q *querier) GetOrganizationByID(ctx context.Context, id uuid.UUID) (databa return fetch(q.log, q.auth, q.db.GetOrganizationByID)(ctx, id) } -func (q *querier) GetOrganizationByName(ctx context.Context, name string) (database.Organization, error) { +func (q *querier) GetOrganizationByName(ctx context.Context, name database.GetOrganizationByNameParams) (database.Organization, error) { return fetch(q.log, q.auth, q.db.GetOrganizationByName)(ctx, name) } @@ -1943,7 +1939,7 @@ func (q *querier) GetOrganizations(ctx context.Context, args database.GetOrganiz return fetchWithPostFilter(q.auth, policy.ActionRead, fetch)(ctx, nil) } -func (q *querier) GetOrganizationsByUserID(ctx context.Context, userID uuid.UUID) ([]database.Organization, error) { +func (q *querier) GetOrganizationsByUserID(ctx context.Context, userID database.GetOrganizationsByUserIDParams) ([]database.Organization, error) { return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetOrganizationsByUserID)(ctx, userID) } @@ -3737,6 +3733,17 @@ func (q *querier) UpdateOrganization(ctx context.Context, arg database.UpdateOrg return updateWithReturn(q.log, q.auth, fetch, q.db.UpdateOrganization)(ctx, arg) } +func (q *querier) UpdateOrganizationDeletedByID(ctx context.Context, arg database.UpdateOrganizationDeletedByIDParams) error { + deleteF := func(ctx context.Context, id uuid.UUID) error { + return q.db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + ID: id, + Deleted: true, + UpdatedAt: dbtime.Now(), + }) + } + return deleteQ(q.log, q.auth, q.db.GetOrganizationByID, deleteF)(ctx, arg.ID) +} + func (q *querier) UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error { if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceProvisionerDaemon); err != nil { return err diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 7f56ea5f463e5..4c5d78b8d288c 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -2157,19 +2157,6 @@ func (q *FakeQuerier) DeleteOldWorkspaceAgentStats(_ context.Context) error { return nil } -func (q *FakeQuerier) DeleteOrganization(_ context.Context, id uuid.UUID) error { - q.mutex.Lock() - defer q.mutex.Unlock() - - for i, org := range q.organizations { - if org.ID == id && !org.IsDefault { - q.organizations = append(q.organizations[:i], q.organizations[i+1:]...) - return nil - } - } - return sql.ErrNoRows -} - func (q *FakeQuerier) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error { err := validateDatabaseType(arg) if err != nil { @@ -3688,12 +3675,12 @@ func (q *FakeQuerier) GetOrganizationByID(_ context.Context, id uuid.UUID) (data return q.getOrganizationByIDNoLock(id) } -func (q *FakeQuerier) GetOrganizationByName(_ context.Context, name string) (database.Organization, error) { +func (q *FakeQuerier) GetOrganizationByName(_ context.Context, params database.GetOrganizationByNameParams) (database.Organization, error) { q.mutex.RLock() defer q.mutex.RUnlock() for _, organization := range q.organizations { - if organization.Name == name { + if organization.Name == params.Name { return organization, nil } } @@ -3740,13 +3727,13 @@ func (q *FakeQuerier) GetOrganizations(_ context.Context, args database.GetOrgan return tmp, nil } -func (q *FakeQuerier) GetOrganizationsByUserID(_ context.Context, userID uuid.UUID) ([]database.Organization, error) { +func (q *FakeQuerier) GetOrganizationsByUserID(_ context.Context, arg database.GetOrganizationsByUserIDParams) ([]database.Organization, error) { q.mutex.RLock() defer q.mutex.RUnlock() organizations := make([]database.Organization, 0) for _, organizationMember := range q.organizationMembers { - if organizationMember.UserID != userID { + if organizationMember.UserID != arg.UserID { continue } for _, organization := range q.organizations { @@ -9822,6 +9809,26 @@ func (q *FakeQuerier) UpdateOrganization(_ context.Context, arg database.UpdateO return database.Organization{}, sql.ErrNoRows } +func (q *FakeQuerier) UpdateOrganizationDeletedByID(_ context.Context, arg database.UpdateOrganizationDeletedByIDParams) error { + if err := validateDatabaseType(arg); err != nil { + return err + } + + q.mutex.Lock() + defer q.mutex.Unlock() + + for index, organization := range q.organizations { + if organization.ID != arg.ID || organization.IsDefault { + continue + } + organization.Deleted = arg.Deleted + organization.UpdatedAt = arg.UpdatedAt + q.organizations[index] = organization + return nil + } + return sql.ErrNoRows +} + func (q *FakeQuerier) UpdateProvisionerDaemonLastSeenAt(_ context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error { err := validateDatabaseType(arg) if err != nil { diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 665c10658a5bc..33f81cdf03a8f 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -77,6 +77,17 @@ func (m queryMetricsStore) InTx(f func(database.Store) error, options *database. return m.dbMetrics.InTx(f, options) } +func (m queryMetricsStore) DeleteOrganization(ctx context.Context, id uuid.UUID) error { + start := time.Now() + r0 := m.s.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + ID: id, + Deleted: true, + UpdatedAt: time.Now(), + }) + m.queryLatencies.WithLabelValues("DeleteOrganization").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) AcquireLock(ctx context.Context, pgAdvisoryXactLock int64) error { start := time.Now() err := m.s.AcquireLock(ctx, pgAdvisoryXactLock) @@ -329,13 +340,6 @@ func (m queryMetricsStore) DeleteOldWorkspaceAgentStats(ctx context.Context) err return err } -func (m queryMetricsStore) DeleteOrganization(ctx context.Context, id uuid.UUID) error { - start := time.Now() - r0 := m.s.DeleteOrganization(ctx, id) - m.queryLatencies.WithLabelValues("DeleteOrganization").Observe(time.Since(start).Seconds()) - return r0 -} - func (m queryMetricsStore) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error { start := time.Now() r0 := m.s.DeleteOrganizationMember(ctx, arg) @@ -945,7 +949,7 @@ func (m queryMetricsStore) GetOrganizationByID(ctx context.Context, id uuid.UUID return organization, err } -func (m queryMetricsStore) GetOrganizationByName(ctx context.Context, name string) (database.Organization, error) { +func (m queryMetricsStore) GetOrganizationByName(ctx context.Context, name database.GetOrganizationByNameParams) (database.Organization, error) { start := time.Now() organization, err := m.s.GetOrganizationByName(ctx, name) m.queryLatencies.WithLabelValues("GetOrganizationByName").Observe(time.Since(start).Seconds()) @@ -966,7 +970,7 @@ func (m queryMetricsStore) GetOrganizations(ctx context.Context, args database.G return organizations, err } -func (m queryMetricsStore) GetOrganizationsByUserID(ctx context.Context, userID uuid.UUID) ([]database.Organization, error) { +func (m queryMetricsStore) GetOrganizationsByUserID(ctx context.Context, userID database.GetOrganizationsByUserIDParams) ([]database.Organization, error) { start := time.Now() organizations, err := m.s.GetOrganizationsByUserID(ctx, userID) m.queryLatencies.WithLabelValues("GetOrganizationsByUserID").Observe(time.Since(start).Seconds()) @@ -2366,6 +2370,13 @@ func (m queryMetricsStore) UpdateOrganization(ctx context.Context, arg database. return r0, r1 } +func (m queryMetricsStore) UpdateOrganizationDeletedByID(ctx context.Context, arg database.UpdateOrganizationDeletedByIDParams) error { + start := time.Now() + r0 := m.s.UpdateOrganizationDeletedByID(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateOrganizationDeletedByID").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error { start := time.Now() r0 := m.s.UpdateProvisionerDaemonLastSeenAt(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index c7711505d7d51..38ee52aa76bbd 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -557,20 +557,6 @@ func (mr *MockStoreMockRecorder) DeleteOldWorkspaceAgentStats(ctx any) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOldWorkspaceAgentStats", reflect.TypeOf((*MockStore)(nil).DeleteOldWorkspaceAgentStats), ctx) } -// DeleteOrganization mocks base method. -func (m *MockStore) DeleteOrganization(ctx context.Context, id uuid.UUID) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteOrganization", ctx, id) - ret0, _ := ret[0].(error) - return ret0 -} - -// DeleteOrganization indicates an expected call of DeleteOrganization. -func (mr *MockStoreMockRecorder) DeleteOrganization(ctx, id any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOrganization", reflect.TypeOf((*MockStore)(nil).DeleteOrganization), ctx, id) -} - // DeleteOrganizationMember mocks base method. func (m *MockStore) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error { m.ctrl.T.Helper() @@ -1942,18 +1928,18 @@ func (mr *MockStoreMockRecorder) GetOrganizationByID(ctx, id any) *gomock.Call { } // GetOrganizationByName mocks base method. -func (m *MockStore) GetOrganizationByName(ctx context.Context, name string) (database.Organization, error) { +func (m *MockStore) GetOrganizationByName(ctx context.Context, arg database.GetOrganizationByNameParams) (database.Organization, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetOrganizationByName", ctx, name) + ret := m.ctrl.Call(m, "GetOrganizationByName", ctx, arg) ret0, _ := ret[0].(database.Organization) ret1, _ := ret[1].(error) return ret0, ret1 } // GetOrganizationByName indicates an expected call of GetOrganizationByName. -func (mr *MockStoreMockRecorder) GetOrganizationByName(ctx, name any) *gomock.Call { +func (mr *MockStoreMockRecorder) GetOrganizationByName(ctx, arg any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrganizationByName", reflect.TypeOf((*MockStore)(nil).GetOrganizationByName), ctx, name) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrganizationByName", reflect.TypeOf((*MockStore)(nil).GetOrganizationByName), ctx, arg) } // GetOrganizationIDsByMemberIDs mocks base method. @@ -1987,18 +1973,18 @@ func (mr *MockStoreMockRecorder) GetOrganizations(ctx, arg any) *gomock.Call { } // GetOrganizationsByUserID mocks base method. -func (m *MockStore) GetOrganizationsByUserID(ctx context.Context, userID uuid.UUID) ([]database.Organization, error) { +func (m *MockStore) GetOrganizationsByUserID(ctx context.Context, arg database.GetOrganizationsByUserIDParams) ([]database.Organization, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetOrganizationsByUserID", ctx, userID) + ret := m.ctrl.Call(m, "GetOrganizationsByUserID", ctx, arg) ret0, _ := ret[0].([]database.Organization) ret1, _ := ret[1].(error) return ret0, ret1 } // GetOrganizationsByUserID indicates an expected call of GetOrganizationsByUserID. -func (mr *MockStoreMockRecorder) GetOrganizationsByUserID(ctx, userID any) *gomock.Call { +func (mr *MockStoreMockRecorder) GetOrganizationsByUserID(ctx, arg any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrganizationsByUserID", reflect.TypeOf((*MockStore)(nil).GetOrganizationsByUserID), ctx, userID) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrganizationsByUserID", reflect.TypeOf((*MockStore)(nil).GetOrganizationsByUserID), ctx, arg) } // GetParameterSchemasByJobID mocks base method. @@ -5039,6 +5025,20 @@ func (mr *MockStoreMockRecorder) UpdateOrganization(ctx, arg any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateOrganization", reflect.TypeOf((*MockStore)(nil).UpdateOrganization), ctx, arg) } +// UpdateOrganizationDeletedByID mocks base method. +func (m *MockStore) UpdateOrganizationDeletedByID(ctx context.Context, arg database.UpdateOrganizationDeletedByIDParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateOrganizationDeletedByID", ctx, arg) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateOrganizationDeletedByID indicates an expected call of UpdateOrganizationDeletedByID. +func (mr *MockStoreMockRecorder) UpdateOrganizationDeletedByID(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateOrganizationDeletedByID", reflect.TypeOf((*MockStore)(nil).UpdateOrganizationDeletedByID), ctx, arg) +} + // UpdateProvisionerDaemonLastSeenAt mocks base method. func (m *MockStore) UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 42b88d855e4c3..a5cedde6c4a73 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -94,7 +94,6 @@ type sqlcQuerier interface { // Logs can take up a lot of space, so it's important we clean up frequently. DeleteOldWorkspaceAgentLogs(ctx context.Context, threshold time.Time) error DeleteOldWorkspaceAgentStats(ctx context.Context) error - DeleteOrganization(ctx context.Context, id uuid.UUID) error DeleteOrganizationMember(ctx context.Context, arg DeleteOrganizationMemberParams) error DeleteProvisionerKey(ctx context.Context, id uuid.UUID) error DeleteReplicasUpdatedBefore(ctx context.Context, updatedAt time.Time) error @@ -197,10 +196,10 @@ type sqlcQuerier interface { GetOAuth2ProviderAppsByUserID(ctx context.Context, userID uuid.UUID) ([]GetOAuth2ProviderAppsByUserIDRow, error) GetOAuthSigningKey(ctx context.Context) (string, error) GetOrganizationByID(ctx context.Context, id uuid.UUID) (Organization, error) - GetOrganizationByName(ctx context.Context, name string) (Organization, error) + GetOrganizationByName(ctx context.Context, arg GetOrganizationByNameParams) (Organization, error) GetOrganizationIDsByMemberIDs(ctx context.Context, ids []uuid.UUID) ([]GetOrganizationIDsByMemberIDsRow, error) GetOrganizations(ctx context.Context, arg GetOrganizationsParams) ([]Organization, error) - GetOrganizationsByUserID(ctx context.Context, userID uuid.UUID) ([]Organization, error) + GetOrganizationsByUserID(ctx context.Context, arg GetOrganizationsByUserIDParams) ([]Organization, error) GetParameterSchemasByJobID(ctx context.Context, jobID uuid.UUID) ([]ParameterSchema, error) GetPresetByWorkspaceBuildID(ctx context.Context, workspaceBuildID uuid.UUID) (TemplateVersionPreset, error) GetPresetParametersByTemplateVersionID(ctx context.Context, templateVersionID uuid.UUID) ([]TemplateVersionPresetParameter, error) @@ -485,6 +484,7 @@ type sqlcQuerier interface { UpdateOAuth2ProviderAppByID(ctx context.Context, arg UpdateOAuth2ProviderAppByIDParams) (OAuth2ProviderApp, error) UpdateOAuth2ProviderAppSecretByID(ctx context.Context, arg UpdateOAuth2ProviderAppSecretByIDParams) (OAuth2ProviderAppSecret, error) UpdateOrganization(ctx context.Context, arg UpdateOrganizationParams) (Organization, error) + UpdateOrganizationDeletedByID(ctx context.Context, arg UpdateOrganizationDeletedByIDParams) error UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg UpdateProvisionerDaemonLastSeenAtParams) error UpdateProvisionerJobByID(ctx context.Context, arg UpdateProvisionerJobByIDParams) error UpdateProvisionerJobWithCancelByID(ctx context.Context, arg UpdateProvisionerJobWithCancelByIDParams) error diff --git a/coderd/database/queries/organizations.sql b/coderd/database/queries/organizations.sql index 3a74170a913e1..95a33fa7a2c40 100644 --- a/coderd/database/queries/organizations.sql +++ b/coderd/database/queries/organizations.sql @@ -1,89 +1,97 @@ -- name: GetDefaultOrganization :one SELECT - * + * FROM - organizations + organizations WHERE - is_default = true + is_default = true LIMIT - 1; + 1; -- name: GetOrganizations :many SELECT - * + * FROM - organizations + organizations WHERE - true - -- Filter by ids - AND CASE - WHEN array_length(@ids :: uuid[], 1) > 0 THEN - id = ANY(@ids) - ELSE true - END - AND CASE - WHEN @name::text != '' THEN - LOWER("name") = LOWER(@name) - ELSE true - END + -- Optionally include deleted organizations + deleted = @deleted + -- Filter by ids + AND CASE + WHEN array_length(@ids :: uuid[], 1) > 0 THEN + id = ANY(@ids) + ELSE true + END + AND CASE + WHEN @name::text != '' THEN + LOWER("name") = LOWER(@name) + ELSE true + END ; -- name: GetOrganizationByID :one SELECT - * + * FROM - organizations + organizations WHERE - id = $1; + id = $1; -- name: GetOrganizationByName :one SELECT - * + * FROM - organizations + organizations WHERE - LOWER("name") = LOWER(@name) + -- Optionally include deleted organizations + deleted = @deleted AND + LOWER("name") = LOWER(@name) LIMIT - 1; + 1; -- name: GetOrganizationsByUserID :many SELECT - * + * FROM - organizations + organizations WHERE - id = ANY( - SELECT - organization_id - FROM - organization_members - WHERE - user_id = $1 - ); + -- Optionally include deleted organizations + deleted = @deleted AND + id = ANY( + SELECT + organization_id + FROM + organization_members + WHERE + user_id = $1 + ); -- name: InsertOrganization :one INSERT INTO - organizations (id, "name", display_name, description, icon, created_at, updated_at, is_default) + organizations (id, "name", display_name, description, icon, created_at, updated_at, is_default) VALUES - -- If no organizations exist, and this is the first, make it the default. - (@id, @name, @display_name, @description, @icon, @created_at, @updated_at, (SELECT TRUE FROM organizations LIMIT 1) IS NULL) RETURNING *; + -- If no organizations exist, and this is the first, make it the default. + (@id, @name, @display_name, @description, @icon, @created_at, @updated_at, (SELECT TRUE FROM organizations WHERE deleted = false LIMIT 1) IS NULL) RETURNING *; -- name: UpdateOrganization :one UPDATE - organizations + organizations SET - updated_at = @updated_at, - name = @name, - display_name = @display_name, - description = @description, - icon = @icon + updated_at = @updated_at, + name = @name, + display_name = @display_name, + description = @description, + icon = @icon WHERE - id = @id + id = @id RETURNING *; --- name: DeleteOrganization :exec -DELETE FROM - organizations +-- name: UpdateOrganizationDeletedByID :exec +UPDATE organizations +SET + deleted = @deleted, + updated_at = @updated_at WHERE - id = $1 AND - is_default = false; + id = @id AND + is_default = false; + diff --git a/coderd/httpmw/organizationparam.go b/coderd/httpmw/organizationparam.go index a72b361b90d71..2eba0dcedf5b8 100644 --- a/coderd/httpmw/organizationparam.go +++ b/coderd/httpmw/organizationparam.go @@ -73,7 +73,10 @@ func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler if err == nil { organization, dbErr = db.GetOrganizationByID(ctx, id) } else { - organization, dbErr = db.GetOrganizationByName(ctx, arg) + organization, dbErr = db.GetOrganizationByName(ctx, database.GetOrganizationByNameParams{ + Name: arg, + Deleted: false, + }) } } if httpapi.Is404Error(dbErr) { diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index 6f755529cdde7..87fd9af5e935d 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -97,7 +97,10 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u return xerrors.Errorf("organization claims: %w", err) } - existingOrgs, err := tx.GetOrganizationsByUserID(ctx, user.ID) + existingOrgs, err := tx.GetOrganizationsByUserID(ctx, database.GetOrganizationsByUserIDParams{ + UserID: user.ID, + Deleted: false, + }) if err != nil { return xerrors.Errorf("failed to get user organizations: %w", err) } diff --git a/coderd/searchquery/search.go b/coderd/searchquery/search.go index 849dd7f584947..103dc80601ad9 100644 --- a/coderd/searchquery/search.go +++ b/coderd/searchquery/search.go @@ -258,7 +258,9 @@ func parseOrganization(ctx context.Context, db database.Store, parser *httpapi.Q if err == nil { return organizationID, nil } - organization, err := db.GetOrganizationByName(ctx, v) + organization, err := db.GetOrganizationByName(ctx, database.GetOrganizationByNameParams{ + Name: v, Deleted: false, + }) if err != nil { return uuid.Nil, xerrors.Errorf("organization %q either does not exist, or you are unauthorized to view it", v) } diff --git a/coderd/users.go b/coderd/users.go index 964f18724449a..5f8866903bc6f 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -1286,7 +1286,10 @@ func (api *API) organizationsByUser(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() user := httpmw.UserParam(r) - organizations, err := api.Database.GetOrganizationsByUserID(ctx, user.ID) + organizations, err := api.Database.GetOrganizationsByUserID(ctx, database.GetOrganizationsByUserIDParams{ + UserID: user.ID, + Deleted: false, + }) if errors.Is(err, sql.ErrNoRows) { err = nil organizations = []database.Organization{} @@ -1324,7 +1327,10 @@ func (api *API) organizationsByUser(rw http.ResponseWriter, r *http.Request) { func (api *API) organizationByUserAndName(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() organizationName := chi.URLParam(r, "organizationname") - organization, err := api.Database.GetOrganizationByName(ctx, organizationName) + organization, err := api.Database.GetOrganizationByName(ctx, database.GetOrganizationByNameParams{ + Name: organizationName, + Deleted: false, + }) if httpapi.Is404Error(err) { httpapi.ResourceNotFound(rw) return diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index fdc372c034903..4ec303b388d49 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -159,17 +159,17 @@ Database migrations are managed with To add new migrations, use the following command: ```shell -./coderd/database/migrations/create_fixture.sh my name +./coderd/database/migrations/create_migration.sh my name /home/coder/src/coder/coderd/database/migrations/000070_my_name.up.sql /home/coder/src/coder/coderd/database/migrations/000070_my_name.down.sql ``` -Run "make gen" to generate models. - Then write queries into the generated `.up.sql` and `.down.sql` files and commit them into the repository. The down script should make a best-effort to retain as much data as possible. +Run `make gen` to generate models. + #### Database fixtures (for testing migrations) There are two types of fixtures that are used to test that migrations don't diff --git a/docs/admin/security/audit-logs.md b/docs/admin/security/audit-logs.md index 4817ea03f4bc5..9a3f1f373caca 100644 --- a/docs/admin/security/audit-logs.md +++ b/docs/admin/security/audit-logs.md @@ -23,7 +23,7 @@ We track the following resources: | NotificationsSettings
| |
FieldTracked
idfalse
notifier_pausedtrue
| | OAuth2ProviderApp
| |
FieldTracked
callback_urltrue
created_atfalse
icontrue
idfalse
nametrue
updated_atfalse
| | OAuth2ProviderAppSecret
| |
FieldTracked
app_idfalse
created_atfalse
display_secretfalse
hashed_secretfalse
idfalse
last_used_atfalse
secret_prefixfalse
| -| Organization
| |
FieldTracked
created_atfalse
deletedtrue
descriptiontrue
display_nametrue
icontrue
idfalse
is_defaulttrue
nametrue
updated_attrue
| +| Organization
| |
FieldTracked
created_atfalse
deletedfalse
descriptiontrue
display_nametrue
icontrue
idfalse
is_defaulttrue
nametrue
updated_attrue
| | OrganizationSyncSettings
| |
FieldTracked
assign_defaulttrue
fieldtrue
mappingtrue
| | RoleSyncSettings
| |
FieldTracked
fieldtrue
mappingtrue
| | Template
write, delete | |
FieldTracked
active_version_idtrue
activity_bumptrue
allow_user_autostarttrue
allow_user_autostoptrue
allow_user_cancel_workspace_jobstrue
autostart_block_days_of_weektrue
autostop_requirement_days_of_weektrue
autostop_requirement_weekstrue
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_usernamefalse
default_ttltrue
deletedfalse
deprecatedtrue
descriptiontrue
display_nametrue
failure_ttltrue
group_acltrue
icontrue
idtrue
max_port_sharing_leveltrue
nametrue
organization_display_namefalse
organization_iconfalse
organization_idfalse
organization_namefalse
provisionertrue
require_active_versiontrue
time_til_dormanttrue
time_til_dormant_autodeletetrue
updated_atfalse
user_acltrue
| diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index 8d5a7fceefaec..9771dd9800bb0 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -440,7 +440,10 @@ func (api *API) groups(rw http.ResponseWriter, r *http.Request) { parser := httpapi.NewQueryParamParser() // Organization selector can be an org ID or name filter.OrganizationID = parser.UUIDorName(r.URL.Query(), uuid.Nil, "organization", func(orgName string) (uuid.UUID, error) { - org, err := api.Database.GetOrganizationByName(ctx, orgName) + org, err := api.Database.GetOrganizationByName(ctx, database.GetOrganizationByNameParams{ + Name: orgName, + Deleted: false, + }) if err != nil { return uuid.Nil, xerrors.Errorf("organization %q not found", orgName) } diff --git a/enterprise/coderd/organizations.go b/enterprise/coderd/organizations.go index a7ec4050ee654..a980b2d4c37db 100644 --- a/enterprise/coderd/organizations.go +++ b/enterprise/coderd/organizations.go @@ -150,7 +150,17 @@ func (api *API) deleteOrganization(rw http.ResponseWriter, r *http.Request) { return } - err := api.Database.DeleteOrganization(ctx, organization.ID) + err := api.Database.InTx(func(tx database.Store) error { + err := tx.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + ID: organization.ID, + Deleted: true, + UpdatedAt: dbtime.Now(), + }) + if err != nil { + return xerrors.Errorf("delete organization: %w", err) + } + return nil + }, nil) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error deleting organization.", @@ -204,7 +214,10 @@ func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) { return } - _, err := api.Database.GetOrganizationByName(ctx, req.Name) + _, err := api.Database.GetOrganizationByName(ctx, database.GetOrganizationByNameParams{ + Name: req.Name, + Deleted: false, + }) if err == nil { httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ Message: "Organization already exists with that name.", From 9a26df611266353e2ac930773d2370664c21b9a3 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Sun, 16 Feb 2025 13:35:15 +0000 Subject: [PATCH 07/32] chore: updates to sql.go --- coderd/database/queries.sql.go | 154 +++++++++++++++++++-------------- 1 file changed, 89 insertions(+), 65 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a7e3df428c9a6..5e683f436aa73 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5066,28 +5066,15 @@ func (q *sqlQuerier) UpdateMemberRoles(ctx context.Context, arg UpdateMemberRole return i, err } -const deleteOrganization = `-- name: DeleteOrganization :exec -DELETE FROM - organizations -WHERE - id = $1 AND - is_default = false -` - -func (q *sqlQuerier) DeleteOrganization(ctx context.Context, id uuid.UUID) error { - _, err := q.db.ExecContext(ctx, deleteOrganization, id) - return err -} - const getDefaultOrganization = `-- name: GetDefaultOrganization :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon, deleted + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM - organizations + organizations WHERE - is_default = true + is_default = true LIMIT - 1 + 1 ` func (q *sqlQuerier) GetDefaultOrganization(ctx context.Context) (Organization, error) { @@ -5109,11 +5096,11 @@ func (q *sqlQuerier) GetDefaultOrganization(ctx context.Context) (Organization, const getOrganizationByID = `-- name: GetOrganizationByID :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon, deleted + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM - organizations + organizations WHERE - id = $1 + id = $1 ` func (q *sqlQuerier) GetOrganizationByID(ctx context.Context, id uuid.UUID) (Organization, error) { @@ -5135,17 +5122,24 @@ func (q *sqlQuerier) GetOrganizationByID(ctx context.Context, id uuid.UUID) (Org const getOrganizationByName = `-- name: GetOrganizationByName :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon, deleted + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM - organizations + organizations WHERE - LOWER("name") = LOWER($1) + -- Optionally include deleted organizations + deleted = $1 AND + LOWER("name") = LOWER($2) LIMIT - 1 + 1 ` -func (q *sqlQuerier) GetOrganizationByName(ctx context.Context, name string) (Organization, error) { - row := q.db.QueryRowContext(ctx, getOrganizationByName, name) +type GetOrganizationByNameParams struct { + Deleted bool `db:"deleted" json:"deleted"` + Name string `db:"name" json:"name"` +} + +func (q *sqlQuerier) GetOrganizationByName(ctx context.Context, arg GetOrganizationByNameParams) (Organization, error) { + row := q.db.QueryRowContext(ctx, getOrganizationByName, arg.Deleted, arg.Name) var i Organization err := row.Scan( &i.ID, @@ -5163,31 +5157,33 @@ func (q *sqlQuerier) GetOrganizationByName(ctx context.Context, name string) (Or const getOrganizations = `-- name: GetOrganizations :many SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon, deleted + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM - organizations + organizations WHERE - true - -- Filter by ids - AND CASE - WHEN array_length($1 :: uuid[], 1) > 0 THEN - id = ANY($1) - ELSE true - END - AND CASE - WHEN $2::text != '' THEN - LOWER("name") = LOWER($2) - ELSE true - END + -- Optionally include deleted organizations + deleted = $1 + -- Filter by ids + AND CASE + WHEN array_length($2 :: uuid[], 1) > 0 THEN + id = ANY($2) + ELSE true + END + AND CASE + WHEN $3::text != '' THEN + LOWER("name") = LOWER($3) + ELSE true + END ` type GetOrganizationsParams struct { - IDs []uuid.UUID `db:"ids" json:"ids"` - Name string `db:"name" json:"name"` + Deleted bool `db:"deleted" json:"deleted"` + IDs []uuid.UUID `db:"ids" json:"ids"` + Name string `db:"name" json:"name"` } func (q *sqlQuerier) GetOrganizations(ctx context.Context, arg GetOrganizationsParams) ([]Organization, error) { - rows, err := q.db.QueryContext(ctx, getOrganizations, pq.Array(arg.IDs), arg.Name) + rows, err := q.db.QueryContext(ctx, getOrganizations, arg.Deleted, pq.Array(arg.IDs), arg.Name) if err != nil { return nil, err } @@ -5221,22 +5217,29 @@ func (q *sqlQuerier) GetOrganizations(ctx context.Context, arg GetOrganizationsP const getOrganizationsByUserID = `-- name: GetOrganizationsByUserID :many SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon, deleted + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM - organizations + organizations WHERE - id = ANY( - SELECT - organization_id - FROM - organization_members - WHERE - user_id = $1 - ) + -- Optionally include deleted organizations + deleted = $2 AND + id = ANY( + SELECT + organization_id + FROM + organization_members + WHERE + user_id = $1 + ) ` -func (q *sqlQuerier) GetOrganizationsByUserID(ctx context.Context, userID uuid.UUID) ([]Organization, error) { - rows, err := q.db.QueryContext(ctx, getOrganizationsByUserID, userID) +type GetOrganizationsByUserIDParams struct { + UserID uuid.UUID `db:"user_id" json:"user_id"` + Deleted bool `db:"deleted" json:"deleted"` +} + +func (q *sqlQuerier) GetOrganizationsByUserID(ctx context.Context, arg GetOrganizationsByUserIDParams) ([]Organization, error) { + rows, err := q.db.QueryContext(ctx, getOrganizationsByUserID, arg.UserID, arg.Deleted) if err != nil { return nil, err } @@ -5270,10 +5273,10 @@ func (q *sqlQuerier) GetOrganizationsByUserID(ctx context.Context, userID uuid.U const insertOrganization = `-- name: InsertOrganization :one INSERT INTO - organizations (id, "name", display_name, description, icon, created_at, updated_at, is_default) + organizations (id, "name", display_name, description, icon, created_at, updated_at, is_default) VALUES - -- If no organizations exist, and this is the first, make it the default. - ($1, $2, $3, $4, $5, $6, $7, (SELECT TRUE FROM organizations LIMIT 1) IS NULL) RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted + -- If no organizations exist, and this is the first, make it the default. + ($1, $2, $3, $4, $5, $6, $7, (SELECT TRUE FROM organizations WHERE deleted = false LIMIT 1) IS NULL) RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted ` type InsertOrganizationParams struct { @@ -5313,15 +5316,15 @@ func (q *sqlQuerier) InsertOrganization(ctx context.Context, arg InsertOrganizat const updateOrganization = `-- name: UpdateOrganization :one UPDATE - organizations + organizations SET - updated_at = $1, - name = $2, - display_name = $3, - description = $4, - icon = $5 + updated_at = $1, + name = $2, + display_name = $3, + description = $4, + icon = $5 WHERE - id = $6 + id = $6 RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted ` @@ -5358,6 +5361,27 @@ func (q *sqlQuerier) UpdateOrganization(ctx context.Context, arg UpdateOrganizat return i, err } +const updateOrganizationDeletedByID = `-- name: UpdateOrganizationDeletedByID :exec +UPDATE organizations +SET + deleted = $1, + updated_at = $2 +WHERE + id = $3 AND + is_default = false +` + +type UpdateOrganizationDeletedByIDParams struct { + Deleted bool `db:"deleted" json:"deleted"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + ID uuid.UUID `db:"id" json:"id"` +} + +func (q *sqlQuerier) UpdateOrganizationDeletedByID(ctx context.Context, arg UpdateOrganizationDeletedByIDParams) error { + _, err := q.db.ExecContext(ctx, updateOrganizationDeletedByID, arg.Deleted, arg.UpdatedAt, arg.ID) + return err +} + const getParameterSchemasByJobID = `-- name: GetParameterSchemasByJobID :many SELECT id, created_at, job_id, name, description, default_source_scheme, default_source_value, allow_override_source, default_destination_scheme, allow_override_destination, default_refresh, redisplay_value, validation_error, validation_condition, validation_type_system, validation_value_type, index From f1f25e101d4c1453a2f75d5a8382c145cfef8d8f Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Sun, 16 Feb 2025 14:19:32 +0000 Subject: [PATCH 08/32] fix: update indexes and unique constraint --- coderd/database/dump.sql | 7 ++----- .../000292_organization_soft_delete.down.sql | 9 +++++++++ .../migrations/000292_organization_soft_delete.up.sql | 11 +++++++++++ coderd/database/queries.sql.go | 2 +- coderd/database/queries/organizations.sql | 2 +- coderd/database/unique_constraint.go | 5 ++--- 6 files changed, 26 insertions(+), 10 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 0224f292b1688..bbf7d970975ee 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -2031,9 +2031,6 @@ ALTER TABLE ONLY oauth2_provider_apps ALTER TABLE ONLY organization_members ADD CONSTRAINT organization_members_pkey PRIMARY KEY (organization_id, user_id); -ALTER TABLE ONLY organizations - ADD CONSTRAINT organizations_name UNIQUE (name); - ALTER TABLE ONLY organizations ADD CONSTRAINT organizations_pkey PRIMARY KEY (id); @@ -2219,9 +2216,9 @@ CREATE INDEX idx_organization_member_organization_id_uuid ON organization_member CREATE INDEX idx_organization_member_user_id_uuid ON organization_members USING btree (user_id); -CREATE UNIQUE INDEX idx_organization_name ON organizations USING btree (name); +CREATE UNIQUE INDEX idx_organization_name ON organizations USING btree (name) WHERE (deleted = false); -CREATE UNIQUE INDEX idx_organization_name_lower ON organizations USING btree (lower(name)); +CREATE UNIQUE INDEX idx_organization_name_lower ON organizations USING btree (lower(name)) WHERE (deleted = false); CREATE UNIQUE INDEX idx_provisioner_daemons_org_name_owner_key ON provisioner_daemons USING btree (organization_id, name, lower(COALESCE((tags ->> 'owner'::text), ''::text))); diff --git a/coderd/database/migrations/000292_organization_soft_delete.down.sql b/coderd/database/migrations/000292_organization_soft_delete.down.sql index dfdf8d4ea5bcc..b47c264abfe5c 100644 --- a/coderd/database/migrations/000292_organization_soft_delete.down.sql +++ b/coderd/database/migrations/000292_organization_soft_delete.down.sql @@ -1 +1,10 @@ ALTER TABLE organizations DROP COLUMN deleted; + +DROP INDEX IF EXISTS idx_organization_name; +DROP INDEX IF EXISTS idx_organization_name_lower; + +CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name ON organizations USING btree (name); +CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name_lower ON organizations USING btree (lower(name)); + +ALTER TABLE ONLY organizations + ADD CONSTRAINT organizations_name UNIQUE (name); diff --git a/coderd/database/migrations/000292_organization_soft_delete.up.sql b/coderd/database/migrations/000292_organization_soft_delete.up.sql index f3eeb189caf6b..6d76da6ce93c1 100644 --- a/coderd/database/migrations/000292_organization_soft_delete.up.sql +++ b/coderd/database/migrations/000292_organization_soft_delete.up.sql @@ -1 +1,12 @@ ALTER TABLE organizations ADD COLUMN deleted boolean DEFAULT FALSE NOT NULL; + +DROP INDEX IF EXISTS idx_organization_name; +DROP INDEX IF EXISTS idx_organization_name_lower; + +CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name ON organizations USING btree (name) + where deleted = false; +CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name_lower ON organizations USING btree (lower(name)) + where deleted = false; + +ALTER TABLE ONLY organizations + DROP CONSTRAINT IF EXISTS organizations_name; diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 5e683f436aa73..0a76f858dbc82 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5276,7 +5276,7 @@ INSERT INTO organizations (id, "name", display_name, description, icon, created_at, updated_at, is_default) VALUES -- If no organizations exist, and this is the first, make it the default. - ($1, $2, $3, $4, $5, $6, $7, (SELECT TRUE FROM organizations WHERE deleted = false LIMIT 1) IS NULL) RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted + ($1, $2, $3, $4, $5, $6, $7, (SELECT TRUE FROM organizations LIMIT 1) IS NULL) RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted ` type InsertOrganizationParams struct { diff --git a/coderd/database/queries/organizations.sql b/coderd/database/queries/organizations.sql index 95a33fa7a2c40..17bde54986747 100644 --- a/coderd/database/queries/organizations.sql +++ b/coderd/database/queries/organizations.sql @@ -71,7 +71,7 @@ INSERT INTO organizations (id, "name", display_name, description, icon, created_at, updated_at, is_default) VALUES -- If no organizations exist, and this is the first, make it the default. - (@id, @name, @display_name, @description, @icon, @created_at, @updated_at, (SELECT TRUE FROM organizations WHERE deleted = false LIMIT 1) IS NULL) RETURNING *; + (@id, @name, @display_name, @description, @icon, @created_at, @updated_at, (SELECT TRUE FROM organizations LIMIT 1) IS NULL) RETURNING *; -- name: UpdateOrganization :one UPDATE diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index ce427cf97c3bc..1b57cb5742b2f 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -38,7 +38,6 @@ const ( UniqueOauth2ProviderAppsNameKey UniqueConstraint = "oauth2_provider_apps_name_key" // ALTER TABLE ONLY oauth2_provider_apps ADD CONSTRAINT oauth2_provider_apps_name_key UNIQUE (name); UniqueOauth2ProviderAppsPkey UniqueConstraint = "oauth2_provider_apps_pkey" // ALTER TABLE ONLY oauth2_provider_apps ADD CONSTRAINT oauth2_provider_apps_pkey PRIMARY KEY (id); UniqueOrganizationMembersPkey UniqueConstraint = "organization_members_pkey" // ALTER TABLE ONLY organization_members ADD CONSTRAINT organization_members_pkey PRIMARY KEY (organization_id, user_id); - UniqueOrganizationsName UniqueConstraint = "organizations_name" // ALTER TABLE ONLY organizations ADD CONSTRAINT organizations_name UNIQUE (name); UniqueOrganizationsPkey UniqueConstraint = "organizations_pkey" // ALTER TABLE ONLY organizations ADD CONSTRAINT organizations_pkey PRIMARY KEY (id); UniqueParameterSchemasJobIDNameKey UniqueConstraint = "parameter_schemas_job_id_name_key" // ALTER TABLE ONLY parameter_schemas ADD CONSTRAINT parameter_schemas_job_id_name_key UNIQUE (job_id, name); UniqueParameterSchemasPkey UniqueConstraint = "parameter_schemas_pkey" // ALTER TABLE ONLY parameter_schemas ADD CONSTRAINT parameter_schemas_pkey PRIMARY KEY (id); @@ -94,8 +93,8 @@ const ( UniqueWorkspacesPkey UniqueConstraint = "workspaces_pkey" // ALTER TABLE ONLY workspaces ADD CONSTRAINT workspaces_pkey PRIMARY KEY (id); UniqueIndexAPIKeyName UniqueConstraint = "idx_api_key_name" // CREATE UNIQUE INDEX idx_api_key_name ON api_keys USING btree (user_id, token_name) WHERE (login_type = 'token'::login_type); UniqueIndexCustomRolesNameLower UniqueConstraint = "idx_custom_roles_name_lower" // CREATE UNIQUE INDEX idx_custom_roles_name_lower ON custom_roles USING btree (lower(name)); - UniqueIndexOrganizationName UniqueConstraint = "idx_organization_name" // CREATE UNIQUE INDEX idx_organization_name ON organizations USING btree (name); - UniqueIndexOrganizationNameLower UniqueConstraint = "idx_organization_name_lower" // CREATE UNIQUE INDEX idx_organization_name_lower ON organizations USING btree (lower(name)); + UniqueIndexOrganizationName UniqueConstraint = "idx_organization_name" // CREATE UNIQUE INDEX idx_organization_name ON organizations USING btree (name) WHERE (deleted = false); + UniqueIndexOrganizationNameLower UniqueConstraint = "idx_organization_name_lower" // CREATE UNIQUE INDEX idx_organization_name_lower ON organizations USING btree (lower(name)) WHERE (deleted = false); UniqueIndexProvisionerDaemonsOrgNameOwnerKey UniqueConstraint = "idx_provisioner_daemons_org_name_owner_key" // CREATE UNIQUE INDEX idx_provisioner_daemons_org_name_owner_key ON provisioner_daemons USING btree (organization_id, name, lower(COALESCE((tags ->> 'owner'::text), ''::text))); UniqueIndexUsersEmail UniqueConstraint = "idx_users_email" // CREATE UNIQUE INDEX idx_users_email ON users USING btree (email) WHERE (deleted = false); UniqueIndexUsersUsername UniqueConstraint = "idx_users_username" // CREATE UNIQUE INDEX idx_users_username ON users USING btree (username) WHERE (deleted = false); From b2022544ab55f241ed635fab2857d767253f9ab6 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 18 Feb 2025 11:37:11 +0000 Subject: [PATCH 09/32] fix: update migration number --- ...t_delete.down.sql => 000295_organization_soft_delete.down.sql} | 0 ..._soft_delete.up.sql => 000295_organization_soft_delete.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000292_organization_soft_delete.down.sql => 000295_organization_soft_delete.down.sql} (100%) rename coderd/database/migrations/{000292_organization_soft_delete.up.sql => 000295_organization_soft_delete.up.sql} (100%) diff --git a/coderd/database/migrations/000292_organization_soft_delete.down.sql b/coderd/database/migrations/000295_organization_soft_delete.down.sql similarity index 100% rename from coderd/database/migrations/000292_organization_soft_delete.down.sql rename to coderd/database/migrations/000295_organization_soft_delete.down.sql diff --git a/coderd/database/migrations/000292_organization_soft_delete.up.sql b/coderd/database/migrations/000295_organization_soft_delete.up.sql similarity index 100% rename from coderd/database/migrations/000292_organization_soft_delete.up.sql rename to coderd/database/migrations/000295_organization_soft_delete.up.sql From 1bf37421c81b1cbc617cdbd66c3bac3811195d53 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 18 Feb 2025 14:43:16 +0000 Subject: [PATCH 10/32] fix: fix TestEnterpriseAuditLogs --- enterprise/coderd/audit_test.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/enterprise/coderd/audit_test.go b/enterprise/coderd/audit_test.go index d5616ea3888b9..a9bae01dae25f 100644 --- a/enterprise/coderd/audit_test.go +++ b/enterprise/coderd/audit_test.go @@ -60,7 +60,7 @@ func TestEnterpriseAuditLogs(t *testing.T) { }, alogs.AuditLogs[0].Organization) // OrganizationID is deprecated, but make sure it is set. - require.Equal(t, o.ID, alogs.AuditLogs[0].OrganizationID) + require.Equal(t, o.ID, alogs.AuditLogs[0].Organization.ID) // Delete the org and try again, should be mostly empty. err = client.DeleteOrganization(ctx, o.ID.String()) @@ -75,12 +75,8 @@ func TestEnterpriseAuditLogs(t *testing.T) { require.Equal(t, int64(1), alogs.Count) require.Len(t, alogs.AuditLogs, 1) - require.Equal(t, &codersdk.MinimalOrganization{ - ID: o.ID, - }, alogs.AuditLogs[0].Organization) - // OrganizationID is deprecated, but make sure it is set. - require.Equal(t, o.ID, alogs.AuditLogs[0].OrganizationID) + require.Equal(t, o.ID, alogs.AuditLogs[0].Organization.ID) // Some audit entries do not have an organization at all, in which case the // response omits the organization. @@ -104,6 +100,6 @@ func TestEnterpriseAuditLogs(t *testing.T) { require.Equal(t, (*codersdk.MinimalOrganization)(nil), alogs.AuditLogs[0].Organization) // OrganizationID is deprecated, but make sure it is empty. - require.Equal(t, uuid.Nil, alogs.AuditLogs[0].OrganizationID) + require.Equal(t, uuid.Nil, alogs.AuditLogs[0].Organization.ID) }) } From 41e943b0f4b71d3fceaa65e374564441403b5ea1 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 18 Feb 2025 14:56:28 +0000 Subject: [PATCH 11/32] fix: revert test --- enterprise/coderd/audit_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/enterprise/coderd/audit_test.go b/enterprise/coderd/audit_test.go index a9bae01dae25f..271671491860d 100644 --- a/enterprise/coderd/audit_test.go +++ b/enterprise/coderd/audit_test.go @@ -60,7 +60,7 @@ func TestEnterpriseAuditLogs(t *testing.T) { }, alogs.AuditLogs[0].Organization) // OrganizationID is deprecated, but make sure it is set. - require.Equal(t, o.ID, alogs.AuditLogs[0].Organization.ID) + require.Equal(t, o.ID, alogs.AuditLogs[0].OrganizationID) // Delete the org and try again, should be mostly empty. err = client.DeleteOrganization(ctx, o.ID.String()) @@ -76,7 +76,7 @@ func TestEnterpriseAuditLogs(t *testing.T) { require.Len(t, alogs.AuditLogs, 1) // OrganizationID is deprecated, but make sure it is set. - require.Equal(t, o.ID, alogs.AuditLogs[0].Organization.ID) + require.Equal(t, o.ID, alogs.AuditLogs[0].OrganizationID) // Some audit entries do not have an organization at all, in which case the // response omits the organization. @@ -100,6 +100,6 @@ func TestEnterpriseAuditLogs(t *testing.T) { require.Equal(t, (*codersdk.MinimalOrganization)(nil), alogs.AuditLogs[0].Organization) // OrganizationID is deprecated, but make sure it is empty. - require.Equal(t, uuid.Nil, alogs.AuditLogs[0].Organization.ID) + require.Equal(t, uuid.Nil, alogs.AuditLogs[0].OrganizationID) }) } From 9fe8da121ef1390fd04333be005aedd521791dcb Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Wed, 19 Feb 2025 22:10:04 +0000 Subject: [PATCH 12/32] fix: updates for pr review --- coderd/database/dbauthz/dbauthz.go | 1 - coderd/database/dbmem/dbmem.go | 6 +++--- coderd/database/dbmetrics/querymetrics.go | 1 - ...down.sql => 000296_organization_soft_delete.down.sql} | 0 ...ete.up.sql => 000296_organization_soft_delete.up.sql} | 0 coderd/database/queries.sql.go | 9 ++++----- coderd/database/queries/organizations.sql | 2 +- enterprise/audit/table.go | 2 +- enterprise/coderd/organizations.go | 1 - 9 files changed, 9 insertions(+), 13 deletions(-) rename coderd/database/migrations/{000295_organization_soft_delete.down.sql => 000296_organization_soft_delete.down.sql} (100%) rename coderd/database/migrations/{000295_organization_soft_delete.up.sql => 000296_organization_soft_delete.up.sql} (100%) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 9685ff2c82936..5c558aaa0d28c 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -3737,7 +3737,6 @@ func (q *querier) UpdateOrganizationDeletedByID(ctx context.Context, arg databas deleteF := func(ctx context.Context, id uuid.UUID) error { return q.db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ ID: id, - Deleted: true, UpdatedAt: dbtime.Now(), }) } diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 4c5d78b8d288c..9488577edca17 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -3680,7 +3680,7 @@ func (q *FakeQuerier) GetOrganizationByName(_ context.Context, params database.G defer q.mutex.RUnlock() for _, organization := range q.organizations { - if organization.Name == params.Name { + if organization.Name == params.Name && organization.Deleted == params.Deleted { return organization, nil } } @@ -3737,7 +3737,7 @@ func (q *FakeQuerier) GetOrganizationsByUserID(_ context.Context, arg database.G continue } for _, organization := range q.organizations { - if organization.ID != organizationMember.OrganizationID { + if organization.ID != organizationMember.OrganizationID || organization.Deleted != arg.Deleted { continue } organizations = append(organizations, organization) @@ -9821,7 +9821,7 @@ func (q *FakeQuerier) UpdateOrganizationDeletedByID(_ context.Context, arg datab if organization.ID != arg.ID || organization.IsDefault { continue } - organization.Deleted = arg.Deleted + organization.Deleted = true organization.UpdatedAt = arg.UpdatedAt q.organizations[index] = organization return nil diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 33f81cdf03a8f..90ea140d0505c 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -81,7 +81,6 @@ func (m queryMetricsStore) DeleteOrganization(ctx context.Context, id uuid.UUID) start := time.Now() r0 := m.s.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ ID: id, - Deleted: true, UpdatedAt: time.Now(), }) m.queryLatencies.WithLabelValues("DeleteOrganization").Observe(time.Since(start).Seconds()) diff --git a/coderd/database/migrations/000295_organization_soft_delete.down.sql b/coderd/database/migrations/000296_organization_soft_delete.down.sql similarity index 100% rename from coderd/database/migrations/000295_organization_soft_delete.down.sql rename to coderd/database/migrations/000296_organization_soft_delete.down.sql diff --git a/coderd/database/migrations/000295_organization_soft_delete.up.sql b/coderd/database/migrations/000296_organization_soft_delete.up.sql similarity index 100% rename from coderd/database/migrations/000295_organization_soft_delete.up.sql rename to coderd/database/migrations/000296_organization_soft_delete.up.sql diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 0a76f858dbc82..ea4124d8fca94 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5364,21 +5364,20 @@ func (q *sqlQuerier) UpdateOrganization(ctx context.Context, arg UpdateOrganizat const updateOrganizationDeletedByID = `-- name: UpdateOrganizationDeletedByID :exec UPDATE organizations SET - deleted = $1, - updated_at = $2 + deleted = true, + updated_at = $1 WHERE - id = $3 AND + id = $2 AND is_default = false ` type UpdateOrganizationDeletedByIDParams struct { - Deleted bool `db:"deleted" json:"deleted"` UpdatedAt time.Time `db:"updated_at" json:"updated_at"` ID uuid.UUID `db:"id" json:"id"` } func (q *sqlQuerier) UpdateOrganizationDeletedByID(ctx context.Context, arg UpdateOrganizationDeletedByIDParams) error { - _, err := q.db.ExecContext(ctx, updateOrganizationDeletedByID, arg.Deleted, arg.UpdatedAt, arg.ID) + _, err := q.db.ExecContext(ctx, updateOrganizationDeletedByID, arg.UpdatedAt, arg.ID) return err } diff --git a/coderd/database/queries/organizations.sql b/coderd/database/queries/organizations.sql index 17bde54986747..822b51c0aa8ba 100644 --- a/coderd/database/queries/organizations.sql +++ b/coderd/database/queries/organizations.sql @@ -89,7 +89,7 @@ RETURNING *; -- name: UpdateOrganizationDeletedByID :exec UPDATE organizations SET - deleted = @deleted, + deleted = true, updated_at = @updated_at WHERE id = @id AND diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 9c4504ffd854f..53f03dd60ae63 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -275,7 +275,7 @@ var auditableResourcesTypes = map[any]map[string]Action{ "id": ActionIgnore, "name": ActionTrack, "description": ActionTrack, - "deleted": ActionIgnore, + "deleted": ActionTrack, "created_at": ActionIgnore, "updated_at": ActionTrack, "is_default": ActionTrack, diff --git a/enterprise/coderd/organizations.go b/enterprise/coderd/organizations.go index a980b2d4c37db..6cf91ec5b856a 100644 --- a/enterprise/coderd/organizations.go +++ b/enterprise/coderd/organizations.go @@ -153,7 +153,6 @@ func (api *API) deleteOrganization(rw http.ResponseWriter, r *http.Request) { err := api.Database.InTx(func(tx database.Store) error { err := tx.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ ID: organization.ID, - Deleted: true, UpdatedAt: dbtime.Now(), }) if err != nil { From 692998800fb43c612aa4fd3b6d1c2c6553d9cf7e Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 20 Feb 2025 11:32:12 +0000 Subject: [PATCH 13/32] feat: add trigger to check for resources on soft delete --- coderd/database/dump.sql | 36 ++++++++++++++++ .../000296_organization_soft_delete.down.sql | 3 ++ .../000296_organization_soft_delete.up.sql | 41 +++++++++++++++++++ docs/admin/security/audit-logs.md | 2 +- 4 files changed, 81 insertions(+), 1 deletion(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index bbf7d970975ee..18eaa55ddc5db 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -438,6 +438,40 @@ BEGIN END; $$; +CREATE FUNCTION protect_provisioned_organizations() RETURNS trigger + LANGUAGE plpgsql + AS $$ +DECLARE + workspace_count int; + template_count int; +BEGIN + workspace_count := ( + SELECT count(*) as count FROM workspaces + WHERE + workspaces.organization_id = OLD.id + AND workspaces.deleted = false + ); + + template_count := ( + SELECT count(*) as count FROM templates + WHERE + templates.organization_id = OLD.id + AND templates.deleted = false + ); + + -- Fail the deletion if one of the following: + -- * the organization has 1 or more workspaces + -- * the organization has 1 or more templates + IF (workspace_count + template_count) > 0 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % workspaces and % templates that must be deleted first', workspace_count, template_count; + END IF; + + -- add more cases to fail a delete + + RETURN OLD; +END; +$$; + CREATE FUNCTION provisioner_tagset_contains(provisioner_tags tagset, job_tags tagset) RETURNS boolean LANGUAGE plpgsql AS $$ @@ -2350,6 +2384,8 @@ CREATE OR REPLACE VIEW provisioner_job_stats AS CREATE TRIGGER inhibit_enqueue_if_disabled BEFORE INSERT ON notification_messages FOR EACH ROW EXECUTE FUNCTION inhibit_enqueue_if_disabled(); +CREATE TRIGGER protect_provisioned_organizations BEFORE UPDATE ON organizations FOR EACH ROW WHEN (((new.deleted = true) AND (old.deleted = false))) EXECUTE FUNCTION protect_provisioned_organizations(); + CREATE TRIGGER remove_organization_member_custom_role BEFORE DELETE ON custom_roles FOR EACH ROW EXECUTE FUNCTION remove_organization_member_role(); COMMENT ON TRIGGER remove_organization_member_custom_role ON custom_roles IS 'When a custom_role is deleted, this trigger removes the role from all organization members.'; diff --git a/coderd/database/migrations/000296_organization_soft_delete.down.sql b/coderd/database/migrations/000296_organization_soft_delete.down.sql index b47c264abfe5c..f474c00e55b7d 100644 --- a/coderd/database/migrations/000296_organization_soft_delete.down.sql +++ b/coderd/database/migrations/000296_organization_soft_delete.down.sql @@ -8,3 +8,6 @@ CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name_lower ON organizations U ALTER TABLE ONLY organizations ADD CONSTRAINT organizations_name UNIQUE (name); + +DROP TRIGGER IF EXISTS protect_provisioned_organizations ON organizations; +DROP FUNCTION IF EXISTS protect_provisioned_organizations; diff --git a/coderd/database/migrations/000296_organization_soft_delete.up.sql b/coderd/database/migrations/000296_organization_soft_delete.up.sql index 6d76da6ce93c1..581152afe2c10 100644 --- a/coderd/database/migrations/000296_organization_soft_delete.up.sql +++ b/coderd/database/migrations/000296_organization_soft_delete.up.sql @@ -10,3 +10,44 @@ CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name_lower ON organizations U ALTER TABLE ONLY organizations DROP CONSTRAINT IF EXISTS organizations_name; + +CREATE FUNCTION protect_provisioned_organizations() + RETURNS TRIGGER AS +$$ +DECLARE + workspace_count int; + template_count int; +BEGIN + workspace_count := ( + SELECT count(*) as count FROM workspaces + WHERE + workspaces.organization_id = OLD.id + AND workspaces.deleted = false + ); + + template_count := ( + SELECT count(*) as count FROM templates + WHERE + templates.organization_id = OLD.id + AND templates.deleted = false + ); + + -- Fail the deletion if one of the following: + -- * the organization has 1 or more workspaces + -- * the organization has 1 or more templates + IF (workspace_count + template_count) > 0 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % workspaces and % templates that must be deleted first', workspace_count, template_count; + END IF; + + -- add more cases to fail a delete + + RETURN OLD; +END; +$$ LANGUAGE plpgsql; + +-- Trigger to protect organizations from being soft deleted with existing resources +CREATE TRIGGER protect_provisioned_organizations + BEFORE UPDATE ON organizations + FOR EACH ROW + WHEN (NEW.deleted = true AND OLD.deleted = false) + EXECUTE FUNCTION protect_provisioned_organizations(); diff --git a/docs/admin/security/audit-logs.md b/docs/admin/security/audit-logs.md index 9a3f1f373caca..4817ea03f4bc5 100644 --- a/docs/admin/security/audit-logs.md +++ b/docs/admin/security/audit-logs.md @@ -23,7 +23,7 @@ We track the following resources: | NotificationsSettings
| |
FieldTracked
idfalse
notifier_pausedtrue
| | OAuth2ProviderApp
| |
FieldTracked
callback_urltrue
created_atfalse
icontrue
idfalse
nametrue
updated_atfalse
| | OAuth2ProviderAppSecret
| |
FieldTracked
app_idfalse
created_atfalse
display_secretfalse
hashed_secretfalse
idfalse
last_used_atfalse
secret_prefixfalse
| -| Organization
| |
FieldTracked
created_atfalse
deletedfalse
descriptiontrue
display_nametrue
icontrue
idfalse
is_defaulttrue
nametrue
updated_attrue
| +| Organization
| |
FieldTracked
created_atfalse
deletedtrue
descriptiontrue
display_nametrue
icontrue
idfalse
is_defaulttrue
nametrue
updated_attrue
| | OrganizationSyncSettings
| |
FieldTracked
assign_defaulttrue
fieldtrue
mappingtrue
| | RoleSyncSettings
| |
FieldTracked
fieldtrue
mappingtrue
| | Template
write, delete | |
FieldTracked
active_version_idtrue
activity_bumptrue
allow_user_autostarttrue
allow_user_autostoptrue
allow_user_cancel_workspace_jobstrue
autostart_block_days_of_weektrue
autostop_requirement_days_of_weektrue
autostop_requirement_weekstrue
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_usernamefalse
default_ttltrue
deletedfalse
deprecatedtrue
descriptiontrue
display_nametrue
failure_ttltrue
group_acltrue
icontrue
idtrue
max_port_sharing_leveltrue
nametrue
organization_display_namefalse
organization_iconfalse
organization_idfalse
organization_namefalse
provisionertrue
require_active_versiontrue
time_til_dormanttrue
time_til_dormant_autodeletetrue
updated_atfalse
user_acltrue
| From eb880912ced1646968fb28dd7cdee2a8a234fa5f Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 20 Feb 2025 23:24:26 +0000 Subject: [PATCH 14/32] chore: update trigger function --- coderd/database/dump.sql | 12 +++++++++++- .../000296_organization_soft_delete.up.sql | 12 +++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 18eaa55ddc5db..8705d51dc25db 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -444,6 +444,7 @@ CREATE FUNCTION protect_provisioned_organizations() RETURNS trigger DECLARE workspace_count int; template_count int; + group_count int; BEGIN workspace_count := ( SELECT count(*) as count FROM workspaces @@ -459,14 +460,23 @@ BEGIN AND templates.deleted = false ); + group_count := ( + SELECT count(*) as count FROM groups + WHERE + groups.organization_id = OLD.id + ); + -- Fail the deletion if one of the following: -- * the organization has 1 or more workspaces -- * the organization has 1 or more templates + -- * the organization has 1 or more groups IF (workspace_count + template_count) > 0 THEN RAISE EXCEPTION 'cannot delete organization: organization has % workspaces and % templates that must be deleted first', workspace_count, template_count; END IF; - -- add more cases to fail a delete + IF (group_count) > 1 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % groups that must be deleted first', group_count; + END IF; RETURN OLD; END; diff --git a/coderd/database/migrations/000296_organization_soft_delete.up.sql b/coderd/database/migrations/000296_organization_soft_delete.up.sql index 581152afe2c10..e36fef3ffd6e6 100644 --- a/coderd/database/migrations/000296_organization_soft_delete.up.sql +++ b/coderd/database/migrations/000296_organization_soft_delete.up.sql @@ -17,6 +17,7 @@ $$ DECLARE workspace_count int; template_count int; + group_count int; BEGIN workspace_count := ( SELECT count(*) as count FROM workspaces @@ -32,14 +33,23 @@ BEGIN AND templates.deleted = false ); + group_count := ( + SELECT count(*) as count FROM groups + WHERE + groups.organization_id = OLD.id + ); + -- Fail the deletion if one of the following: -- * the organization has 1 or more workspaces -- * the organization has 1 or more templates + -- * the organization has 1 or more groups IF (workspace_count + template_count) > 0 THEN RAISE EXCEPTION 'cannot delete organization: organization has % workspaces and % templates that must be deleted first', workspace_count, template_count; END IF; - -- add more cases to fail a delete + IF (group_count) > 1 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % groups that must be deleted first', group_count; + END IF; RETURN OLD; END; From 10d1564647f8f0f79eb452799358a785b05370b0 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 12:34:41 +0000 Subject: [PATCH 15/32] fix: drop index add members check to trigger --- coderd/database/dump.sql | 17 +++++++++++++---- .../000296_organization_soft_delete.down.sql | 1 - .../000296_organization_soft_delete.up.sql | 19 +++++++++++++------ 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 8705d51dc25db..b4e578a1c5e48 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -445,6 +445,7 @@ DECLARE workspace_count int; template_count int; group_count int; + member_count int; BEGIN workspace_count := ( SELECT count(*) as count FROM workspaces @@ -466,19 +467,27 @@ BEGIN groups.organization_id = OLD.id ); + member_count := ( + SELECT count(*) as count FROM organization_members + WHERE + organization_members.organization_id = OLD.id + ); + -- Fail the deletion if one of the following: -- * the organization has 1 or more workspaces -- * the organization has 1 or more templates - -- * the organization has 1 or more groups + -- * the organization has 1 or more groups other than "Everyone" group + -- * the organization has 1 or more members other than the organization owner + IF (workspace_count + template_count) > 0 THEN RAISE EXCEPTION 'cannot delete organization: organization has % workspaces and % templates that must be deleted first', workspace_count, template_count; END IF; - IF (group_count) > 1 THEN - RAISE EXCEPTION 'cannot delete organization: organization has % groups that must be deleted first', group_count; + IF (group_count + member_count) > 2 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % groups and % members that must be deleted first', group_count - 1, member_count - 1; END IF; - RETURN OLD; + RETURN NEW; END; $$; diff --git a/coderd/database/migrations/000296_organization_soft_delete.down.sql b/coderd/database/migrations/000296_organization_soft_delete.down.sql index f474c00e55b7d..6d4a48685ed9b 100644 --- a/coderd/database/migrations/000296_organization_soft_delete.down.sql +++ b/coderd/database/migrations/000296_organization_soft_delete.down.sql @@ -1,6 +1,5 @@ ALTER TABLE organizations DROP COLUMN deleted; -DROP INDEX IF EXISTS idx_organization_name; DROP INDEX IF EXISTS idx_organization_name_lower; CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name ON organizations USING btree (name); diff --git a/coderd/database/migrations/000296_organization_soft_delete.up.sql b/coderd/database/migrations/000296_organization_soft_delete.up.sql index e36fef3ffd6e6..29651647c0559 100644 --- a/coderd/database/migrations/000296_organization_soft_delete.up.sql +++ b/coderd/database/migrations/000296_organization_soft_delete.up.sql @@ -3,8 +3,6 @@ ALTER TABLE organizations ADD COLUMN deleted boolean DEFAULT FALSE NOT NULL; DROP INDEX IF EXISTS idx_organization_name; DROP INDEX IF EXISTS idx_organization_name_lower; -CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name ON organizations USING btree (name) - where deleted = false; CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name_lower ON organizations USING btree (lower(name)) where deleted = false; @@ -18,6 +16,7 @@ DECLARE workspace_count int; template_count int; group_count int; + member_count int; BEGIN workspace_count := ( SELECT count(*) as count FROM workspaces @@ -39,19 +38,27 @@ BEGIN groups.organization_id = OLD.id ); + member_count := ( + SELECT count(*) as count FROM organization_members + WHERE + organization_members.organization_id = OLD.id + ); + -- Fail the deletion if one of the following: -- * the organization has 1 or more workspaces -- * the organization has 1 or more templates - -- * the organization has 1 or more groups + -- * the organization has 1 or more groups other than "Everyone" group + -- * the organization has 1 or more members other than the organization owner + IF (workspace_count + template_count) > 0 THEN RAISE EXCEPTION 'cannot delete organization: organization has % workspaces and % templates that must be deleted first', workspace_count, template_count; END IF; - IF (group_count) > 1 THEN - RAISE EXCEPTION 'cannot delete organization: organization has % groups that must be deleted first', group_count; + IF (group_count + member_count) > 2 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % groups and % members that must be deleted first', group_count - 1, member_count - 1; END IF; - RETURN OLD; + RETURN NEW; END; $$ LANGUAGE plpgsql; From 56260b095f8efd68bc2328bacda5838f6505e435 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 12:41:00 +0000 Subject: [PATCH 16/32] fix: fix frontend display of error messages --- .../OrganizationSettingsPage.tsx | 19 +++++++++++++++---- .../OrganizationSettingsPageView.tsx | 5 ++++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx index 13c339dcc3c09..80b9291c522a1 100644 --- a/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx @@ -1,9 +1,11 @@ +import { getErrorMessage } from "api/errors"; import { deleteOrganization, updateOrganization, } from "api/queries/organizations"; import { EmptyState } from "components/EmptyState/EmptyState"; import { displaySuccess } from "components/GlobalSnackbar/utils"; +import { displayError } from "components/GlobalSnackbar/utils"; import { useOrganizationSettings } from "modules/management/OrganizationSettingsLayout"; import type { FC } from "react"; import { useMutation, useQueryClient } from "react-query"; @@ -42,10 +44,19 @@ const OrganizationSettingsPage: FC = () => { navigate(`/organizations/${updatedOrganization.name}/settings`); displaySuccess("Organization settings updated."); }} - onDeleteOrganization={() => { - deleteOrganizationMutation.mutate(organization.id); - displaySuccess("Organization deleted."); - navigate("/organizations"); + onDeleteOrganization={async () => { + try { + await deleteOrganizationMutation.mutateAsync(organization.id); + displaySuccess(`Organization ${organization.name} deleted`); + navigate("/organizations"); + } catch (error) { + displayError( + getErrorMessage( + error, + `Failed to delete organization: ${organization.name}`, + ), + ); + } }} /> ); diff --git a/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPageView.tsx b/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPageView.tsx index 08199c0d65f4f..8ca6c517b251e 100644 --- a/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPageView.tsx +++ b/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPageView.tsx @@ -146,7 +146,10 @@ export const OrganizationSettingsPageView: FC< { + await onDeleteOrganization(); + setIsDeleting(false); + }} onCancel={() => setIsDeleting(false)} entity="organization" name={organization.name} From 5968da18980e1272ed5029887054e44ba99b8111 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 13:22:22 +0000 Subject: [PATCH 17/32] fix: fix tests --- coderd/database/dbauthz/dbauthz_test.go | 2 +- coderd/database/dump.sql | 2 -- .../migrations/000296_organization_soft_delete.down.sql | 4 ++-- coderd/database/unique_constraint.go | 1 - 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index c960f06c65f1b..e4b695cdaa11b 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -815,7 +815,7 @@ func (s *MethodTestSuite) TestOrganization() { })) s.Run("GetOrganizationByName", s.Subtest(func(db database.Store, check *expects) { o := dbgen.Organization(s.T(), db, database.Organization{}) - check.Args(o.Name).Asserts(o, policy.ActionRead).Returns(o) + check.Args(database.GetOrganizationByNameParams{Name: o.Name, Deleted: o.Deleted}).Asserts(o, policy.ActionRead).Returns(o) })) s.Run("GetOrganizationIDsByMemberIDs", s.Subtest(func(db database.Store, check *expects) { oa := dbgen.Organization(s.T(), db, database.Organization{}) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index b4e578a1c5e48..a0e19f0e8a28e 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -2269,8 +2269,6 @@ CREATE INDEX idx_organization_member_organization_id_uuid ON organization_member CREATE INDEX idx_organization_member_user_id_uuid ON organization_members USING btree (user_id); -CREATE UNIQUE INDEX idx_organization_name ON organizations USING btree (name) WHERE (deleted = false); - CREATE UNIQUE INDEX idx_organization_name_lower ON organizations USING btree (lower(name)) WHERE (deleted = false); CREATE UNIQUE INDEX idx_provisioner_daemons_org_name_owner_key ON provisioner_daemons USING btree (organization_id, name, lower(COALESCE((tags ->> 'owner'::text), ''::text))); diff --git a/coderd/database/migrations/000296_organization_soft_delete.down.sql b/coderd/database/migrations/000296_organization_soft_delete.down.sql index 6d4a48685ed9b..44a0628a41c6d 100644 --- a/coderd/database/migrations/000296_organization_soft_delete.down.sql +++ b/coderd/database/migrations/000296_organization_soft_delete.down.sql @@ -1,5 +1,3 @@ -ALTER TABLE organizations DROP COLUMN deleted; - DROP INDEX IF EXISTS idx_organization_name_lower; CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name ON organizations USING btree (name); @@ -10,3 +8,5 @@ ALTER TABLE ONLY organizations DROP TRIGGER IF EXISTS protect_provisioned_organizations ON organizations; DROP FUNCTION IF EXISTS protect_provisioned_organizations; + +ALTER TABLE organizations DROP COLUMN deleted; diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index 1b57cb5742b2f..db68849777247 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -93,7 +93,6 @@ const ( UniqueWorkspacesPkey UniqueConstraint = "workspaces_pkey" // ALTER TABLE ONLY workspaces ADD CONSTRAINT workspaces_pkey PRIMARY KEY (id); UniqueIndexAPIKeyName UniqueConstraint = "idx_api_key_name" // CREATE UNIQUE INDEX idx_api_key_name ON api_keys USING btree (user_id, token_name) WHERE (login_type = 'token'::login_type); UniqueIndexCustomRolesNameLower UniqueConstraint = "idx_custom_roles_name_lower" // CREATE UNIQUE INDEX idx_custom_roles_name_lower ON custom_roles USING btree (lower(name)); - UniqueIndexOrganizationName UniqueConstraint = "idx_organization_name" // CREATE UNIQUE INDEX idx_organization_name ON organizations USING btree (name) WHERE (deleted = false); UniqueIndexOrganizationNameLower UniqueConstraint = "idx_organization_name_lower" // CREATE UNIQUE INDEX idx_organization_name_lower ON organizations USING btree (lower(name)) WHERE (deleted = false); UniqueIndexProvisionerDaemonsOrgNameOwnerKey UniqueConstraint = "idx_provisioner_daemons_org_name_owner_key" // CREATE UNIQUE INDEX idx_provisioner_daemons_org_name_owner_key ON provisioner_daemons USING btree (organization_id, name, lower(COALESCE((tags ->> 'owner'::text), ''::text))); UniqueIndexUsersEmail UniqueConstraint = "idx_users_email" // CREATE UNIQUE INDEX idx_users_email ON users USING btree (email) WHERE (deleted = false); From 70bf5e38c836877e1c78367799598162adcf19e8 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 16:25:54 +0000 Subject: [PATCH 18/32] fix: fix test --- coderd/database/dbauthz/dbauthz_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index e4b695cdaa11b..db4e68721538d 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -839,7 +839,7 @@ func (s *MethodTestSuite) TestOrganization() { _ = dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{UserID: u.ID, OrganizationID: a.ID}) b := dbgen.Organization(s.T(), db, database.Organization{}) _ = dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{UserID: u.ID, OrganizationID: b.ID}) - check.Args(u.ID).Asserts(a, policy.ActionRead, b, policy.ActionRead).Returns(slice.New(a, b)) + check.Args(database.GetOrganizationsByUserIDParams{UserID: u.ID, Deleted: false}).Asserts(a, policy.ActionRead, b, policy.ActionRead).Returns(slice.New(a, b)) })) s.Run("InsertOrganization", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertOrganizationParams{ @@ -960,13 +960,14 @@ func (s *MethodTestSuite) TestOrganization() { Name: "something-different", }).Asserts(o, policy.ActionUpdate) })) - s.Run("DeleteOrganization", s.Subtest(func(db database.Store, check *expects) { + s.Run("UpdateOrganizationDeletedByID", s.Subtest(func(db database.Store, check *expects) { o := dbgen.Organization(s.T(), db, database.Organization{ Name: "doomed", }) - check.Args( - o.ID, - ).Asserts(o, policy.ActionDelete) + check.Args(database.UpdateOrganizationDeletedByIDParams{ + ID: o.ID, + UpdatedAt: o.UpdatedAt, + }).Asserts(o, policy.ActionDelete).Returns() })) s.Run("OrganizationMembers", s.Subtest(func(db database.Store, check *expects) { o := dbgen.Organization(s.T(), db, database.Organization{}) From 50074d1b34500dcbf38d7725dd3b3bb53a3a163e Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 17:58:33 +0000 Subject: [PATCH 19/32] fix fix e2e test --- site/e2e/tests/organizations.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/e2e/tests/organizations.spec.ts b/site/e2e/tests/organizations.spec.ts index 5a1cf4ba82c0c..825b69b8f4036 100644 --- a/site/e2e/tests/organizations.spec.ts +++ b/site/e2e/tests/organizations.spec.ts @@ -52,5 +52,5 @@ test("create and delete organization", async ({ page }) => { const dialog = page.getByTestId("dialog"); await dialog.getByLabel("Name").fill(newName); await dialog.getByRole("button", { name: "Delete" }).click(); - await expect(page.getByText("Organization deleted.")).toBeVisible(); + await expect(page.getByText(`Organization ${newName} deleted.`)).toBeVisible(); }); From eed03feb41fa1e3b8d30badbb0b92ccefa75767f Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 20:01:44 +0000 Subject: [PATCH 20/32] fix: fix format --- site/e2e/tests/organizations.spec.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/site/e2e/tests/organizations.spec.ts b/site/e2e/tests/organizations.spec.ts index 825b69b8f4036..4a1578af73aaf 100644 --- a/site/e2e/tests/organizations.spec.ts +++ b/site/e2e/tests/organizations.spec.ts @@ -52,5 +52,7 @@ test("create and delete organization", async ({ page }) => { const dialog = page.getByTestId("dialog"); await dialog.getByLabel("Name").fill(newName); await dialog.getByRole("button", { name: "Delete" }).click(); - await expect(page.getByText(`Organization ${newName} deleted.`)).toBeVisible(); + await expect( + page.getByText(`Organization ${newName} deleted.`), + ).toBeVisible(); }); From 891cbae8d1dbf089a7dff4721b3be61fa2e03668 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 20:15:15 +0000 Subject: [PATCH 21/32] fix: update trigger --- .../000296_organization_soft_delete.up.sql | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/coderd/database/migrations/000296_organization_soft_delete.up.sql b/coderd/database/migrations/000296_organization_soft_delete.up.sql index 29651647c0559..0eacdba063cdc 100644 --- a/coderd/database/migrations/000296_organization_soft_delete.up.sql +++ b/coderd/database/migrations/000296_organization_soft_delete.up.sql @@ -17,6 +17,7 @@ DECLARE template_count int; group_count int; member_count int; + provisioner_keys_count int; BEGIN workspace_count := ( SELECT count(*) as count FROM workspaces @@ -44,6 +45,12 @@ BEGIN organization_members.organization_id = OLD.id ); + provisioner_keys_count := ( + Select count(*) as count FROM provisioner_keys + WHERE + provisioner_keys.organization_id = OLD.id + ) + -- Fail the deletion if one of the following: -- * the organization has 1 or more workspaces -- * the organization has 1 or more templates @@ -54,8 +61,20 @@ BEGIN RAISE EXCEPTION 'cannot delete organization: organization has % workspaces and % templates that must be deleted first', workspace_count, template_count; END IF; - IF (group_count + member_count) > 2 THEN - RAISE EXCEPTION 'cannot delete organization: organization has % groups and % members that must be deleted first', group_count - 1, member_count - 1; + IF (group_count) > 1 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % groups that must be deleted first', group_count - 1; + END IF; + + IF (provisioner_keys_count) > 0 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % provisioner keys that must be deleted first', provisioner_keys_count; + END IF; + + -- If member count > 1 + -- Allow 1 member to exist, because you cannot remove yourself. You can + -- remove everyone else. Ideally, we only omit the member that matches + -- the user_id of the caller, however in a trigger, the caller is unknown. + IF (member_count) > 1 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % members that must be deleted first', member_count - 1; END IF; RETURN NEW; From 22d27f82ad916ff792ff75977ddb54e8d2b9963c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 21 Feb 2025 14:30:21 -0600 Subject: [PATCH 22/32] chore: implement unit test to test trigger with >0 workspaces --- coderd/database/querier_test.go | 35 +++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 00b189967f5a6..c3c3867e763a7 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -21,6 +21,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" @@ -2916,6 +2917,40 @@ func TestGetUserStatusCounts(t *testing.T) { } } +func TestOrganizationDeleteTrigger(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.SkipNow() + } + + t.Run("WorkspaceExists", func(t *testing.T) { + db, _ := dbtestutil.NewDB(t) + + orgA := dbgen.Organization(t, db, database.Organization{}) + _, err := db.InsertAllUsersGroup(context.Background(), orgA.ID) + require.NoError(t, err) + + user := dbgen.User(t, db, database.User{}) + + dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OrganizationID: orgA.ID, + OwnerID: user.ID, + }).Do() + + ctx := testutil.Context(t, testutil.WaitShort) + err = db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + UpdatedAt: dbtime.Now(), + ID: orgA.ID, + }) + require.Error(t, err) + // cannot delete organization: organization has 1 workspaces and 1 templates that must be deleted first + require.ErrorContains(t, err, "cannot delete organization") + require.ErrorContains(t, err, "has 1 workspaces") + require.ErrorContains(t, err, "1 templates") + }) +} + func requireUsersMatch(t testing.TB, expected []database.User, found []database.GetUsersRow, msg string) { t.Helper() require.ElementsMatch(t, expected, database.ConvertUserRows(found), msg) From dc5fc46c4876cee61a061329b33b7f8a55132461 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 21 Feb 2025 14:35:51 -0600 Subject: [PATCH 23/32] use dbfake --- coderd/database/querier_test.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index c3c3867e763a7..806a7f7866777 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -2927,21 +2927,19 @@ func TestOrganizationDeleteTrigger(t *testing.T) { t.Run("WorkspaceExists", func(t *testing.T) { db, _ := dbtestutil.NewDB(t) - orgA := dbgen.Organization(t, db, database.Organization{}) - _, err := db.InsertAllUsersGroup(context.Background(), orgA.ID) - require.NoError(t, err) + orgA := dbfake.Organization(t, db).Do() user := dbgen.User(t, db, database.User{}) dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ - OrganizationID: orgA.ID, + OrganizationID: orgA.Org.ID, OwnerID: user.ID, }).Do() ctx := testutil.Context(t, testutil.WaitShort) - err = db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + err := db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ UpdatedAt: dbtime.Now(), - ID: orgA.ID, + ID: orgA.Org.ID, }) require.Error(t, err) // cannot delete organization: organization has 1 workspaces and 1 templates that must be deleted first From 9466ff724bfd5f1a326d8911d4c371a50666c649 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 20:39:24 +0000 Subject: [PATCH 24/32] fix: trigger --- .../migrations/000296_organization_soft_delete.up.sql | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/coderd/database/migrations/000296_organization_soft_delete.up.sql b/coderd/database/migrations/000296_organization_soft_delete.up.sql index 0eacdba063cdc..53530887f0793 100644 --- a/coderd/database/migrations/000296_organization_soft_delete.up.sql +++ b/coderd/database/migrations/000296_organization_soft_delete.up.sql @@ -49,7 +49,7 @@ BEGIN Select count(*) as count FROM provisioner_keys WHERE provisioner_keys.organization_id = OLD.id - ) + ); -- Fail the deletion if one of the following: -- * the organization has 1 or more workspaces @@ -57,19 +57,14 @@ BEGIN -- * the organization has 1 or more groups other than "Everyone" group -- * the organization has 1 or more members other than the organization owner - IF (workspace_count + template_count) > 0 THEN - RAISE EXCEPTION 'cannot delete organization: organization has % workspaces and % templates that must be deleted first', workspace_count, template_count; + IF (workspace_count + template_count + provisioner_keys_count) > 0 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % workspaces, % templates, and % provisioner keys that must be deleted first', workspace_count, template_count, provisioner_keys_count; END IF; IF (group_count) > 1 THEN RAISE EXCEPTION 'cannot delete organization: organization has % groups that must be deleted first', group_count - 1; END IF; - IF (provisioner_keys_count) > 0 THEN - RAISE EXCEPTION 'cannot delete organization: organization has % provisioner keys that must be deleted first', provisioner_keys_count; - END IF; - - -- If member count > 1 -- Allow 1 member to exist, because you cannot remove yourself. You can -- remove everyone else. Ideally, we only omit the member that matches -- the user_id of the caller, however in a trigger, the caller is unknown. From 041630cf3c752885dfb0ca545c2bd4d7eb172f30 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 21:28:31 +0000 Subject: [PATCH 25/32] chore: update delete org trigger name --- .../migrations/000296_organization_soft_delete.up.sql | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/coderd/database/migrations/000296_organization_soft_delete.up.sql b/coderd/database/migrations/000296_organization_soft_delete.up.sql index 53530887f0793..e313981d8c21c 100644 --- a/coderd/database/migrations/000296_organization_soft_delete.up.sql +++ b/coderd/database/migrations/000296_organization_soft_delete.up.sql @@ -9,7 +9,7 @@ CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name_lower ON organizations U ALTER TABLE ONLY organizations DROP CONSTRAINT IF EXISTS organizations_name; -CREATE FUNCTION protect_provisioned_organizations() +CREATE FUNCTION protect_deleted_organizations() RETURNS TRIGGER AS $$ DECLARE @@ -56,6 +56,7 @@ BEGIN -- * the organization has 1 or more templates -- * the organization has 1 or more groups other than "Everyone" group -- * the organization has 1 or more members other than the organization owner + -- * the organization has 1 or more provisioner keys IF (workspace_count + template_count + provisioner_keys_count) > 0 THEN RAISE EXCEPTION 'cannot delete organization: organization has % workspaces, % templates, and % provisioner keys that must be deleted first', workspace_count, template_count, provisioner_keys_count; @@ -77,7 +78,7 @@ END; $$ LANGUAGE plpgsql; -- Trigger to protect organizations from being soft deleted with existing resources -CREATE TRIGGER protect_provisioned_organizations +CREATE TRIGGER protect_deleted_organizations BEFORE UPDATE ON organizations FOR EACH ROW WHEN (NEW.deleted = true AND OLD.deleted = false) From 607311f1cb366e5e32045c23b3e4b4df57b4f4e1 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 21:28:47 +0000 Subject: [PATCH 26/32] chore: add additional org delete trigger tests --- coderd/database/querier_test.go | 86 +++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 806a7f7866777..f830483784a0d 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -2925,6 +2925,7 @@ func TestOrganizationDeleteTrigger(t *testing.T) { } t.Run("WorkspaceExists", func(t *testing.T) { + t.Parallel() db, _ := dbtestutil.NewDB(t) orgA := dbfake.Organization(t, db).Do() @@ -2947,6 +2948,91 @@ func TestOrganizationDeleteTrigger(t *testing.T) { require.ErrorContains(t, err, "has 1 workspaces") require.ErrorContains(t, err, "1 templates") }) + + t.Run("TemplateExists", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + + orgA := dbfake.Organization(t, db).Do() + + dbgen.Template(t, db, database.Template{ + OrganizationID: orgA.Org.ID, + }) + + ctx := testutil.Context(t, testutil.WaitShort) + err := db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + UpdatedAt: dbtime.Now(), + ID: orgA.Org.ID, + }) + require.Error(t, err) + // cannot delete organization: organization has 1 workspaces and 1 templates that must be deleted first + require.ErrorContains(t, err, "cannot delete organization") + require.ErrorContains(t, err, "has 0 workspaces") + require.ErrorContains(t, err, "1 templates") + }) + + t.Run("ProvisionerKeyExists", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + + orgA := dbfake.Organization(t, db).Do() + + dbgen.ProvisionerKey(t, db, database.ProvisionerKey{ + OrganizationID: orgA.Org.ID, + }) + + ctx := testutil.Context(t, testutil.WaitShort) + err := db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + UpdatedAt: dbtime.Now(), + ID: orgA.Org.ID, + }) + require.Error(t, err) + // cannot delete organization: organization has 1 provisioner keys that must be deleted first + require.ErrorContains(t, err, "cannot delete organization") + require.ErrorContains(t, err, "has 1 provisioner keys") + }) + + t.Run("GroupExists", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + + orgA := dbfake.Organization(t, db).Do() + + dbgen.Group(t, db, database.Group{ + OrganizationID: orgA.Org.ID, + }) + + ctx := testutil.Context(t, testutil.WaitShort) + err := db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + UpdatedAt: dbtime.Now(), + ID: orgA.Org.ID, + }) + require.Error(t, err) + // cannot delete organization: organization has 1 groups that must be deleted first + require.ErrorContains(t, err, "cannot delete organization") + require.ErrorContains(t, err, "has 1 groups") + }) + + t.Run("MemberExists", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + + orgA := dbfake.Organization(t, db).Do() + + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + OrganizationID: orgA.Org.ID, + }) + + ctx := testutil.Context(t, testutil.WaitShort) + err := db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + UpdatedAt: dbtime.Now(), + ID: orgA.Org.ID, + }) + require.Error(t, err) + // cannot delete organization: organization has 1 members that must be deleted first + require.ErrorContains(t, err, "cannot delete organization") + require.ErrorContains(t, err, "has 1 members") + }) } func requireUsersMatch(t testing.TB, expected []database.User, found []database.GetUsersRow, msg string) { From 7291c02602c1b285c03efbaba0df5f40b797fbbf Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 21:34:35 +0000 Subject: [PATCH 27/32] chore: change name --- .../migrations/000296_organization_soft_delete.up.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/database/migrations/000296_organization_soft_delete.up.sql b/coderd/database/migrations/000296_organization_soft_delete.up.sql index e313981d8c21c..275a91d63ae51 100644 --- a/coderd/database/migrations/000296_organization_soft_delete.up.sql +++ b/coderd/database/migrations/000296_organization_soft_delete.up.sql @@ -9,7 +9,7 @@ CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name_lower ON organizations U ALTER TABLE ONLY organizations DROP CONSTRAINT IF EXISTS organizations_name; -CREATE FUNCTION protect_deleted_organizations() +CREATE FUNCTION protect_deleting_organizations() RETURNS TRIGGER AS $$ DECLARE @@ -78,7 +78,7 @@ END; $$ LANGUAGE plpgsql; -- Trigger to protect organizations from being soft deleted with existing resources -CREATE TRIGGER protect_deleted_organizations +CREATE TRIGGER protect_deleting_organizations BEFORE UPDATE ON organizations FOR EACH ROW WHEN (NEW.deleted = true AND OLD.deleted = false) From b9b7a14c6849a713d8579d3d53c38e921f4f5612 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 21:39:41 +0000 Subject: [PATCH 28/32] fix: update trigger name --- coderd/database/dump.sql | 27 ++++++++++++++----- .../000296_organization_soft_delete.down.sql | 4 +-- .../000296_organization_soft_delete.up.sql | 2 +- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index a0e19f0e8a28e..e05d3a06d31f5 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -438,7 +438,7 @@ BEGIN END; $$; -CREATE FUNCTION protect_provisioned_organizations() RETURNS trigger +CREATE FUNCTION protect_deleting_organizations() RETURNS trigger LANGUAGE plpgsql AS $$ DECLARE @@ -446,6 +446,7 @@ DECLARE template_count int; group_count int; member_count int; + provisioner_keys_count int; BEGIN workspace_count := ( SELECT count(*) as count FROM workspaces @@ -473,18 +474,32 @@ BEGIN organization_members.organization_id = OLD.id ); + provisioner_keys_count := ( + Select count(*) as count FROM provisioner_keys + WHERE + provisioner_keys.organization_id = OLD.id + ); + -- Fail the deletion if one of the following: -- * the organization has 1 or more workspaces -- * the organization has 1 or more templates -- * the organization has 1 or more groups other than "Everyone" group -- * the organization has 1 or more members other than the organization owner + -- * the organization has 1 or more provisioner keys + + IF (workspace_count + template_count + provisioner_keys_count) > 0 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % workspaces, % templates, and % provisioner keys that must be deleted first', workspace_count, template_count, provisioner_keys_count; + END IF; - IF (workspace_count + template_count) > 0 THEN - RAISE EXCEPTION 'cannot delete organization: organization has % workspaces and % templates that must be deleted first', workspace_count, template_count; + IF (group_count) > 1 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % groups that must be deleted first', group_count - 1; END IF; - IF (group_count + member_count) > 2 THEN - RAISE EXCEPTION 'cannot delete organization: organization has % groups and % members that must be deleted first', group_count - 1, member_count - 1; + -- Allow 1 member to exist, because you cannot remove yourself. You can + -- remove everyone else. Ideally, we only omit the member that matches + -- the user_id of the caller, however in a trigger, the caller is unknown. + IF (member_count) > 1 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % members that must be deleted first', member_count - 1; END IF; RETURN NEW; @@ -2401,7 +2416,7 @@ CREATE OR REPLACE VIEW provisioner_job_stats AS CREATE TRIGGER inhibit_enqueue_if_disabled BEFORE INSERT ON notification_messages FOR EACH ROW EXECUTE FUNCTION inhibit_enqueue_if_disabled(); -CREATE TRIGGER protect_provisioned_organizations BEFORE UPDATE ON organizations FOR EACH ROW WHEN (((new.deleted = true) AND (old.deleted = false))) EXECUTE FUNCTION protect_provisioned_organizations(); +CREATE TRIGGER protect_deleting_organizations BEFORE UPDATE ON organizations FOR EACH ROW WHEN (((new.deleted = true) AND (old.deleted = false))) EXECUTE FUNCTION protect_deleting_organizations(); CREATE TRIGGER remove_organization_member_custom_role BEFORE DELETE ON custom_roles FOR EACH ROW EXECUTE FUNCTION remove_organization_member_role(); diff --git a/coderd/database/migrations/000296_organization_soft_delete.down.sql b/coderd/database/migrations/000296_organization_soft_delete.down.sql index 44a0628a41c6d..3db107e8a79f5 100644 --- a/coderd/database/migrations/000296_organization_soft_delete.down.sql +++ b/coderd/database/migrations/000296_organization_soft_delete.down.sql @@ -6,7 +6,7 @@ CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name_lower ON organizations U ALTER TABLE ONLY organizations ADD CONSTRAINT organizations_name UNIQUE (name); -DROP TRIGGER IF EXISTS protect_provisioned_organizations ON organizations; -DROP FUNCTION IF EXISTS protect_provisioned_organizations; +DROP TRIGGER IF EXISTS protect_deleting_organizations ON organizations; +DROP FUNCTION IF EXISTS protect_deleting_organizations; ALTER TABLE organizations DROP COLUMN deleted; diff --git a/coderd/database/migrations/000296_organization_soft_delete.up.sql b/coderd/database/migrations/000296_organization_soft_delete.up.sql index 275a91d63ae51..34b25139c950a 100644 --- a/coderd/database/migrations/000296_organization_soft_delete.up.sql +++ b/coderd/database/migrations/000296_organization_soft_delete.up.sql @@ -82,4 +82,4 @@ CREATE TRIGGER protect_deleting_organizations BEFORE UPDATE ON organizations FOR EACH ROW WHEN (NEW.deleted = true AND OLD.deleted = false) - EXECUTE FUNCTION protect_provisioned_organizations(); + EXECUTE FUNCTION protect_deleting_organizations(); From 9af7cb52099081e90a3062ed48479515c5e05e1e Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 22:49:16 +0000 Subject: [PATCH 29/32] fix: fix tests --- coderd/database/querier_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index f830483784a0d..8fe08af841293 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -2955,8 +2955,11 @@ func TestOrganizationDeleteTrigger(t *testing.T) { orgA := dbfake.Organization(t, db).Do() + user := dbgen.User(t, db, database.User{}) + dbgen.Template(t, db, database.Template{ OrganizationID: orgA.Org.ID, + CreatedBy: user.ID, }) ctx := testutil.Context(t, testutil.WaitShort) @@ -2989,7 +2992,7 @@ func TestOrganizationDeleteTrigger(t *testing.T) { require.Error(t, err) // cannot delete organization: organization has 1 provisioner keys that must be deleted first require.ErrorContains(t, err, "cannot delete organization") - require.ErrorContains(t, err, "has 1 provisioner keys") + require.ErrorContains(t, err, "1 provisioner keys") }) t.Run("GroupExists", func(t *testing.T) { @@ -3019,8 +3022,11 @@ func TestOrganizationDeleteTrigger(t *testing.T) { orgA := dbfake.Organization(t, db).Do() + user := dbgen.User(t, db, database.User{}) + dbgen.OrganizationMember(t, db, database.OrganizationMember{ OrganizationID: orgA.Org.ID, + UserID: user.ID, }) ctx := testutil.Context(t, testutil.WaitShort) From aee6834c778f990ed9672c0e20f2ad207aec48d9 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 22:57:35 +0000 Subject: [PATCH 30/32] fix: fix test --- coderd/database/querier_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 8fe08af841293..34dcef734c7e6 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -3022,11 +3022,17 @@ func TestOrganizationDeleteTrigger(t *testing.T) { orgA := dbfake.Organization(t, db).Do() - user := dbgen.User(t, db, database.User{}) + userA := dbgen.User(t, db, database.User{}) + userB := dbgen.User(t, db, database.User{}) + + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + OrganizationID: orgA.Org.ID, + UserID: userA.ID, + }) dbgen.OrganizationMember(t, db, database.OrganizationMember{ OrganizationID: orgA.Org.ID, - UserID: user.ID, + UserID: userB.ID, }) ctx := testutil.Context(t, testutil.WaitShort) From 74ef3deeb6b7e15b22b1e2da7d1c382e9fb24930 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 21 Feb 2025 23:20:52 +0000 Subject: [PATCH 31/32] fix: update e2e test --- site/e2e/tests/organizations.spec.ts | 5 ++--- .../OrganizationSettingsPage.tsx | 9 ++------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/site/e2e/tests/organizations.spec.ts b/site/e2e/tests/organizations.spec.ts index 4a1578af73aaf..ff4f5ad993f19 100644 --- a/site/e2e/tests/organizations.spec.ts +++ b/site/e2e/tests/organizations.spec.ts @@ -52,7 +52,6 @@ test("create and delete organization", async ({ page }) => { const dialog = page.getByTestId("dialog"); await dialog.getByLabel("Name").fill(newName); await dialog.getByRole("button", { name: "Delete" }).click(); - await expect( - page.getByText(`Organization ${newName} deleted.`), - ).toBeVisible(); + await page.waitForTimeout(1000); + await expect(page.getByText("Organization deleted")).toBeVisible(); }); diff --git a/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx index 80b9291c522a1..3ae72b701c851 100644 --- a/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx @@ -47,15 +47,10 @@ const OrganizationSettingsPage: FC = () => { onDeleteOrganization={async () => { try { await deleteOrganizationMutation.mutateAsync(organization.id); - displaySuccess(`Organization ${organization.name} deleted`); + displaySuccess("Organization deleted"); navigate("/organizations"); } catch (error) { - displayError( - getErrorMessage( - error, - `Failed to delete organization: ${organization.name}`, - ), - ); + displayError(getErrorMessage(error, "Failed to delete organization")); } }} /> From 4c70aaa6c839e78bba02f0bcf2c137a446844005 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Mon, 24 Feb 2025 12:34:34 -0500 Subject: [PATCH 32/32] Update coderd/database/querier_test.go Co-authored-by: Steven Masley --- coderd/database/querier_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 34dcef734c7e6..b60554de75359 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -2968,7 +2968,7 @@ func TestOrganizationDeleteTrigger(t *testing.T) { ID: orgA.Org.ID, }) require.Error(t, err) - // cannot delete organization: organization has 1 workspaces and 1 templates that must be deleted first + // cannot delete organization: organization has 0 workspaces and 1 templates that must be deleted first require.ErrorContains(t, err, "cannot delete organization") require.ErrorContains(t, err, "has 0 workspaces") require.ErrorContains(t, err, "1 templates")